The attached testcase causes a panic in m-c rev 20170831-fb22415719a9 with stylo enabled by pref thread '<unnamed>' panicked at 'Resolving style on <mo> (0x7f39c5505d40) without current styles: ElementData { styles: ElementStyles { primary: Some(Some(StrongRuleNode { p: 0x7f39de1b5790 })), pseudos: EagerPseudoStyles(None) }, restyle: RestyleData { hint: RESTYLE_SELF | RESTYLE_DESCENDANTS, flags: , damage: GeckoRestyleDamage(nsChangeHint(0)) } }', /builds/worker/workspace/build/src/servo/ports/geckolib/glue.rs:2900 stack backtrace: 0: 0x7f39ea93bff3 - std::sys::imp::backtrace::tracing::imp::unwind_backtrace::hcab99e0793da62c7 1: 0x7f39ea937316 - std::sys_common::backtrace::_print::hbfe5b0c7e79c0711 2: 0x7f39ea94968a - std::panicking::default_hook::{{closure}}::h9ba2c6973907a2be 3: 0x7f39ea94928b - std::panicking::default_hook::he4d55e2dd21c3cca 4: 0x7f39ea949ada - std::panicking::rust_panic_with_hook::ha138c05cd33ad44d 5: 0x7f39ea949974 - std::panicking::begin_panic::hcdbfa35c94142fa2 6: 0x7f39ea9498a9 - std::panicking::begin_panic_fmt::hc09fe500d9b7be81 7: 0x7f39ea273c67 - Servo_ResolveStyle 8: 0x7f39e87f0719 - mozilla::ServoStyleSet::ResolveServoStyle(mozilla::dom::Element*) 9: 0x7f39e8804024 - mozilla::ServoStyleSet::ResolveStyleFor(mozilla::dom::Element*, mozilla::ServoStyleContext*, mozilla::LazyComputeBehavior) 10: 0x7f39e89084ba - nsCSSFrameConstructor::ResolveStyleContext(nsStyleContext*, nsIContent*, nsFrameConstructorState*, mozilla::dom::Element*) 11: 0x7f39e8908822 - nsCSSFrameConstructor::ResolveStyleContext(nsIFrame*, nsIContent*, nsIContent*, nsFrameConstructorState*) 12: 0x7f39e890916d - nsCSSFrameConstructor::ResolveStyleContext(nsCSSFrameConstructor::InsertionPoint const&, nsIContent*, nsFrameConstructorState*) 13: 0x7f39e89260b4 - nsCSSFrameConstructor::AddFrameConstructionItems(nsFrameConstructorState&, nsIContent*, bool, nsCSSFrameConstructor::InsertionPoint const&, nsCSSFrameConstructor::FrameConstructionItemList&) 14: 0x7f39e8927ba1 - nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState&, nsIContent*, nsStyleContext*, nsContainerFrame*, bool, nsFrameItems&, bool, PendingBinding*, nsIFrame*) 15: 0x7f39e8928c5d - nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem&, nsFrameConstructorState&, nsContainerFrame*, nsFrameItems&) 16: 0x7f39e892913c - nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsContainerFrame*, nsFrameItems&) 17: 0x7f39e89292c2 - nsCSSFrameConstructor::ConstructFramesFromItemList(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList&, nsContainerFrame*, bool, nsFrameItems&) 18: 0x7f39e89278c3 - nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState&, nsIContent*, nsStyleContext*, nsContainerFrame*, bool, nsFrameItems&, bool, PendingBinding*, nsIFrame*) 19: 0x7f39e8928c5d - nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem&, nsFrameConstructorState&, nsContainerFrame*, nsFrameItems&) 20: 0x7f39e892913c - nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsContainerFrame*, nsFrameItems&) 21: 0x7f39e89292c2 - nsCSSFrameConstructor::ConstructFramesFromItemList(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList&, nsContainerFrame*, bool, nsFrameItems&) 22: 0x7f39e892a17e - nsCSSFrameConstructor::ConstructInline(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItem&, nsContainerFrame*, nsStyleDisplay const*, nsFrameItems&) 23: 0x7f39e89284c3 - nsCSSFrameConstructor::ConstructFrameFromItemInternal(nsCSSFrameConstructor::FrameConstructionItem&, nsFrameConstructorState&, nsContainerFrame*, nsFrameItems&) 24: 0x7f39e892913c - nsCSSFrameConstructor::ConstructFramesFromItem(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList::Iterator&, nsContainerFrame*, nsFrameItems&) 25: 0x7f39e89292c2 - nsCSSFrameConstructor::ConstructFramesFromItemList(nsFrameConstructorState&, nsCSSFrameConstructor::FrameConstructionItemList&, nsContainerFrame*, bool, nsFrameItems&) 26: 0x7f39e89278c3 - nsCSSFrameConstructor::ProcessChildren(nsFrameConstructorState&, nsIContent*, nsStyleContext*, nsContainerFrame*, bool, nsFrameItems&, bool, PendingBinding*, nsIFrame*) 27: 0x7f39e892dd0d - nsCSSFrameConstructor::ConstructBlock(nsFrameConstructorState&, nsIContent*, nsContainerFrame*, nsContainerFrame*, nsStyleContext*, nsContainerFrame**, nsFrameItems&, nsIFrame*, PendingBinding*) 28: 0x7f39e892e937 - nsCSSFrameConstructor::ConstructDocElementFrame(mozilla::dom::Element*, nsILayoutHistoryState*) 29: 0x7f39e892f486 - nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsIContent*, nsILayoutHistoryState*, bool, bool, TreeMatchContext*) 30: 0x7f39e893105e - nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, nsCSSFrameConstructor::InsertionKind, nsCSSFrameConstructor::RemoveFlags) 31: 0x7f39e8932158 - nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame*, nsCSSFrameConstructor::InsertionKind, nsCSSFrameConstructor::RemoveFlags) 32: 0x7f39e8930fa5 - nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, nsCSSFrameConstructor::InsertionKind, nsCSSFrameConstructor::RemoveFlags) 33: 0x7f39e892f85f - nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsIContent*, nsILayoutHistoryState*, bool, bool, TreeMatchContext*) 34: 0x7f39e893105e - nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*, nsCSSFrameConstructor::InsertionKind, nsCSSFrameConstructor::RemoveFlags) 35: 0x7f39e8933205 - nsCSSFrameConstructor::CharacterDataChanged(nsIContent*, CharacterDataChangeInfo*) 36: 0x7f39e88d8a33 - mozilla::PresShell::CharacterDataChanged(nsIDocument*, nsIContent*, CharacterDataChangeInfo*) 37: 0x7f39e7822536 - nsNodeUtils::CharacterDataChanged(nsIContent*, CharacterDataChangeInfo*) 38: 0x7f39e7819a71 - nsGenericDOMDataNode::SetTextInternal(unsigned int, unsigned int, char16_t const*, unsigned int, bool, CharacterDataChangeInfo::Details*)

This isn't unfortunately fixed by bug 1394935.

I thought initially this bug was caused by bug 1383332, but it's not. It's bug 1389743.

Bug 1389743 might unveil an underlying issue.

Though I don't quite understand about bug 1389743, replacing |aInsertionKind| in ReframeContainingBlock() [1] stops this panic, something like this. --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -13203,7 +13203,8 @@ nsCSSFrameConstructor::ReframeContaining printf(" ==> blockContent=%p\n", static_cast<void*>(blockContent)); } #endif - RecreateFramesForContent(blockContent->AsElement(), aInsertionKind, + RecreateFramesForContent(blockContent->AsElement(), + InsertionKind::Async, REMOVE_FOR_RECONSTRUCTION); return; } Emilio, does this fact help to know what's going on in the test case? Also, we don't use |aFlags| in ReframeContainingBlock(), is it intentional (it was replaced by REMOVE_FOR_RECONSTRUCTION)? If so, we can drop the argument from the function. [1] https://hg.mozilla.org/mozilla-central/rev/5c0a8f5eb8c9#l7.991

It is. I'll take a look ASAP.

In bug 1394560 comment 13, Hiro said we see this same crash signature in some Nightly crash reports like bp-885c44ec-48ac-4814-bd86-721330170904.

This is a debug assertion, so this can't crash on nightly. It's still not totally clear to me that the underlying issue is the same, but seems possible.

Boris, let me know if you have the time to review these. I think you're the most suitable one to do so given our conversation yesterday is hopefully fresh enough ;). But I can also redirect to Cam or Mats I guess.

Comment on attachment 8905500 [details] Bug 1395719: Make ReconstructDocElementHierarchy take an InsertionKind. https://reviewboard.mozilla.org/r/177318/#review182420 r=me

Comment on attachment 8905501 [details] Bug 1395719: Convert aAllowLazyFrameConstruction to an enum class. https://reviewboard.mozilla.org/r/177320/#review182422 r=me ::: layout/base/PresShell.cpp:1818 (Diff revision 1) > mFrameConstructor->BeginUpdate(); > > // Have the style sheet processor construct frame for the root > // content object down > - mFrameConstructor->ContentInserted(nullptr, root, nullptr, false); > + mFrameConstructor->ContentInserted( > + nullptr, root, nullptr, nsCSSFrameConstructor::LazyConstructionAllowed::No); This is over 80 chars. Please put the last arg on a separate line. ::: layout/base/nsCSSFrameConstructor.cpp:7471 (Diff revision 1) > > nsCSSFrameConstructor::InsertionPoint > nsCSSFrameConstructor::GetRangeInsertionPoint(nsIContent* aContainer, > nsIContent* aStartChild, > nsIContent* aEndChild, > - bool aAllowLazyConstruction, > + LazyConstructionAllowed aLazyConstructionAllowed, This might be over 80 chars too...

Comment on attachment 8905502 [details] Bug 1395719: Reconstruct ancestors asynchronously when lazy frame construction is allowed. https://reviewboard.mozilla.org/r/177322/#review182424 ::: commit-message-e245b:19 (Diff revision 1) > +to lazily construct. > + > +In that case, we haven't even performed initial styling on the element, so we > +just panic and crash. > + > +After this the only sync frame construction case remaining is You mean the only case remaining when we might not have up-to-date style info, right? ::: layout/base/nsCSSFrameConstructor.h:1943 (Diff revision 1) > // aIsAppend is true, then the caller MUST call > // nsCSSFrameConstructor::AppendFramesToParent (as opposed to > // nsFrameManager::InsertFrames directly) to add the new frames. > // @return true if we reconstructed the containing block, false > // otherwise > bool WipeContainingBlock(nsFrameConstructorState& aState, So WipeContainingBlock used to always do async reconstructs, right? And now it might end up doing sync ones in some cases? This worries me; it can have some nasty performance implications. For example, say we're reframing a bunch of kids of something that would end up doing WipeContainingBlock. Will we now end up doing multiple wipes? I'm not actually sure; I'd need to either audit code or step through in a debugger. Since this part is not actually needed for the change you're really making (which is eliminating sync cases), it would be best to split it into a separate bug, for regression-tracking purposes. You can leave all the changes in this bug, just pass "Async" at the callsites of WipeContainingBlock for now, and update to passing the "for ancestor" thing in the separate bug. ::: layout/base/nsCSSFrameConstructor.cpp:7602 (Diff revision 1) > MOZ_ASSERT(!aProvidedTreeMatchContext || > aLazyConstructionAllowed == LazyConstructionAllowed::No); > MOZ_ASSERT(aLazyConstructionAllowed == LazyConstructionAllowed::No || > !RestyleManager()->IsInStyleRefresh()); > > + const InsertionKind insertionKind = Maybe call this insertionKindForAncestorReframe? This is not the insertionKind we plan to use for ourselves, after all. ::: layout/base/nsCSSFrameConstructor.cpp:8029 (Diff revision 1) > NS_PRECONDITION(mUpdateCount != 0, > "Should be in an update while creating frames"); > > NS_PRECONDITION(aStartChild, "must always pass a child"); > > + const InsertionKind insertionKind = Again, insertionKindForAncestorReframe.

Comment on attachment 8905503 [details] Bug 1395719: Minimal cleanup when using the accessibility service. https://reviewboard.mozilla.org/r/177324/#review182430 r=me

Comment on attachment 8905504 [details] Bug 1395719: Crashtest. https://reviewboard.mozilla.org/r/177326/#review182432

Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f432a118c6e0 Make ReconstructDocElementHierarchy take an InsertionKind. r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/08951d83c938 Convert aAllowLazyFrameConstruction to an enum class. r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/45f8148d33c8 Reconstruct ancestors asynchronously when lazy frame construction is allowed. r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/4ae543bcdc2f Minimal cleanup when using the accessibility service. r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/dee58c528463 Crashtest. r=bz

https://hg.mozilla.org/mozilla-central/rev/f432a118c6e0 https://hg.mozilla.org/mozilla-central/rev/08951d83c938 https://hg.mozilla.org/mozilla-central/rev/45f8148d33c8 https://hg.mozilla.org/mozilla-central/rev/4ae543bcdc2f https://hg.mozilla.org/mozilla-central/rev/dee58c528463

Good news: this signature was the Windows #8 topcrash in Nightly 20170908100218, which was the last Nightly before these patches landed, but hasn't been seen since.