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

Status: RESOLVED FIXED

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

Description

[Tyson Smith [:tsmith]]

Attachment: 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)

Comment 1

[Emilio Cobos Álvarez (:emilio)]

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...

Comment 2

[Cameron McCormack (:heycam)]

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.

Comment 3

[Cameron McCormack (:heycam)]

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 4

[Cameron McCormack (:heycam)]

Attachment: Bug 1404180 - Don't allow associating form elements by ID to <form>s across anonymous subtree boundaries.

Review commit: https://reviewboard.mozilla.org/r/185516/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/185516/

Comment 5

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a70cc091c066897e789e1a7691e1b050b158de8d

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

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.

Comment 7

[Cameron McCormack (:heycam)]

(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?

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Feel free to file just a bug saying something like
"Ensure formcontrol.form works per spec in Shadow DOM"

Comment 9

[Cameron McCormack (:heycam)]

OK, filed bug 1404864.

Comment 10

[Mozilla Autoland]

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)

Comment 11

[Pulsebot]

Pushed by [e-mail]:
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

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/123004932443

Comment 13

[Cameron McCormack (:heycam)]

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.

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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+

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/717f66b3515e