Closed Bug 1404134 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: DOM: Core & HTML, defect, P2)

57 Branch
defect

Tracking

()

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

People

(Reporter: tsmith, Assigned: emilio)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(2 files)

Attached file 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)
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
Version: Trunk → 57 Branch
Priority: -- → P2
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+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f06d589bf31b
Drop assertion that can be hit mid-unbind. r=heycam
Flags: needinfo?(emilio)
https://hg.mozilla.org/mozilla-central/rev/f06d589bf31b
Status: NEW → RESOLVED
Closed: 3 years ago
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)
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+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.