Closed Bug 1404180 Opened 3 years ago Closed 3 years ago

stylo: Assertion failure: !mInStyleRefresh [@ mozilla::ServoRestyleManager::ContentStateChanged]

Categories

(Core :: CSS Parsing and Computation, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 --- fixed
firefox58 --- fixed

People

(Reporter: tsmith, Assigned: heycam)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(2 files)

Attached file test_case.html
Assertion failure: !mInStyleRefresh, at /src/layout/base/ServoRestyleManager.cpp:1221

#0 mozilla::ServoRestyleManager::ContentStateChanged(nsIContent*, mozilla::EventStates) /src/layout/base/ServoRestyleManager.cpp:1221:3
#1 mozilla::PresShell::ContentStateChanged(nsIDocument*, nsIContent*, mozilla::EventStates) /src/layout/base/PresShell.cpp:4279:37
#2 nsDocument::ContentStateChanged(nsIContent*, mozilla::EventStates) /src/dom/base/nsDocument.cpp:5668:3
#3 mozilla::dom::Element::UpdateState(bool) /src/dom/base/Element.cpp:272:14
#4 nsGenericHTMLFormElement::UpdateFormOwner(bool, mozilla::dom::Element*) /src/dom/html/nsGenericHTMLElement.cpp:2358:5
#5 nsGenericHTMLFormElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/html/nsGenericHTMLElement.cpp:1896:5
#6 mozilla::dom::HTMLOutputElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/html/HTMLOutputElement.cpp:132:43
#7 mozilla::dom::Element::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/base/Element.cpp:1678:17
#8 nsGenericHTMLElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/html/nsGenericHTMLElement.cpp:482:43
#9 mozilla::dom::Element::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/base/Element.cpp:1678:17
#10 nsGenericHTMLElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/html/nsGenericHTMLElement.cpp:482:43
#11 mozilla::dom::Element::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/base/Element.cpp:1678:17
#12 nsGenericHTMLElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/html/nsGenericHTMLElement.cpp:482:43
#13 mozilla::dom::Element::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/base/Element.cpp:1678:17
#14 nsGenericHTMLElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/html/nsGenericHTMLElement.cpp:482:43
#15 mozilla::dom::HTMLTableElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/html/HTMLTableElement.cpp:1177:39
#16 mozilla::dom::Element::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/base/Element.cpp:1678:17
#17 nsGenericHTMLElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/html/nsGenericHTMLElement.cpp:482:43
#18 mozilla::dom::HTMLLinkElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/html/HTMLLinkElement.cpp:152:39
#19 mozilla::dom::Element::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/base/Element.cpp:1678:17
#20 nsGenericHTMLElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/html/nsGenericHTMLElement.cpp:482:43
#21 nsGenericHTMLFormElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/html/nsGenericHTMLElement.cpp:1872:39
#22 mozilla::dom::HTMLOutputElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/html/HTMLOutputElement.cpp:132:43
#23 mozilla::dom::Element::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/base/Element.cpp:1678:17
#24 nsGenericHTMLElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/html/nsGenericHTMLElement.cpp:482:43
#25 mozilla::dom::Element::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/base/Element.cpp:1678:17
#26 nsSVGElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/svg/nsSVGElement.cpp:268:35
#27 mozilla::dom::SVGAnimationElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/svg/SVGAnimationElement.cpp:185:42
#28 mozilla::dom::Element::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/base/Element.cpp:1678:17
#29 nsSVGElement::BindToTree(nsIDocument*, nsIContent*, nsIContent*, bool) /src/dom/svg/nsSVGElement.cpp:268:35
#30 nsCSSFrameConstructor::GetAnonymousContent(nsIContent*, nsIFrame*, nsTArray<nsIAnonymousContentCreator::ContentInfo>&) /src/layout/base/nsCSSFrameConstructor.cpp:4388:19
#31 nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState&, nsIContent*, nsStyleContext*, nsContainerFrame*, bool, nsFrameItems&, bool, PendingBinding*, nsIFrame*) /src/layout/base/nsCSSFrameConstructor.cpp:11282:3
#32 nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem&, nsFrameConstructorState&, nsContainerFrame*, nsFrameItems&) /src/layout/base/nsCSSFrameConstructor.cpp:4199:9
#33 nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsContainerFrame*, nsFrameItems&) /src/layout/base/nsCSSFrameConstructor.cpp:6406:3
#34 nsCSSFrameConstructor::ConstructFramesFromItemList(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList&, nsContainerFrame*, bool, nsFrameItems&) /src/layout/base/nsCSSFrameConstructor.cpp:11054:5
#35 nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsIContent*, nsILayoutHistoryState*, nsCSSFrameConstructor::InsertionKind, TreeMatchContext*) /src/layout/base/nsCSSFrameConstructor.cpp:8427:3
#36 nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, nsCSSFrameConstructor::InsertionKind) /src/layout/base/nsCSSFrameConstructor.cpp:10090:9
#37 mozilla::RestyleManager::ProcessRestyledFrames(nsStyleChangeList&) /src/layout/base/RestyleManager.cpp:1514:25
#38 mozilla::ServoRestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:1126:9
#39 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /src/layout/base/PresShell.cpp:4170:41
#40 nsRefreshDriver::Tick(long, mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:1921:18
#41 mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) /src/layout/base/nsRefreshDriver.cpp:307:7
#42 mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:328:5
#43 mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:770:5
#44 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:683:35
#45 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() /src/layout/base/nsRefreshDriver.cpp:529:20
#46 nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1039:14
#47 NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:524:10
#48 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21
#49 MessageLoop::RunInternal() /src/ipc/chromium/src/base/message_loop.cc:326:10
#50 MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299:3
#51 nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:158:27
#52 nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:288:30
#53 XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4701:22
#54 XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4865:8
#55 XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4960:21
#56 do_main(int, char**, char**) /src/browser/app/nsBrowserApp.cpp:236:22
#57 main /src/browser/app/nsBrowserApp.cpp:309:16
#58 __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
#59 _start (firefox+0x41eb24)
Flags: in-testsuite?
Hmm..., anonymous content bound to a <form> element is no good, I think... Though it seems this anon content is just the content of the page? I'm not familiar with <svg:use> elements, but that can probably cause some badness...
Priority: -- → P2
I'll grab this.  An SVG <use> element's anonymous subtree is NAC but we don't use the NAC flag on it.  We probably shouldn't be finding the ancestor form in nsGenericHTMLElement::FindAncestorForm, but the lack of NODE_IS_NATIVE_ANONYMOUS flag means we continue looking up outside the shadow tree.
Assignee: nobody → cam
Status: NEW → ASSIGNED
No, it's not to do with the flags at all.  We would correctly stop the FindAncestorForm loop at the <use> element, but that's not what we're calling.  The <output> element has a form="" attribute, so we just look up the element (which is outside the AC subtree) by ID.
Comment on attachment 8914172 [details]
Bug 1404180 - Don't allow associating form elements by ID to <form>s across anonymous subtree boundaries.

https://reviewboard.mozilla.org/r/185516/#review190512

Could you please file a followup bug to fix .form handling in Shadow DOM. Make it block bug 1205323.
Attachment #8914172 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #6)
> Could you please file a followup bug to fix .form handling in Shadow DOM.
> Make it block bug 1205323.

To clarify, fix which way?
Flags: needinfo?(bugs)
Feel free to file just a bug saying something like
"Ensure formcontrol.form works per spec in Shadow DOM"
Flags: needinfo?(bugs)
OK, filed bug 1404864.
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 57f6961cd94b -d cb221931ca1d: rebasing 423597:57f6961cd94b "Bug 1404180 - Don't allow associating form elements by ID to <form>s across anonymous subtree boundaries. r=smaug" (tip)
merging layout/style/crashtests/crashtests.list
warning: conflicts while merging layout/style/crashtests/crashtests.list! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/123004932443
Don't allow associating form elements by ID to <form>s across anonymous subtree boundaries. r=smaug
https://hg.mozilla.org/mozilla-central/rev/123004932443
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Comment on attachment 8914172 [details]
Bug 1404180 - Don't allow associating form elements by ID to <form>s across anonymous subtree boundaries.

Approval Request Comment
[Feature/Bug causing the regression]: not sure, but something stylo related
[User impact if declined]: potential incorrect styling of <form> elements when SVG <use> elements are involved
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no, crashtest is enough
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: very small patch, removes potentially confusing interactions between SVG <use> and <form>s
[String changes made/needed]: none

This also fixes a fatal debug assertion, so is probably helpful for fuzzers to get this fixed.
Attachment #8914172 - Flags: approval-mozilla-beta?
Flags: in-testsuite? → in-testsuite+
Comment on attachment 8914172 [details]
Bug 1404180 - Don't allow associating form elements by ID to <form>s across anonymous subtree boundaries.

Stylo related, Beta57+
Attachment #8914172 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
See Also: → 1409183
You need to log in before you can comment on or make changes to this bug.