diff options
author | Matt A. Tobin <email@mattatobin.com> | 2020-04-17 06:14:10 -0400 |
---|---|---|
committer | Matt A. Tobin <email@mattatobin.com> | 2020-04-17 06:14:10 -0400 |
commit | e482e335bb8de6630a171cd0a784b420d91106b2 (patch) | |
tree | 70d27fa03ec0624a04e3418fb20ac1a434dcc3e5 | |
parent | f4a1d0123c41647f2f05aeaa2ae14bd1806fbb5c (diff) | |
download | uxp-e482e335bb8de6630a171cd0a784b420d91106b2.tar.gz |
Bug 1389743 - Only reconstruct frames synchronously from ContentRemoved when called from frame construction
Tag #1375
-rw-r--r-- | dom/base/Element.cpp | 19 | ||||
-rw-r--r-- | dom/xbl/nsXBLService.cpp | 32 | ||||
-rw-r--r-- | layout/base/RestyleManager.cpp | 6 | ||||
-rw-r--r-- | layout/base/RestyleManagerBase.cpp | 6 | ||||
-rw-r--r-- | layout/base/nsCSSFrameConstructor.cpp | 269 | ||||
-rw-r--r-- | layout/base/nsCSSFrameConstructor.h | 88 | ||||
-rw-r--r-- | layout/base/nsIPresShell.h | 18 | ||||
-rw-r--r-- | layout/base/nsPresShell.cpp | 44 | ||||
-rw-r--r-- | layout/base/nsPresShell.h | 4 |
9 files changed, 201 insertions, 285 deletions
diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index 31af0c1db4..a2b78092ad 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -1112,12 +1112,9 @@ Element::CreateShadowRoot(ErrorResult& aError) return nullptr; } - nsIDocument* doc = GetComposedDoc(); - nsIContent* destroyedFramesFor = nullptr; - if (doc) { - nsIPresShell* shell = doc->GetShell(); - if (shell) { - shell->DestroyFramesFor(this, &destroyedFramesFor); + if (nsIDocument* doc = GetComposedDoc()) { + if (nsIPresShell* shell = doc->GetShell()) { + shell->DestroyFramesFor(this); MOZ_ASSERT(!shell->FrameManager()->GetDisplayContentsStyleFor(this)); } } @@ -1160,16 +1157,6 @@ Element::CreateShadowRoot(ErrorResult& aError) SetXBLBinding(xblBinding); - // Recreate the frame for the bound content because binding a ShadowRoot - // changes how things are rendered. - if (doc) { - MOZ_ASSERT(doc == GetComposedDoc()); - nsIPresShell* shell = doc->GetShell(); - if (shell) { - shell->CreateFramesFor(destroyedFramesFor); - } - } - return shadowRoot.forget(); } diff --git a/dom/xbl/nsXBLService.cpp b/dom/xbl/nsXBLService.cpp index 3f98eefe0d..ef0d205643 100644 --- a/dom/xbl/nsXBLService.cpp +++ b/dom/xbl/nsXBLService.cpp @@ -124,38 +124,10 @@ public: // Destroy the frames for mBoundElement. Do this after getting the binding, // since if the binding fetch fails then we don't want to destroy the // frames. - nsIContent* destroyedFramesFor = nullptr; - nsIPresShell* shell = doc->GetShell(); - if (shell) { - shell->DestroyFramesFor(mBoundElement, &destroyedFramesFor); + if (nsIPresShell* shell = doc->GetShell()) { + shell->DestroyFramesFor(mBoundElement->AsElement()); } MOZ_ASSERT(!mBoundElement->GetPrimaryFrame()); - - // If |mBoundElement| is (in addition to having binding |mBinding|) - // also a descendant of another element with binding |mBinding|, - // then we might have just constructed it due to the - // notification of its parent. (We can know about both if the - // binding loads were triggered from the DOM rather than frame - // construction.) So we have to check both whether the element - // has a primary frame and whether it's in the frame manager maps - // before sending a ContentInserted notification, or bad things - // will happen. - MOZ_ASSERT(shell == doc->GetShell()); - if (shell) { - nsIFrame* childFrame = mBoundElement->GetPrimaryFrame(); - if (!childFrame) { - // Check to see if it's in the undisplayed content map... - nsFrameManager* fm = shell->FrameManager(); - nsStyleContext* sc = fm->GetUndisplayedContent(mBoundElement); - if (!sc) { - // or in the display:contents map. - sc = fm->GetDisplayContentsStyleFor(mBoundElement); - } - if (!sc) { - shell->CreateFramesFor(destroyedFramesFor); - } - } - } } nsXBLBindingRequest(nsIURI* aURI, nsIContent* aBoundElement) diff --git a/layout/base/RestyleManager.cpp b/layout/base/RestyleManager.cpp index 8db3d93020..6520fe1f6e 100644 --- a/layout/base/RestyleManager.cpp +++ b/layout/base/RestyleManager.cpp @@ -146,8 +146,10 @@ RestyleManager::RestyleElement(Element* aElement, } if (aMinHint & nsChangeHint_ReconstructFrame) { - FrameConstructor()->RecreateFramesForContent(aElement, false, - nsCSSFrameConstructor::REMOVE_FOR_RECONSTRUCTION, nullptr); + FrameConstructor()->RecreateFramesForContent( + aElement, + nsCSSFrameConstructor::InsertionKind::Sync, + nsCSSFrameConstructor::REMOVE_FOR_RECONSTRUCTION); } else if (aPrimaryFrame) { ComputeAndProcessStyleChange(aPrimaryFrame, aMinHint, aRestyleTracker, aRestyleHint, aRestyleHintData); diff --git a/layout/base/RestyleManagerBase.cpp b/layout/base/RestyleManagerBase.cpp index 6ef048a19e..1a3cae4bfe 100644 --- a/layout/base/RestyleManagerBase.cpp +++ b/layout/base/RestyleManagerBase.cpp @@ -1249,8 +1249,10 @@ if (!mDestroyedFrames) { // We could also have problems with triggering of CSS transitions // on elements whose frames are reconstructed, since we depend on // the reconstruction happening synchronously. - frameConstructor->RecreateFramesForContent(content, false, - nsCSSFrameConstructor::REMOVE_FOR_RECONSTRUCTION, nullptr); + frameConstructor->RecreateFramesForContent( + content, + nsCSSFrameConstructor::InsertionKind::Sync, + nsCSSFrameConstructor::REMOVE_FOR_RECONSTRUCTION); } else { NS_ASSERTION(frame, "This shouldn't happen"); diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index 8b89b2a223..1341f96338 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -6220,8 +6220,8 @@ nsCSSFrameConstructor::ReconstructDocElementHierarchy() /* nothing to do */ return; } - RecreateFramesForContent(rootElement, false, REMOVE_FOR_RECONSTRUCTION, - nullptr); + RecreateFramesForContent(rootElement, InsertionKind::Sync, + REMOVE_FOR_RECONSTRUCTION); } nsContainerFrame* @@ -7014,7 +7014,8 @@ nsCSSFrameConstructor::ReframeTextIfNeeded(nsIContent* aParentContent, } NS_ASSERTION(!aContent->GetPrimaryFrame(), "Text node has a frame and NS_CREATE_FRAME_IF_NON_WHITESPACE"); - ContentInserted(aParentContent, aContent, nullptr, false); + const bool allowLazyConstruction = true; + ContentInserted(aParentContent, aContent, nullptr, allowLazyConstruction); } // For inserts aChild should be valid, for appends it should be null. @@ -7297,8 +7298,9 @@ nsCSSFrameConstructor::MaybeRecreateForFrameset(nsIFrame* aParentFrame, cur = cur->GetNextSibling()) { if (IsSpecialFramesetChild(cur)) { // Just reframe the parent, since framesets are weird like that. - RecreateFramesForContent(aParentFrame->GetContent(), false, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(aParentFrame->GetContent(), + InsertionKind::Sync, + REMOVE_FOR_RECONSTRUCTION); return true; } } @@ -7363,8 +7365,8 @@ nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer, //XXXsmaug This is super unefficient! nsIContent* bindingParent = aContainer->GetBindingParent(); LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(bindingParent, false, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(bindingParent, InsertionKind::Sync, + REMOVE_FOR_RECONSTRUCTION); LAYOUT_PHASE_TEMP_REENTER(); return; } @@ -7406,8 +7408,9 @@ nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer, if (parentFrame->IsFrameOfType(nsIFrame::eMathML)) { LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(parentFrame->GetContent(), false, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(parentFrame->GetContent(), + InsertionKind::Sync, + REMOVE_FOR_RECONSTRUCTION); LAYOUT_PHASE_TEMP_REENTER(); return; } @@ -7799,8 +7802,8 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, //XXXsmaug This is super unefficient! nsIContent* bindingParent = aContainer->GetBindingParent(); LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(bindingParent, false, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(bindingParent, InsertionKind::Sync, + REMOVE_FOR_RECONSTRUCTION); LAYOUT_PHASE_TEMP_REENTER(); return; } @@ -7882,8 +7885,9 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, // and if so, proceed. But we'd have to extend nsFieldSetFrame // to locate this legend in the inserted frames and extract it. LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(insertion.mParentFrame->GetContent(), false, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(insertion.mParentFrame->GetContent(), + InsertionKind::Sync, + REMOVE_FOR_RECONSTRUCTION); LAYOUT_PHASE_TEMP_REENTER(); return; } @@ -7897,8 +7901,9 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, // expensive to recreate the entire details frame, but it's the simplest way // to handle the insertion. LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(insertion.mParentFrame->GetContent(), false, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(insertion.mParentFrame->GetContent(), + InsertionKind::Sync, + REMOVE_FOR_RECONSTRUCTION); LAYOUT_PHASE_TEMP_REENTER(); return; } @@ -7912,8 +7917,9 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, if (insertion.mParentFrame->IsFrameOfType(nsIFrame::eMathML)) { LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(insertion.mParentFrame->GetContent(), false, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(insertion.mParentFrame->GetContent(), + InsertionKind::Sync, + REMOVE_FOR_RECONSTRUCTION); LAYOUT_PHASE_TEMP_REENTER(); return; } @@ -8236,21 +8242,25 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, } void -nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, - nsIContent* aChild, - nsIContent* aOldNextSibling, - RemoveFlags aFlags, - bool* aDidReconstruct, - nsIContent** aDestroyedFramesFor) -{ +nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, + nsIContent* aChild, + nsIContent* aOldNextSibling, + RemoveFlags aFlags, + bool* aDidReconstruct) +{ + MOZ_ASSERT(aChild); + MOZ_ASSERT(aDidReconstruct); AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC); NS_PRECONDITION(mUpdateCount != 0, "Should be in an update while destroying frames"); *aDidReconstruct = false; - if (aDestroyedFramesFor) { - *aDestroyedFramesFor = aChild; - } + + // Only recreate sync if we're already in frame construction, that is, + // recreate async for XBL DOM changes, and normal content removals. + const InsertionKind insertionKind = (aFlags == REMOVE_FOR_RECONSTRUCTION) + ? InsertionKind::Sync + : InsertionKind::Async; nsPresContext* presContext = mPresShell->GetPresContext(); MOZ_ASSERT(presContext, "Our presShell should have a valid presContext"); @@ -8292,22 +8302,21 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, MOZ_ASSERT(!childFrame || !GetDisplayContentsStyleFor(aChild), "display:contents nodes shouldn't have a frame"); if (!childFrame && GetDisplayContentsStyleFor(aChild)) { - nsIContent* ancestor = aContainer; + nsIContent* ancestor = aChild->GetFlattenedTreeParent(); MOZ_ASSERT(ancestor, "display: contents on the root?"); while (!ancestor->GetPrimaryFrame()) { - // FIXME(emilio): Should this use the flattened tree parent instead? - ancestor = ancestor->GetParent(); + ancestor = ancestor->GetFlattenedTreeParent(); MOZ_ASSERT(ancestor, "we can't have a display: contents subtree root!"); } nsIFrame* ancestorFrame = ancestor->GetPrimaryFrame(); if (ancestorFrame->GetProperty(nsIFrame::GenConProperty())) { *aDidReconstruct = true; - LAYOUT_PHASE_TEMP_EXIT(); // XXXmats Can we recreate frames only for the ::after/::before content? // XXX Perhaps even only those that belong to the aChild sub-tree? - RecreateFramesForContent(ancestor, false, aFlags, aDestroyedFramesFor); + LAYOUT_PHASE_TEMP_EXIT(); + RecreateFramesForContent(ancestor, insertionKind, aFlags); LAYOUT_PHASE_TEMP_REENTER(); return; } @@ -8316,7 +8325,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, for (nsIContent* c = iter.GetNextChild(); c; c = iter.GetNextChild()) { if (c->GetPrimaryFrame() || GetDisplayContentsStyleFor(c)) { LAYOUT_PHASE_TEMP_EXIT(); - ContentRemoved(aChild, c, nullptr, aFlags, aDidReconstruct, aDestroyedFramesFor); + ContentRemoved(aChild, c, nullptr, aFlags, aDidReconstruct); LAYOUT_PHASE_TEMP_REENTER(); if (aFlags != REMOVE_DESTROY_FRAMES && *aDidReconstruct) { return; @@ -8370,7 +8379,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, nsIContent* bindingParent = aContainer->GetBindingParent(); *aDidReconstruct = true; LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(bindingParent, false, aFlags, aDestroyedFramesFor); + RecreateFramesForContent(bindingParent, insertionKind, aFlags); LAYOUT_PHASE_TEMP_REENTER(); return; } @@ -8384,14 +8393,10 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, // See whether we need to remove more than just childFrame LAYOUT_PHASE_TEMP_EXIT(); - nsIContent* container; - if (MaybeRecreateContainerForFrameRemoval(childFrame, aFlags, &container)) { - LAYOUT_PHASE_TEMP_REENTER(); - MOZ_ASSERT(container); + if (MaybeRecreateContainerForFrameRemoval( + childFrame, insertionKind, aFlags)) { *aDidReconstruct = true; - if (aDestroyedFramesFor) { - *aDestroyedFramesFor = container; - } + LAYOUT_PHASE_TEMP_REENTER(); return; } LAYOUT_PHASE_TEMP_REENTER(); @@ -8405,8 +8410,8 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, // Just reframe the parent, since framesets are weird like that. *aDidReconstruct = true; LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(parentFrame->GetContent(), false, - aFlags, aDestroyedFramesFor); + RecreateFramesForContent(parentFrame->GetContent(), insertionKind, + aFlags); LAYOUT_PHASE_TEMP_REENTER(); return; } @@ -8419,8 +8424,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, if (possibleMathMLAncestor->IsFrameOfType(nsIFrame::eMathML)) { *aDidReconstruct = true; LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(possibleMathMLAncestor->GetContent(), - false, aFlags, aDestroyedFramesFor); + RecreateFramesForContent(parentFrame->GetContent(), insertionKind, aFlags); LAYOUT_PHASE_TEMP_REENTER(); return; } @@ -8436,15 +8440,14 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, !AnyKidsNeedBlockParent(childFrame->GetNextSibling())) { *aDidReconstruct = true; LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(grandparentFrame->GetContent(), true, - aFlags, aDestroyedFramesFor); + RecreateFramesForContent(grandparentFrame->GetContent(), insertionKind, + aFlags); LAYOUT_PHASE_TEMP_REENTER(); return; } #ifdef ACCESSIBILITY - nsAccessibilityService* accService = nsIPresShell::AccService(); - if (accService) { + if (nsAccessibilityService* accService = nsIPresShell::AccService()) { accService->ContentRemoved(mPresShell, aChild); } #endif @@ -8547,6 +8550,9 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, // and hence it's adjacent to a block end. // If aOldNextSibling is null, then the text node before the node being // removed is the last node, and we don't need to worry about it. + // + // FIXME(emilio): This should probably use the lazy frame construction + // bits if possible instead of reframing it in place. if (aOldNextSibling) { nsIContent* prevSibling = aOldNextSibling->GetPreviousSibling(); if (prevSibling && prevSibling->GetPreviousSibling()) { @@ -8651,8 +8657,8 @@ nsCSSFrameConstructor::CharacterDataChanged(nsIContent* aContent, "Bit should never be set on generated content"); #endif LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(aContent, false, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(aContent, InsertionKind::Sync, + REMOVE_FOR_RECONSTRUCTION); LAYOUT_PHASE_TEMP_REENTER(); return; } @@ -9315,7 +9321,8 @@ nsCSSFrameConstructor::MaybeRecreateFramesForElement(Element* aElement) } } - RecreateFramesForContent(aElement, false, REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(aElement, InsertionKind::Sync, + REMOVE_FOR_RECONSTRUCTION); return nullptr; } @@ -9358,17 +9365,16 @@ FindPreviousNonWhitespaceSibling(nsIFrame* aFrame) } bool -nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, - RemoveFlags aFlags, - nsIContent** aDestroyedFramesFor) +nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval( + nsIFrame* aFrame, + InsertionKind aInsertionKind, + RemoveFlags aFlags) { NS_PRECONDITION(aFrame, "Must have a frame"); NS_PRECONDITION(aFrame->GetParent(), "Frame shouldn't be root"); NS_PRECONDITION(aFrame == aFrame->FirstContinuation(), "aFrame not the result of GetPrimaryFrame()?"); - *aDestroyedFramesFor = nullptr; - if (IsFramePartOfIBSplit(aFrame)) { // The removal functions can't handle removal of an {ib} split directly; we // need to rebuild the containing block. @@ -9381,17 +9387,15 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, } #endif - ReframeContainingBlock(aFrame, aFlags, aDestroyedFramesFor); + ReframeContainingBlock(aFrame, aInsertionKind, aFlags); return true; } nsContainerFrame* insertionFrame = aFrame->GetContentInsertionFrame(); if (insertionFrame && insertionFrame->GetType() == nsGkAtoms::legendFrame && aFrame->GetParent()->GetType() == nsGkAtoms::fieldSetFrame) { - // When we remove the legend for a fieldset, we should reframe - // the fieldset to ensure another legend is used, if there is one - RecreateFramesForContent(aFrame->GetParent()->GetContent(), false, - aFlags, aDestroyedFramesFor); + RecreateFramesForContent(aFrame->GetParent()->GetContent(), aInsertionKind, + aFlags); return true; } @@ -9415,9 +9419,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, // When removing a summary, we should reframe the parent details frame to // ensure that another summary is used or the default summary is // generated. - RecreateFramesForContent(parent->GetContent(), - false, REMOVE_FOR_RECONSTRUCTION, - aDestroyedFramesFor); + RecreateFramesForContent(parent->GetContent(), aInsertionKind, aFlags); return true; } } @@ -9440,10 +9442,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, // Similar if we're a table-caption. (inFlowFrame->IsTableCaption() && parent->GetChildList(nsIFrame::kCaptionList).FirstChild() == inFlowFrame)) { - // We're the first or last frame in the pseudo. Need to reframe. - // Good enough to recreate frames for |parent|'s content - RecreateFramesForContent(parent->GetContent(), true, aFlags, - aDestroyedFramesFor); + RecreateFramesForContent(parent->GetContent(), aInsertionKind, aFlags); return true; } } @@ -9472,8 +9471,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, #endif // Good enough to recreate frames for aFrame's parent's content; even if // aFrame's parent is a pseudo, that'll be the right content node. - RecreateFramesForContent(parent->GetContent(), true, aFlags, - aDestroyedFramesFor); + RecreateFramesForContent(parent->GetContent(), aInsertionKind, aFlags); return true; } } @@ -9489,8 +9487,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, // frames may be constructed or destroyed accordingly. // 2. The type of the first child of a ruby frame determines // whether a pseudo ruby base container should exist. - RecreateFramesForContent(parent->GetContent(), true, aFlags, - aDestroyedFramesFor); + RecreateFramesForContent(parent->GetContent(), aInsertionKind, aFlags); return true; } @@ -9513,8 +9510,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, } #endif // DEBUG // Recreate frames for the flex container (the removed frame's parent) - RecreateFramesForContent(parent->GetContent(), true, aFlags, - aDestroyedFramesFor); + RecreateFramesForContent(parent->GetContent(), aInsertionKind, aFlags); return true; } @@ -9532,8 +9528,8 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, } #endif // DEBUG // Recreate frames for the flex container (the removed frame's grandparent) - RecreateFramesForContent(parent->GetParent()->GetContent(), true, - aFlags, aDestroyedFramesFor); + RecreateFramesForContent(parent->GetParent()->GetContent(), aInsertionKind, + aFlags); return true; } @@ -9553,8 +9549,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, !inFlowFrame->GetNextSibling() && ((parent->GetPrevContinuation() && !parent->GetPrevInFlow()) || (parent->GetNextContinuation() && !parent->GetNextInFlow()))) { - RecreateFramesForContent(parent->GetContent(), true, aFlags, - aDestroyedFramesFor); + RecreateFramesForContent(parent->GetContent(), aInsertionKind, aFlags); return true; } @@ -9590,18 +9585,18 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, } #endif - ReframeContainingBlock(parent, aFlags, aDestroyedFramesFor); + ReframeContainingBlock(parent, aInsertionKind, aFlags); return true; } void -nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, - bool aAsyncInsert, - RemoveFlags aFlags, - nsIContent** aDestroyedFramesFor) +nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, + InsertionKind aInsertionKind, + RemoveFlags aFlags) { - NS_PRECONDITION(!aAsyncInsert || aContent->IsElement(), - "Can only insert elements async"); + MOZ_ASSERT(aInsertionKind != InsertionKind::Async || aContent->IsElement()); + MOZ_ASSERT(aContent); + // If there is no document, we don't want to recreate frames for it. (You // shouldn't generally be giving this method content without a document // anyway). @@ -9635,8 +9630,8 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, if (frame) { nsIFrame* nonGeneratedAncestor = nsLayoutUtils::GetNonGeneratedAncestor(frame); if (nonGeneratedAncestor->GetContent() != aContent) { - return RecreateFramesForContent(nonGeneratedAncestor->GetContent(), aAsyncInsert, - aFlags, aDestroyedFramesFor); + return RecreateFramesForContent(nonGeneratedAncestor->GetContent(), + aInsertionKind, aFlags); } if (frame->GetStateBits() & NS_FRAME_ANONYMOUSCONTENTCREATOR_CONTENT) { @@ -9656,8 +9651,8 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, if (ancestor->GetType() != nsGkAtoms::svgUseFrame) { NS_ASSERTION(aContent->IsInNativeAnonymousSubtree(), "Why is NS_FRAME_ANONYMOUSCONTENTCREATOR_CONTENT set?"); - return RecreateFramesForContent(ancestor->GetContent(), aAsyncInsert, - aFlags, aDestroyedFramesFor); + return RecreateFramesForContent(ancestor->GetContent(), aInsertionKind, + aFlags); } } @@ -9668,18 +9663,12 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, // with native anonymous content from the editor. if (parent && parent->IsLeaf() && parentContent && parentContent != aContent) { - return RecreateFramesForContent(parentContent, aAsyncInsert, aFlags, - aDestroyedFramesFor); + return RecreateFramesForContent(parentContent, aInsertionKind, aFlags); } } - nsIContent* container; - if (frame && MaybeRecreateContainerForFrameRemoval(frame, aFlags, - &container)) { - MOZ_ASSERT(container); - if (aDestroyedFramesFor) { - *aDestroyedFramesFor = container; - } + if (frame && + MaybeRecreateContainerForFrameRemoval(frame, aInsertionKind, aFlags)) { return; } @@ -9692,25 +9681,25 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, // Need the nsIContent parent, which might be null here, since we need to // pass it to ContentInserted and ContentRemoved. + // + // FIXME(emilio): Do we need a strong ref here? nsCOMPtr<nsIContent> container = aContent->GetParent(); // Remove the frames associated with the content object. bool didReconstruct; nsIContent* nextSibling = aContent->IsRootOfAnonymousSubtree() ? nullptr : aContent->GetNextSibling(); - const bool reconstruct = aFlags != REMOVE_DESTROY_FRAMES; - RemoveFlags flags = reconstruct ? REMOVE_FOR_RECONSTRUCTION : aFlags; - ContentRemoved(container, aContent, nextSibling, flags, - &didReconstruct, aDestroyedFramesFor); - if (reconstruct && !didReconstruct) { - // Now, recreate the frames associated with this content object. If - // ContentRemoved triggered reconstruction, then we don't need to do this - // because the frames will already have been built. - if (aAsyncInsert) { + ContentRemoved(container, aContent, nextSibling, aFlags, &didReconstruct); + if (!didReconstruct) { + if (aInsertionKind == InsertionKind::Async) { // XXXmats doesn't frame state need to be restored in this case too? - RestyleManager()->PostRestyleEvent( - aContent->AsElement(), nsRestyleHint(0), nsChangeHint_ReconstructFrame); + RestyleManager()->PostRestyleEvent(aContent->AsElement(), + nsRestyleHint(0), + nsChangeHint_ReconstructFrame); } else { + // Now, recreate the frames associated with this content object. If + // ContentRemoved triggered reconstruction, then we don't need to do this + // because the frames will already have been built. ContentInserted(container, aContent, mTempFrameTreeState, false); } } @@ -9718,16 +9707,18 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, } void -nsCSSFrameConstructor::DestroyFramesFor(nsIContent* aContent, - nsIContent** aDestroyedFramesFor) +nsCSSFrameConstructor::DestroyFramesFor(nsIContent* aContent, + bool* aDidReconstruct) { MOZ_ASSERT(aContent && aContent->GetParentNode()); - bool didReconstruct; nsIContent* nextSibling = aContent->IsRootOfAnonymousSubtree() ? nullptr : aContent->GetNextSibling(); - ContentRemoved(aContent->GetParent(), aContent, nextSibling, - REMOVE_DESTROY_FRAMES, &didReconstruct, aDestroyedFramesFor); + ContentRemoved(aContent->GetParent(), + aContent, + nextSibling, + REMOVE_DESTROY_FRAMES, + aDidReconstruct); } ////////////////////////////////////////////////////////////////////// @@ -12281,8 +12272,8 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, if (aFrame->IsXULBoxFrame() && !(aFrame->GetStateBits() & NS_STATE_BOX_WRAPS_KIDS_IN_BLOCK) && aItems.AnyItemsNeedBlockParent()) { - RecreateFramesForContent(aFrame->GetContent(), true, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(aFrame->GetContent(), InsertionKind::Async, + REMOVE_FOR_RECONSTRUCTION); return true; } @@ -12301,8 +12292,8 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, const bool isWebkitBox = IsFlexContainerForLegacyBox(aFrame, frameType); if (aPrevSibling && IsAnonymousFlexOrGridItem(aPrevSibling) && iter.item().NeedsAnonFlexOrGridItem(aState, isWebkitBox)) { - RecreateFramesForContent(aFrame->GetContent(), true, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(aFrame->GetContent(), InsertionKind::Async, + REMOVE_FOR_RECONSTRUCTION); return true; } @@ -12313,8 +12304,9 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, iter.SetToEnd(); iter.Prev(); if (iter.item().NeedsAnonFlexOrGridItem(aState, isWebkitBox)) { - RecreateFramesForContent(aFrame->GetContent(), true, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(aFrame->GetContent(), + InsertionKind::Async, + REMOVE_FOR_RECONSTRUCTION); return true; } } @@ -12344,8 +12336,9 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, isWebkitBox)) { // We hit something that _doesn't_ need an anonymous flex item! // Rebuild the flex container to bust it out. - RecreateFramesForContent(containerFrame->GetContent(), true, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(containerFrame->GetContent(), + InsertionKind::Async, + REMOVE_FOR_RECONSTRUCTION); return true; } @@ -12369,8 +12362,8 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, // We want to optimize it better, and avoid reframing as much as // possible. But given the cases above, and the fact that a ruby // usually won't be very large, it should be fine to reframe it. - RecreateFramesForContent(aFrame->GetContent(), true, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(aFrame->GetContent(), InsertionKind::Async, + REMOVE_FOR_RECONSTRUCTION); return true; } @@ -12536,8 +12529,8 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, if (!aItems.AllWantParentType(parentType)) { // Reframing aFrame->GetContent() is good enough, since the content of // table pseudo-frames is the ancestor content. - RecreateFramesForContent(aFrame->GetContent(), true, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(aFrame->GetContent(), InsertionKind::Async, + REMOVE_FOR_RECONSTRUCTION); return true; } } @@ -12625,15 +12618,15 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, static_cast<void*>(blockContent)); } #endif - RecreateFramesForContent(blockContent, true, REMOVE_FOR_RECONSTRUCTION, - nullptr); + RecreateFramesForContent(blockContent, InsertionKind::Async, + REMOVE_FOR_RECONSTRUCTION); return true; } void -nsCSSFrameConstructor::ReframeContainingBlock(nsIFrame* aFrame, - RemoveFlags aFlags, - nsIContent** aDestroyedFramesFor) +nsCSSFrameConstructor::ReframeContainingBlock(nsIFrame* aFrame, + InsertionKind aInsertionKind, + RemoveFlags aFlags) { #ifdef DEBUG @@ -12669,20 +12662,22 @@ nsCSSFrameConstructor::ReframeContainingBlock(nsIFrame* aFrame, // so GetFloatContainingBlock(aFrame) has been removed // And get the containingBlock's content - nsCOMPtr<nsIContent> blockContent = containingBlock->GetContent(); - if (blockContent) { + if (nsIContent* blockContent = containingBlock->GetContent()) { #ifdef DEBUG if (gNoisyContentUpdates) { printf(" ==> blockContent=%p\n", static_cast<void*>(blockContent)); } #endif - return RecreateFramesForContent(blockContent, true, aFlags, aDestroyedFramesFor); + RecreateFramesForContent(blockContent->AsElement(), aInsertionKind, + REMOVE_FOR_RECONSTRUCTION); + return; } } // If we get here, we're screwed! - return RecreateFramesForContent(mPresShell->GetDocument()->GetRootElement(), - true, aFlags, nullptr); + RecreateFramesForContent(mPresShell->GetDocument()->GetRootElement(), + aInsertionKind, + REMOVE_FOR_RECONSTRUCTION); } void diff --git a/layout/base/nsCSSFrameConstructor.h b/layout/base/nsCSSFrameConstructor.h index 655519e0f6..49107ee522 100644 --- a/layout/base/nsCSSFrameConstructor.h +++ b/layout/base/nsCSSFrameConstructor.h @@ -97,8 +97,8 @@ private: // aChild is the child being inserted for inserts, and the first // child being appended for appends. bool MaybeConstructLazily(Operation aOperation, - nsIContent* aContainer, - nsIContent* aChild); + nsIContent* aContainer, + nsIContent* aChild); // Issues a single ContentInserted for each child of aContainer in the range // [aStartChild, aEndChild). @@ -152,8 +152,8 @@ private: // Returns true if parent was recreated due to frameset child, false otherwise. bool MaybeRecreateForFrameset(nsIFrame* aParentFrame, - nsIContent* aStartChild, - nsIContent* aEndChild); + nsIContent* aStartChild, + nsIContent* aEndChild); public: /** @@ -225,6 +225,11 @@ public: nsILayoutHistoryState* aFrameState, bool aAllowLazyConstruction); +public: + // FIXME(emilio): How important is it to keep the frame tree state around for + // REMOVE_DESTROY_FRAMES? + // + // Seems like otherwise we could just remove it. enum RemoveFlags { REMOVE_CONTENT, REMOVE_FOR_RECONSTRUCTION, REMOVE_DESTROY_FRAMES }; /** @@ -243,12 +248,11 @@ public: * only when aFlags == REMOVE_DESTROY_FRAMES, otherwise it will only be * captured if we reconstructed frames for an ancestor. */ - void ContentRemoved(nsIContent* aContainer, - nsIContent* aChild, - nsIContent* aOldNextSibling, - RemoveFlags aFlags, - bool* aDidReconstruct, - nsIContent** aDestroyedFramesFor = nullptr); + void ContentRemoved(nsIContent* aContainer, + nsIContent* aChild, + nsIContent* aOldNextSibling, + RemoveFlags aFlags, + bool* aDidReconstruct); void CharacterDataChanged(nsIContent* aContent, CharacterDataChangeInfo* aInfo); @@ -280,14 +284,10 @@ public: /** * Destroy the frames for aContent. Note that this may destroy frames - * for an ancestor instead - aDestroyedFramesFor contains the content node - * where frames were actually destroyed (which should be used in the - * ContentInserted call to recreate frames). The frame tree state - * is captured before the frames are destroyed and can be retrieved using - * GetLastCapturedLayoutHistoryState(). + * for an ancestor instead - aDidReconstruct contains whether a reconstruct + * was posted for any ancestor. */ - void DestroyFramesFor(nsIContent* aContent, - nsIContent** aDestroyedFramesFor); + void DestroyFramesFor(nsIContent* aContent, bool* aDidReconstruct); // Request to create a continuing frame. This method never returns null. nsIFrame* CreateContinuingFrame(nsPresContext* aPresContext, @@ -322,15 +322,6 @@ public: nsContainerFrame* GetDocElementContainingBlock() { return mDocElementContainingBlock; } - /** - * Return the layout history state that was captured in the last - * ContentRemoved / RecreateFramesForContent call. - */ - nsILayoutHistoryState* GetLastCapturedLayoutHistoryState() - { - return mTempFrameTreeState; - } - private: struct FrameConstructionItem; class FrameConstructionItemList; @@ -1713,33 +1704,36 @@ private: nsStyleContext* MaybeRecreateFramesForElement(Element* aElement); /** + * Whether insertion should be done synchronously or asynchronously. + * + * Generally, insertion is synchronous if we're reconstructing something from + * frame construction/reconstruction, and async if we're removing stuff, like + * from ContentRemoved. + */ + enum class InsertionKind + { + Sync, + Async, + }; + + /** * Recreate frames for aContent. * @param aContent the content to recreate frames for - * @param aAsyncInsert if true then a restyle event will be posted to handle - * the required ContentInserted call instead of doing it immediately. * @param aFlags normally you want to pass REMOVE_FOR_RECONSTRUCTION here - * @param aDestroyedFramesFor if non-null, it will contain the content that - * was actually reframed - it may be different than aContent. */ - void - RecreateFramesForContent(nsIContent* aContent, - bool aAsyncInsert, - RemoveFlags aFlags, - nsIContent** aDestroyedFramesFor); + void RecreateFramesForContent(nsIContent* aContent, + InsertionKind aInsertionKind, + RemoveFlags aFlags); // If removal of aFrame from the frame tree requires reconstruction of some // containing block (either of aFrame or of its parent) due to {ib} splits or // table pseudo-frames, recreate the relevant frame subtree. The return value - // indicates whether this happened. If this method returns true, *aResult is - // the return value of ReframeContainingBlock or RecreateFramesForContent. If - // this method returns false, the value of *aResult is not affected. aFrame - // and aResult must not be null. aFrame must be the result of a + // indicates whether this happened. aFrame must be the result of a // GetPrimaryFrame() call on a content node (which means its parent is also - // not null). If this method returns true, aDestroyedFramesFor contains the - // content that was reframed. - bool MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, - RemoveFlags aFlags, - nsIContent** aDestroyedFramesFor); + // not null). + bool MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, + InsertionKind aInsertionKind, + RemoveFlags aFlags); nsIFrame* CreateContinuingOuterTableFrame(nsIPresShell* aPresShell, nsPresContext* aPresContext, @@ -1863,9 +1857,9 @@ private: bool aIsAppend, nsIFrame* aPrevSibling); - void ReframeContainingBlock(nsIFrame* aFrame, - RemoveFlags aFlags, - nsIContent** aReframeContent); + void ReframeContainingBlock(nsIFrame* aFrame, + InsertionKind aInsertionKind, + RemoveFlags aFlags); //---------------------------------------- diff --git a/layout/base/nsIPresShell.h b/layout/base/nsIPresShell.h index 0a5f9e6e73..2df75489f8 100644 --- a/layout/base/nsIPresShell.h +++ b/layout/base/nsIPresShell.h @@ -539,20 +539,12 @@ public: virtual void NotifyCounterStylesAreDirty() = 0; /** - * Destroy the frames for aContent. Note that this may destroy frames - * for an ancestor instead - aDestroyedFramesFor contains the content node - * where frames were actually destroyed (which should be used in the - * CreateFramesFor call). The frame tree state will be captured before - * the frames are destroyed in the frame constructor. - */ - virtual void DestroyFramesFor(nsIContent* aContent, - nsIContent** aDestroyedFramesFor) = 0; - /** - * Create new frames for aContent. It will use the last captured layout - * history state captured in the frame constructor to restore the state - * in the new frame tree. + * Destroy the frames for aElement, and reconstruct them asynchronously if + * needed. + * + * Note that this may destroy frames for an ancestor instead. */ - virtual void CreateFramesFor(nsIContent* aContent) = 0; + virtual void DestroyFramesFor(mozilla::dom::Element* aElement) = 0; /** * Recreates the frames for a node diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp index 0839f6bb7a..f861e7b06e 100644 --- a/layout/base/nsPresShell.cpp +++ b/layout/base/nsPresShell.cpp @@ -2828,10 +2828,9 @@ PresShell::CancelAllPendingReflows() } void -PresShell::DestroyFramesFor(nsIContent* aContent, - nsIContent** aDestroyedFramesFor) +PresShell::DestroyFramesFor(Element* aElement) { - MOZ_ASSERT(aContent); + MOZ_ASSERT(aElement); NS_ENSURE_TRUE_VOID(mPresContext); if (!mDidInitialize) { return; @@ -2843,43 +2842,18 @@ PresShell::DestroyFramesFor(nsIContent* aContent, ++mChangeNestCount; nsCSSFrameConstructor* fc = FrameConstructor(); + bool didReconstruct; fc->BeginUpdate(); - fc->DestroyFramesFor(aContent, aDestroyedFramesFor); + fc->DestroyFramesFor(aElement, &didReconstruct); fc->EndUpdate(); - --mChangeNestCount; -} - -void -PresShell::CreateFramesFor(nsIContent* aContent) -{ - NS_ENSURE_TRUE_VOID(mPresContext); - if (!mDidInitialize) { - // Nothing to do here. In fact, if we proceed and aContent is the - // root we will crash. - return; + // XXXmats doesn't frame state need to be restored in this case? + if (!didReconstruct) { + PostRecreateFramesFor(aElement); } - // Don't call RecreateFramesForContent since that is not exported and we want - // to keep the number of entrypoints down. - - NS_ASSERTION(mViewManager, "Should have view manager"); - MOZ_ASSERT(aContent); - - // Have to make sure that the content notifications are flushed before we - // start messing with the frame model; otherwise we can get content doubling. - mDocument->FlushPendingNotifications(Flush_ContentAndNotify); - - nsAutoScriptBlocker scriptBlocker; - - // Mark ourselves as not safe to flush while we're doing frame construction. - ++mChangeNestCount; - - nsCSSFrameConstructor* fc = FrameConstructor(); - nsILayoutHistoryState* layoutState = fc->GetLastCapturedLayoutHistoryState(); - fc->BeginUpdate(); - fc->ContentInserted(aContent->GetParent(), aContent, layoutState, false); - fc->EndUpdate(); + mPresContext->RestyleManager()->PostRestyleEvent( + aElement, eRestyle_Subtree, nsChangeHint(0)); --mChangeNestCount; } diff --git a/layout/base/nsPresShell.h b/layout/base/nsPresShell.h index c25d07e662..a0d2ea7530 100644 --- a/layout/base/nsPresShell.h +++ b/layout/base/nsPresShell.h @@ -139,9 +139,7 @@ public: virtual bool IsSafeToFlush() const override; virtual void FlushPendingNotifications(mozFlushType aType) override; virtual void FlushPendingNotifications(mozilla::ChangesToFlush aType) override; - virtual void DestroyFramesFor(nsIContent* aContent, - nsIContent** aDestroyedFramesFor) override; - virtual void CreateFramesFor(nsIContent* aContent) override; + virtual void DestroyFramesFor(mozilla::dom::Element* aElement) override; /** * Recreates the frames for a node |