This one is a bit of a pain to repro but I can repro consistently. I have had the best luck launching test_case.html via launcher.html and it works best on launch. STR: 1) Put launcher.html and test_case.html side by side 2) Launch Firefox from the command line pointing at launcher.html Assertion failure: !element->HasAnyOfFlags(Element::kAllServoDescendantBits), at /src/dom/base/Element.cpp:4363 #0 AssertNoBitsPropagatedFrom(nsINode*) /src/dom/base/Element.cpp:4367:1 #1 NoteDirtyElement(mozilla::dom::Element*, unsigned int) /src/dom/base/Element.cpp:4466:3 #2 style::gecko::wrapper::GeckoElement::maybe_restyle::h070f4904531c1ee5 /src/servo/components/style/gecko/wrapper.rs:685 #3 _$LT$core..option..Option$LT$T$GT$$GT$::map_or::h83b9bcc3e66cdadd /checkout/src/libcore/option.rs:421 #4 style::gecko::wrapper::GeckoElement::note_explicit_hints::h60e7fbbabcaba3b5 /src/servo/components/style/gecko/wrapper.rs:711 #5 Servo_NoteExplicitHints /src/servo/ports/geckolib/glue.rs:3068 #6 mozilla::ServoRestyleManager::ContentStateChanged(nsIContent*, mozilla::EventStates) /src/layout/base/ServoRestyleManager.cpp:1279:5 #7 mozilla::PresShell::ContentStateChanged(nsIDocument*, nsIContent*, mozilla::EventStates) /src/layout/base/PresShell.cpp:4254:37 #8 nsDocument::ContentStateChanged(nsIContent*, mozilla::EventStates) /src/dom/base/nsDocument.cpp:5694:3 #9 mozilla::dom::Element::UpdateState(bool) /src/dom/base/Element.cpp:273:14 #10 mozilla::dom::Link::ResetLinkState(bool, bool) /src/dom/base/Link.cpp:792:15 #11 nsDocument::RefreshLinkHrefs() /src/dom/base/nsDocument.cpp:9717:23 #12 nsDocument::SetBaseURI(nsIURI*) /src/dom/base/nsDocument.cpp:3908:3 #13 mozilla::dom::SetBaseURIUsingFirstBaseWithHref(nsIDocument*, nsIContent*) /src/dom/html/HTMLSharedElement.cpp:201:14 #14 mozilla::dom::HTMLSharedElement::UnbindFromTree(bool, bool) /src/dom/html/HTMLSharedElement.cpp:294:7 #15 mozilla::dom::Element::UnbindFromTree(bool, bool) /src/dom/base/Element.cpp:2016:37 #16 nsGenericHTMLElement::UnbindFromTree(bool, bool) /src/dom/html/nsGenericHTMLElement.cpp:537:20 #17 nsINode::doRemoveChildAt(unsigned int, bool, nsIContent*, nsAttrAndChildArray&) /src/dom/base/nsINode.cpp:1945:9 #18 mozilla::dom::FragmentOrElement::RemoveChildAt(unsigned int, bool) /src/dom/base/FragmentOrElement.cpp:1363:5 #19 nsINode::RemoveChild(nsINode&, mozilla::ErrorResult&) /src/dom/base/nsINode.cpp:604:3 #20 mozilla::dom::NodeBinding::removeChild(JSContext*, JS::Handle<JSObject*>, nsINode*, JSJitMethodCallArgs const&) /src/obj-firefox/dom/bindings/NodeBinding.cpp:1012:45 #21 mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /src/dom/bindings/BindingUtils.cpp:3053:13 #22 js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /src/js/src/jscntxtinlines.h:293:15 #23 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:495:16 #24 InternalCall(JSContext*, js::AnyInvokeArgs const&) /src/js/src/vm/Interpreter.cpp:540:12 #25 Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3085:18 #26 js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:435:12 #27 js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:513:15 #28 InternalCall(JSContext*, js::AnyInvokeArgs const&) /src/js/src/vm/Interpreter.cpp:540:12 #29 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 #30 JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:2975:12 #31 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 #32 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 #33 mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*) /src/dom/events/JSEventHandler.cpp:215:12 #34 mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1112:51 #35 mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /src/dom/events/EventListenerManager.cpp:1283:20 #36 mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:313:17 #37 mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:462:16 #38 mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /src/dom/events/EventDispatcher.cpp:822:9 #39 nsDocumentViewer::LoadComplete(nsresult) /src/layout/base/nsDocumentViewer.cpp:1081:7 #40 nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) /src/docshell/base/nsDocShell.cpp:7760:21 #41 nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp:7558:7 #42 non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) /src/docshell/base/nsDocShell.cpp:7455:13 #43 nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) /src/uriloader/base/nsDocLoader.cpp:1320:3 #44 nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) /src/uriloader/base/nsDocLoader.cpp:861:14 #45 nsDocLoader::DocLoaderIsEmpty(bool) /src/uriloader/base/nsDocLoader.cpp:750:9 #46 nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /src/uriloader/base/nsDocLoader.cpp:632:5 #47 non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) /src/uriloader/base/nsDocLoader.cpp:488:14 #48 mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) /src/netwerk/base/nsLoadGroup.cpp:629:28 #49 imgRequestProxy::RemoveFromLoadGroup(bool) /src/image/imgRequestProxy.cpp:382:15 #50 imgRequestProxy::OnLoadComplete(bool) /src/image/imgRequestProxy.cpp:1004:5 #51 void mozilla::image::ImageObserverNotifier<mozilla::image::ObserverTable const*>::operator()<void mozilla::image::SyncNotifyInternal<mozilla::image::ObserverTable const*>(mozilla::image::ObserverTable const* const&, bool, unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&)::{lambda(mozilla::image::IProgressObserver*)#9}>(void mozilla::image::SyncNotifyInternal<mozilla::image::ObserverTable const*>(mozilla::image::ObserverTable const* const&, bool, unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&)::{lambda(mozilla::image::IProgressObserver*)#9}) /src/image/ProgressTracker.cpp:292:9 #52 void mozilla::image::SyncNotifyInternal<mozilla::image::ObserverTable const*>(mozilla::image::ObserverTable const* const&, bool, unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) /src/image/ProgressTracker.cpp:377:5 #53 mozilla::image::ProgressTracker::SyncNotifyProgress(unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&)::$_1::operator()(mozilla::image::ObserverTable const*) const /src/image/ProgressTracker.cpp:412:5 #54 _ZNK7mozilla5image11CopyOnWriteINS0_13ObserverTableEE4ReadIZNS0_15ProgressTracker18SyncNotifyProgressEjRKNS_3gfx12IntRectTypedINS6_12UnknownUnitsEEEE3$_1EEDTclfp_scPKS2_LDnEEET_ /src/image/CopyOnWrite.h:154:12 #55 mozilla::image::ProgressTracker::SyncNotifyProgress(unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&) /src/image/ProgressTracker.cpp:411:14 #56 mozilla::image::RasterImage::NotifyProgress(unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::Maybe<unsigned int> const&, mozilla::image::DecoderFlags, mozilla::image::SurfaceFlags) /src/image/RasterImage.cpp:1757:28 #57 mozilla::image::RasterImage::NotifyForLoadEvent(unsigned int) /src/image/RasterImage.cpp:1035:3 #58 mozilla::image::RasterImage::NotifyDecodeComplete(mozilla::image::DecoderFinalStatus const&, mozilla::image::ImageMetadata const&, mozilla::image::DecoderTelemetry const&, unsigned int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::Maybe<unsigned int> const&, mozilla::image::DecoderFlags, mozilla::image::SurfaceFlags) /src/image/RasterImage.cpp:1844:7 #59 mozilla::image::IDecodingTask::NotifyDecodeComplete(mozilla::NotNull<mozilla::image::RasterImage*>, mozilla::NotNull<mozilla::image::Decoder*>)::$_2::operator()() const /src/image/IDecodingTask.cpp:130:12 #60 mozilla::detail::RunnableFunction<mozilla::image::IDecodingTask::NotifyDecodeComplete(mozilla::NotNull<mozilla::image::RasterImage*>, mozilla::NotNull<mozilla::image::Decoder*>)::$_2>::Run() /src/xpcom/threads/nsThreadUtils.h:527:5 #61 nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1039:14 #62 NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:524:10 #63 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21 #64 MessageLoop::RunInternal() /src/ipc/chromium/src/base/message_loop.cc:326:10 #65 MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299:3 #66 nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:158:27 #67 nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:288:30 #68 XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4694:22 #69 XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4858:8 #70 XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4953:21 #71 do_main(int, char**, char**) /src/browser/app/nsBrowserApp.cpp:231:22 #72 main /src/browser/app/nsBrowserApp.cpp:304:16 #73 __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291 #74 _start (firefox+0x41eae4)

Nit: please file bugs with "stylo:" rather than "servo:" so that they end up in the right radar. NI just to ack, since I'm not sure how much of the bugmail you read on fuzz bugs. :-)

This feels a bit similar to bug 1404180, in that it's a restyle happening during element unbinding.

I can reproduce, will take a look.

This is what it looks like the test case is doing: 1. We have a document that looks like this: <html> <head> <script>...</script> </head> <body id="id4"> <base href=""> <i> <small id="id15"> <link href=""> </small> </i> <small id="id15"> <video id="a"></video> </small> </body> </html> (Yes, two <small id="id15">s.) 2. Script sets a width="" on the <video>, thus marking it as the restyle root. 3. Script sets a style="" on the first <small>, leaving dirty descendants bits on the <body>, the <i>, the second <small>, and making the <body> the new restyle root. 4. Script removes the <body>. 5. In Element::UnbindFromTree for the <body>, we clear its Servo data and also clear the document's restyle root, since the <body> is it. 6. At this intermediate point in unbinding, we have no restyle root, but some elements with dirty descendants bits set, which is unusual. 7. Next we start unbinding the <base>. We first do the generic work of Element::UnbindFromTree for it, then the base-specific stuff in HTMLSharedElement::UnbindFromTree. This ends up calling nsIDocument::SetBaseURI(nullptr), which then calls UpdateState on the <link>, which is soon to be unbound, but hasn't been yet. 8. The UpdateState causes us to call into NoteDirtyElement for the <link> (in response to taking the snapshot for the content state), and seeing as how there currently is no restyle root, we set the <link> to be it. 9. We then call Servo_NoteExplicitHints for the nsChangeHint_RepaintFrame due to the change in NS_EVENT_STATE_VISITED state bit. This calls back into NoteDirtyElement again. 10. In NoteDirtyElement again, we see that we have an existing restyle root, and decide to to the "set the bits up the tree and find the common ancestor" thing. (Even though this is unnecessary, since aElement is the current restyle root.) This is where we call AssertNoBitsPropagatedFrom(existingRoot), which fails, since the <i> has the dirty descendants bit still.

Oh, and: 7.5. The ClearServoData call on the <body> clears its dirty descendants bit. so by step 10, it's just the <i> and the second <small> with the stale bit set.

So, in the <body>'s UnbindFromTree, when we detect that we are unbinding the current restyle root, we know that we could be leaving stale bits on its descendants. We could record this fact along side the restyle root, so that if we do decide to post a restyle during the subtree's unbinding, we could check to see if it would have been under the old restyle root. If so, we can clear the bits from the new restyle root to the old one. Although this is really only for the benefit of the assertion. If we leave the stale bits, and then do another restyle, we could incorrectly determine that we're under the current restyle root. But that should only affect elements that are about to be unbound anyway.

(In reply to Cameron McCormack (:heycam) from comment #5) > 10. In NoteDirtyElement again, we see that we have an existing restyle root, > and decide to to the "set the bits up the tree and find the common ancestor" > thing. (Even though this is unnecessary, since aElement is the current > restyle root.) I was wrong about this. We do return early if aElement is the current restyle root. We just do it after the assertions.

I'm having trouble getting the test from comment 0 into a form where it triggers the assertion as a crashtest. Also, I tried making a smaller test case, which I thought should also cause the assertion, but doesn't: <!DOCTYPE html> <div id=div> <base href=""> <div> <link href=""> <span id=s1></span> </div> <span id=s2></span> </div> <script> div.offsetWidth; s1.style.color = "blue"; s2.style.color = "blue"; div.remove(); </script>

Comment on attachment 8915896 [details] Bug 1406275 - Don't assert the status of dirty bits in an unbinding subtree when choosing a new restyle root. https://reviewboard.mozilla.org/r/187154/#review192200 ::: dom/base/Element.cpp:4369 (Diff revision 2) > + // Just skip asserting the absence of bits on those element that are in > + // the unbinding subtree. (The incorrect bits will only cause us to > + // incorrectly choose a new restyle root if the newly dirty element is > + // also within the unbinding subtree, so it is OK to leave them there.) > + for (nsINode* n = aRoot; n; n = n->GetFlattenedTreeParentElementForStyle()) { > + if (!n->IsInComposedDoc()) { I'd really just return here, but if you really really think it's worth to keep the assertions up, that's fine too I guess.

(In reply to Emilio Cobos Álvarez [:emilio] from comment #13) > I'd really just return here, but if you really really think it's worth to > keep the assertions up, that's fine too I guess. I don't know about "really really", but it seems not hard to keep asserting for the elements that are important, and in case I got my comment wrong and there are indeed ways that updating restyle roots in the unbinding subtree could cause problems outside the subtree, it would be good to know.

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #2) > Nit: please file bugs with "stylo:" rather than "servo:" so that they end up > in the right radar. Doh! Sorry typo.

Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2c2fbf414099 Don't assert the status of dirty bits in an unbinding subtree when choosing a new restyle root. r=emilio

Do we want this on Beta or can it ride the 58 train? It grafts cleanly.

Probably not necessary, unless not having this will negatively impact fuzzers running on beta. Tyson?

(In reply to Cameron McCormack (:heycam) from comment #19) > Probably not necessary, unless not having this will negatively impact > fuzzers running on beta. Tyson? We can live without it. Thanks for checking.