stylo: Assertion failure: aElement->GetComposedDoc()->GetServoRestyleRoot() [@ NoteDirtyElement]

RESOLVED FIXED in Firefox 57

Status

()

P2
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: tsmith, Assigned: emilio)

Tracking

(Blocks: 1 bug, {assertion, testcase})

57 Branch
mozilla58
assertion, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 unaffected, firefox57 fixed, firefox58 fixed)

Details

Attachments

(2 attachments)

Created attachment 8913427 [details]
test_case.html

Assertion failure: aElement->GetComposedDoc()->GetServoRestyleRoot(), at /src/dom/base/Element.cpp:4385

#0 NoteDirtyElement(mozilla::dom::Element*, unsigned int) /src/dom/base/Element.cpp:4399:31
#1 mozilla::ServoRestyleManager::SnapshotFor(mozilla::dom::Element*) /src/layout/base/ServoRestyleManager.cpp:1049:13
#2 mozilla::ServoRestyleManager::ContentStateChanged(nsIContent*, mozilla::EventStates) /src/layout/base/ServoRestyleManager.cpp:1245:36
#3 mozilla::PresShell::ContentStateChanged(nsIDocument*, nsIContent*, mozilla::EventStates) /src/layout/base/PresShell.cpp:4279:37
#4 nsDocument::ContentStateChanged(nsIContent*, mozilla::EventStates) /src/dom/base/nsDocument.cpp:5667:3
#5 mozilla::dom::Element::UpdateState(bool) /src/dom/base/Element.cpp:272:14
#6 mozilla::dom::Link::ResetLinkState(bool, bool) /src/dom/base/Link.cpp:792:15
#7 nsDocument::RefreshLinkHrefs() /src/dom/base/nsDocument.cpp:9598:23
#8 nsDocument::SetBaseURI(nsIURI*) /src/dom/base/nsDocument.cpp:3881:3
#9 mozilla::dom::SetBaseURIUsingFirstBaseWithHref(nsIDocument*, nsIContent*) /src/dom/html/HTMLSharedElement.cpp:201:14
#10 mozilla::dom::HTMLSharedElement::UnbindFromTree(bool, bool) /src/dom/html/HTMLSharedElement.cpp:294:7
#11 mozilla::dom::Element::UnbindFromTree(bool, bool) /src/dom/base/Element.cpp:1977:37
#12 nsGenericHTMLElement::UnbindFromTree(bool, bool) /src/dom/html/nsGenericHTMLElement.cpp:537:20
#13 nsINode::doRemoveChildAt(unsigned int, bool, nsIContent*, nsAttrAndChildArray&) /src/dom/base/nsINode.cpp:1945:9
#14 mozilla::dom::FragmentOrElement::RemoveChildAt(unsigned int, bool) /src/dom/base/FragmentOrElement.cpp:1368:5
#15 nsINode::ReplaceOrInsertBefore(bool, nsINode*, nsINode*, mozilla::ErrorResult&) /src/dom/base/nsINode.cpp:2432:5
#16 nsINode::ReplaceWith(mozilla::dom::Sequence<mozilla::dom::OwningNodeOrString> const&, mozilla::ErrorResult&) /src/dom/base/nsINode.cpp:1836:13
#17 mozilla::dom::ElementBinding::replaceWith(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, JSJitMethodCallArgs const&) /src/obj-firefox/dom/bindings/ElementBinding.cpp:3687:9
#18 mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /src/dom/bindings/BindingUtils.cpp:3055:13
#19 js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /src/js/src/jscntxtinlines.h:293:15
#20 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:16
#21 InternalCall(JSContext*, js::AnyInvokeArgs const&) /src/js/src/vm/Interpreter.cpp:540:12
#22 Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3084:18
#23 js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:435:12
#24 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:513:15
#25 InternalCall(JSContext*, js::AnyInvokeArgs const&) /src/js/src/vm/Interpreter.cpp:540:12
#26 js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:559:10
#27 JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:2965:12
#28 mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/EventHandlerBinding.cpp:260:37
#29 void mozilla::dom::EventHandlerNonNull::Call<nsISupports*>(nsISupports* const&, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JSCompartment*) /src/obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:362:12
#30 mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*) /src/dom/events/JSEventHandler.cpp:215:12
#31 mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1112:51
#32 mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /src/dom/events/EventListenerManager.cpp:1283:20
#33 mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:313:17
#34 mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:462:16
#35 mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /src/dom/events/EventDispatcher.cpp:822:9
#36 nsDocumentViewer::LoadComplete(nsresult) /src/layout/base/nsDocumentViewer.cpp:1081:7
#37 nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /src/docshell/base/nsDocShell.cpp:7760:21
#38 nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp:7558:7
#39 non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp:7455:13
#40 nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) /src/uriloader/base/nsDocLoader.cpp:1320:3
#41 nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) /src/uriloader/base/nsDocLoader.cpp:861:14
#42 nsDocLoader::DocLoaderIsEmpty(bool) /src/uriloader/base/nsDocLoader.cpp:750:9
#43 nsDocLoader::DocLoaderIsEmpty(bool) /src/uriloader/base/nsDocLoader.cpp:753:19
#44 nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /src/uriloader/base/nsDocLoader.cpp:632:5
#45 non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /src/uriloader/base/nsDocLoader.cpp:488:14
#46 mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /src/netwerk/base/nsLoadGroup.cpp:629:28
#47 nsDocument::DoUnblockOnload() /src/dom/base/nsDocument.cpp:9234:18
#48 nsDocument::UnblockOnload(bool) /src/dom/base/nsDocument.cpp:9156:9
#49 nsDocument::DispatchContentLoadedEvents() /src/dom/base/nsDocument.cpp:5600:3
#50 mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, (mozilla::RunnableKind)0>::Run() /src/obj-firefox/dist/include/nsThreadUtils.h:1192:13
#51 mozilla::SchedulerGroup::Runnable::Run() /src/xpcom/threads/SchedulerGroup.cpp:396:25
#52 nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1039:14
#53 NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:524:10
#54 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21
#55 MessageLoop::RunInternal() /src/ipc/chromium/src/base/message_loop.cc:326:10
#56 MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299:3
#57 nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:158:27
#58 XRE_RunAppShell() /src/toolkit/xre/nsEmbedFunctions.cpp:880:22
#59 mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:269:9
#60 MessageLoop::RunInternal() /src/ipc/chromium/src/base/message_loop.cc:326:10
#61 MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299:3
#62 XRE_InitChildProcess(int, char**, XREChildData const*) /src/toolkit/xre/nsEmbedFunctions.cpp:705:34
#63 content_process_main(mozilla::Bootstrap*, int, char**) /src/browser/app/../../ipc/contentproc/plugin-container.cpp:63:30
#64 main /src/browser/app/nsBrowserApp.cpp:285:18
#65 __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
#66 _start (/home/user/workspace/browsers/m-c-1506533910-asan-debug/firefox+0x41eb24)
Flags: in-testsuite?
Summary: style: Assertion failure: aElement->GetComposedDoc()->GetServoRestyleRoot() [@ NoteDirtyElement] → stylo: Assertion failure: aElement->GetComposedDoc()->GetServoRestyleRoot() [@ NoteDirtyElement]
Flags: needinfo?(emilio)
(Assignee)

Comment 1

a year ago
Yup, I guess this is my department now... We set the body as a restyle root, then unbind the body, then one of those children changes state again.

There's nothing safety-wise problematic now here, thankfully, so the key here is knowing whether we want to make that assertion hold.

The proper fix for this would be to not clear the flags or the assertion from the current ClearServoData() call, and do that only and the end of UnbindSubtree...
INFO: Last good revision: 698252497ed8387d10de995262938316faaf41de
INFO: First bad revision: f44a80769c80ef7aec9d175f88dc68897928275d
INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=698252497ed8387d10de995262938316faaf41de&tochange=f44a80769c80ef7aec9d175f88dc68897928275d
Assignee: nobody → emilio
Blocks: 1402684
Has Regression Range: --- → yes
status-firefox55: --- → unaffected
status-firefox56: --- → unaffected
status-firefox57: unaffected → affected
status-firefox-esr52: --- → unaffected
Version: Trunk → 57 Branch
Priority: -- → P2
Comment hidden (mozreview-request)

Comment 4

a year ago
mozreview-review
Comment on attachment 8914209 [details]
Bug 1404134: Drop assertion that can be hit mid-unbind.

https://reviewboard.mozilla.org/r/185532/#review190468
Attachment #8914209 - Flags: review?(cam) → review+

Comment 5

a year ago
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f06d589bf31b
Drop assertion that can be hit mid-unbind. r=heycam
(Assignee)

Updated

a year ago
Flags: needinfo?(emilio)

Comment 6

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f06d589bf31b
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox58: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Did we want to land the testcase from this bug still? Also, please request Beta approval on this when you get a chance.
Flags: needinfo?(emilio)
(Assignee)

Comment 8

a year ago
Comment on attachment 8914209 [details]
Bug 1404134: Drop assertion that can be hit mid-unbind.

I didn't manage to construct a reliable test-case.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1383332
[User impact if declined]: none
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: just removes a debug-only assertion that doesn't hold during one of the edge-case situation.
[String changes made/needed]: none
Flags: needinfo?(emilio)
Attachment #8914209 - Flags: approval-mozilla-beta?
Flags: in-testsuite? → in-testsuite-
Comment on attachment 8914209 [details]
Bug 1404134: Drop assertion that can be hit mid-unbind.

Stylo related, Beta57+
Attachment #8914209 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 10

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/711cce0acdcc
status-firefox57: affected → fixed
You need to log in before you can comment on or make changes to this bug.