From 9f6cb6874e537fd4f451e507b1832b94b04d9d97 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 17 Apr 2020 05:04:10 -0400 Subject: Bug 1296516 - Cleanup a bit of code in layout/base * Tidy RestyleManager::ContentStateChanged * Convert UndisplayedMap to a typed hashtable * Cleanup infallible or unchecked nsCSSFrameConstructor methods Tag #1375 --- layout/base/RestyleManager.cpp | 9 +- layout/base/RestyleManager.h | 4 +- layout/base/RestyleManagerHandle.h | 4 +- layout/base/RestyleManagerHandleInlines.h | 2 +- layout/base/ServoRestyleManager.cpp | 5 +- layout/base/ServoRestyleManager.h | 3 +- layout/base/nsCSSFrameConstructor.cpp | 286 +++++++++++-------------- layout/base/nsCSSFrameConstructor.h | 125 ++++++----- layout/base/nsFrameManager.cpp | 336 +++++++++++++----------------- layout/base/nsFrameManager.h | 27 +-- layout/base/nsIPresShell.h | 2 +- layout/base/nsPresShell.cpp | 12 +- layout/base/nsPresShell.h | 2 +- 13 files changed, 358 insertions(+), 459 deletions(-) (limited to 'layout/base') diff --git a/layout/base/RestyleManager.cpp b/layout/base/RestyleManager.cpp index 16db4c18b..4d08dee06 100644 --- a/layout/base/RestyleManager.cpp +++ b/layout/base/RestyleManager.cpp @@ -246,14 +246,14 @@ ElementForStyleContext(nsIContent* aParentContent, // Forwarded nsIDocumentObserver method, to handle restyling (and // passing the notification to the frame). -nsresult +void RestyleManager::ContentStateChanged(nsIContent* aContent, EventStates aStateMask) { // XXXbz it would be good if this function only took Elements, but // we'd have to make ESM guarantee that usefully. if (!aContent->IsElement()) { - return NS_OK; + return; } Element* aElement = aContent->AsElement(); @@ -263,7 +263,6 @@ RestyleManager::ContentStateChanged(nsIContent* aContent, ContentStateChangedInternal(aElement, aStateMask, &changeHint, &restyleHint); PostRestyleEvent(aElement, restyleHint, changeHint); - return NS_OK; } // Forwarded nsIMutationObserver method, to handle restyling. @@ -1830,7 +1829,7 @@ ElementRestyler::ConditionallyRestyleUndisplayedNodes( } for (UndisplayedNode* undisplayed = aUndisplayed; undisplayed; - undisplayed = undisplayed->mNext) { + undisplayed = undisplayed->getNext()) { if (!undisplayed->mContent->IsElement()) { continue; @@ -3471,7 +3470,7 @@ ElementRestyler::RestyleUndisplayedNodes(nsRestyleHint aChildRestyleHint, if (undisplayed) { pusher.PushAncestorAndStyleScope(undisplayedParent); } - for (; undisplayed; undisplayed = undisplayed->mNext) { + for (; undisplayed; undisplayed = undisplayed->getNext()) { NS_ASSERTION(undisplayedParent || undisplayed->mContent == mPresContext->Document()->GetRootElement(), diff --git a/layout/base/RestyleManager.h b/layout/base/RestyleManager.h index e22fe9058..3b60b331a 100644 --- a/layout/base/RestyleManager.h +++ b/layout/base/RestyleManager.h @@ -59,8 +59,8 @@ public: // Forwarded nsIDocumentObserver method, to handle restyling (and // passing the notification to the frame). - nsresult ContentStateChanged(nsIContent* aContent, - EventStates aStateMask); + void ContentStateChanged(nsIContent* aContent, + EventStates aStateMask); // Forwarded nsIMutationObserver method, to handle restyling. void AttributeWillChange(Element* aElement, diff --git a/layout/base/RestyleManagerHandle.h b/layout/base/RestyleManagerHandle.h index 8be04d12c..bee1fc6d9 100644 --- a/layout/base/RestyleManagerHandle.h +++ b/layout/base/RestyleManagerHandle.h @@ -125,8 +125,8 @@ public: nsIContent* aChild); inline void RestyleForAppend(nsIContent* aContainer, nsIContent* aFirstNewContent); - inline nsresult ContentStateChanged(nsIContent* aContent, - EventStates aStateMask); + inline void ContentStateChanged(nsIContent* aContent, + EventStates aStateMask); inline void AttributeWillChange(dom::Element* aElement, int32_t aNameSpaceID, nsIAtom* aAttribute, diff --git a/layout/base/RestyleManagerHandleInlines.h b/layout/base/RestyleManagerHandleInlines.h index cc374edd5..8d6ca9142 100644 --- a/layout/base/RestyleManagerHandleInlines.h +++ b/layout/base/RestyleManagerHandleInlines.h @@ -122,7 +122,7 @@ RestyleManagerHandle::Ptr::RestyleForAppend(nsIContent* aContainer, FORWARD(RestyleForAppend, (aContainer, aFirstNewContent)); } -nsresult +void RestyleManagerHandle::Ptr::ContentStateChanged(nsIContent* aContent, EventStates aStateMask) { diff --git a/layout/base/ServoRestyleManager.cpp b/layout/base/ServoRestyleManager.cpp index 42ca23bb1..9624c678b 100644 --- a/layout/base/ServoRestyleManager.cpp +++ b/layout/base/ServoRestyleManager.cpp @@ -513,12 +513,12 @@ ServoRestyleManager::ContentRemoved(nsINode* aContainer, NS_WARNING("stylo: ServoRestyleManager::ContentRemoved not implemented"); } -nsresult +void ServoRestyleManager::ContentStateChanged(nsIContent* aContent, EventStates aChangedBits) { if (!aContent->IsElement()) { - return NS_OK; + return; } Element* aElement = aContent->AsElement(); @@ -552,7 +552,6 @@ ServoRestyleManager::ContentStateChanged(nsIContent* aContent, snapshot->AddState(previousState); PostRestyleEvent(aElement, restyleHint, changeHint); - return NS_OK; } void diff --git a/layout/base/ServoRestyleManager.h b/layout/base/ServoRestyleManager.h index 6856171c1..b6bb63d08 100644 --- a/layout/base/ServoRestyleManager.h +++ b/layout/base/ServoRestyleManager.h @@ -63,8 +63,7 @@ public: nsIContent* aChild); void RestyleForAppend(nsIContent* aContainer, nsIContent* aFirstNewContent); - nsresult ContentStateChanged(nsIContent* aContent, - EventStates aStateMask); + void ContentStateChanged(nsIContent* aContent, EventStates aStateMask); void AttributeWillChange(dom::Element* aElement, int32_t aNameSpaceID, nsIAtom* aAttribute, diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index b574e7690..b9778d36d 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -3189,7 +3189,7 @@ nsCSSFrameConstructor::ConstructSelectFrame(nsFrameConstructorState& aState, * But the select tag should really be fixed to use GFX scrollbars that can * be create with BuildScrollFrame. */ -nsresult +void nsCSSFrameConstructor::InitializeSelectFrame(nsFrameConstructorState& aState, nsContainerFrame* scrollFrame, nsContainerFrame* scrolledFrame, @@ -3235,7 +3235,6 @@ nsCSSFrameConstructor::InitializeSelectFrame(nsFrameConstructorState& aState, // Set the scrolled frame's initial child lists scrolledFrame->SetInitialChildList(kPrincipalList, childItems); - return NS_OK; } nsIFrame* @@ -6210,16 +6209,16 @@ IsRootBoxFrame(nsIFrame *aFrame) return (aFrame->GetType() == nsGkAtoms::rootFrame); } -nsresult +void nsCSSFrameConstructor::ReconstructDocElementHierarchy() { Element* rootElement = mDocument->GetRootElement(); if (!rootElement) { /* nothing to do */ - return NS_OK; + return; } - return RecreateFramesForContent(rootElement, false, REMOVE_FOR_RECONSTRUCTION, - nullptr); + RecreateFramesForContent(rootElement, false, REMOVE_FOR_RECONSTRUCTION, + nullptr); } nsContainerFrame* @@ -6469,7 +6468,7 @@ GetInsertNextSibling(nsIFrame* aParentFrame, nsIFrame* aPrevSibling) * appending flowed frames to a parent's principal child list. It handles the * case where the parent is the trailing inline of an {ib} split. */ -nsresult +void nsCSSFrameConstructor::AppendFramesToParent(nsFrameConstructorState& aState, nsContainerFrame* aParentFrame, nsFrameItems& aFrameList, @@ -6554,13 +6553,11 @@ nsCSSFrameConstructor::AppendFramesToParent(nsFrameConstructorState& aStat return AppendFramesToParent(aState, aParentFrame->GetParent(), ibSiblings, aParentFrame, true); } - - return NS_OK; + return; } // Insert the frames after our aPrevSibling InsertFrames(aParentFrame, kPrincipalList, aPrevSibling, aFrameList); - return NS_OK; } #define UNSET_DISPLAY static_cast(255) @@ -7308,7 +7305,7 @@ nsCSSFrameConstructor::MaybeRecreateForFrameset(nsIFrame* aParentFrame, return false; } -nsresult +void nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer, nsIContent* aFirstNewContent, bool aAllowLazyConstruction) @@ -7351,8 +7348,8 @@ nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer, if (tag == nsGkAtoms::treechildren || tag == nsGkAtoms::treeitem || tag == nsGkAtoms::treerow) - return NS_OK; + return; } #endif // MOZ_XUL @@ -7365,21 +7362,21 @@ nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer, //XXXsmaug This is super unefficient! nsIContent* bindingParent = aContainer->GetBindingParent(); LAYOUT_PHASE_TEMP_EXIT(); - nsresult rv = RecreateFramesForContent(bindingParent, false, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(bindingParent, false, + REMOVE_FOR_RECONSTRUCTION, nullptr); LAYOUT_PHASE_TEMP_REENTER(); - return rv; + return; } // See comment in ContentRangeInserted for why this is necessary. if (!GetContentInsertionFrameFor(aContainer) && !aContainer->IsActiveChildrenElement()) { - return NS_OK; + return; } if (aAllowLazyConstruction && MaybeConstructLazily(CONTENTAPPEND, aContainer, aFirstNewContent)) { - return NS_OK; + return; } LAYOUT_PHASE_TEMP_EXIT(); @@ -7389,13 +7386,13 @@ nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer, nsContainerFrame*& parentFrame = insertion.mParentFrame; LAYOUT_PHASE_TEMP_REENTER(); if (!parentFrame) { - return NS_OK; + return; } LAYOUT_PHASE_TEMP_EXIT(); if (MaybeRecreateForFrameset(parentFrame, aFirstNewContent, nullptr)) { LAYOUT_PHASE_TEMP_REENTER(); - return NS_OK; + return; } LAYOUT_PHASE_TEMP_REENTER(); @@ -7403,15 +7400,15 @@ nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer, // Nothing to do here; we shouldn't be constructing kids of leaves // Clear lazy bits so we don't try to construct again. ClearLazyBits(aFirstNewContent, nullptr); - return NS_OK; + return; } if (parentFrame->IsFrameOfType(nsIFrame::eMathML)) { LAYOUT_PHASE_TEMP_EXIT(); - nsresult rv = RecreateFramesForContent(parentFrame->GetContent(), false, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(parentFrame->GetContent(), false, + REMOVE_FOR_RECONSTRUCTION, nullptr); LAYOUT_PHASE_TEMP_REENTER(); - return rv; + return; } // If the frame we are manipulating is a ib-split frame (that is, one @@ -7509,7 +7506,7 @@ nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer, if (WipeContainingBlock(state, containingBlock, parentFrame, items, true, prevSibling)) { LAYOUT_PHASE_TEMP_REENTER(); - return NS_OK; + return; } LAYOUT_PHASE_TEMP_REENTER(); @@ -7596,7 +7593,7 @@ nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer, } #endif - return NS_OK; + return; } #ifdef MOZ_XUL @@ -7639,17 +7636,17 @@ bool NotifyListBoxBody(nsPresContext* aPresContext, } #endif // MOZ_XUL -nsresult +void nsCSSFrameConstructor::ContentInserted(nsIContent* aContainer, nsIContent* aChild, nsILayoutHistoryState* aFrameState, bool aAllowLazyConstruction) { - return ContentRangeInserted(aContainer, - aChild, - aChild->GetNextSibling(), - aFrameState, - aAllowLazyConstruction); + ContentRangeInserted(aContainer, + aChild, + aChild->GetNextSibling(), + aFrameState, + aAllowLazyConstruction); } // ContentRangeInserted handles creating frames for a range of nodes that @@ -7670,7 +7667,7 @@ nsCSSFrameConstructor::ContentInserted(nsIContent* aContainer, // in the caption list, while skipping any nodes in the range being inserted // (because when we treat the caption frames the other nodes have had their // frames constructed but not yet inserted into the frame tree). -nsresult +void nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, nsIContent* aStartChild, nsIContent* aEndChild, @@ -7727,7 +7724,7 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, // The insert case in NotifyListBoxBody // doesn't use "old next sibling". aStartChild, nullptr, nullptr, CONTENT_INSERTED)) { - return NS_OK; + return; } } else { // We don't handle a range insert to a listbox parent, issue single @@ -7736,7 +7733,7 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, IssueSingleInsertNofications(aContainer, aStartChild, aEndChild, aAllowLazyConstruction); LAYOUT_PHASE_TEMP_REENTER(); - return NS_OK; + return; } } #endif // MOZ_XUL @@ -7751,7 +7748,7 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, if (aStartChild != docElement) { // Not the root element; just bail out - return NS_OK; + return; } NS_PRECONDITION(nullptr == mRootElementFrame, @@ -7788,7 +7785,7 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, } #endif - return NS_OK; + return; } if (aContainer->HasFlag(NODE_IS_IN_SHADOW_TREE) && @@ -7801,10 +7798,10 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, //XXXsmaug This is super unefficient! nsIContent* bindingParent = aContainer->GetBindingParent(); LAYOUT_PHASE_TEMP_EXIT(); - nsresult rv = RecreateFramesForContent(bindingParent, false, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(bindingParent, false, + REMOVE_FOR_RECONSTRUCTION, nullptr); LAYOUT_PHASE_TEMP_REENTER(); - return rv; + return; } // Put 'parentFrame' inside a scope so we don't confuse it with @@ -7815,7 +7812,7 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, // a parent. While its uncommon to change the structure of the default content itself, a label, // for example, can be reframed by having its value attribute set or removed. if (!parentFrame && !aContainer->IsActiveChildrenElement()) { - return NS_OK; + return; } // Otherwise, we've got parent content. Find its frame. @@ -7824,7 +7821,7 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, if (aAllowLazyConstruction && MaybeConstructLazily(CONTENTINSERT, aContainer, aStartChild)) { - return NS_OK; + return; } } @@ -7844,7 +7841,7 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, } if (!insertion.mParentFrame) { - return NS_OK; + return; } bool isAppend, isRangeInsertSafe; @@ -7858,7 +7855,7 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, IssueSingleInsertNofications(aContainer, aStartChild, aEndChild, aAllowLazyConstruction); LAYOUT_PHASE_TEMP_REENTER(); - return NS_OK; + return; } nsIContent* container = insertion.mParentFrame->GetContent(); @@ -7867,7 +7864,7 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, LAYOUT_PHASE_TEMP_EXIT(); if (MaybeRecreateForFrameset(insertion.mParentFrame, aStartChild, aEndChild)) { LAYOUT_PHASE_TEMP_REENTER(); - return NS_OK; + return; } LAYOUT_PHASE_TEMP_REENTER(); @@ -7884,10 +7881,10 @@ 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(); - nsresult rv = RecreateFramesForContent(insertion.mParentFrame->GetContent(), false, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(insertion.mParentFrame->GetContent(), false, + REMOVE_FOR_RECONSTRUCTION, nullptr); LAYOUT_PHASE_TEMP_REENTER(); - return rv; + return; } // We should only get here with details when doing a single insertion because @@ -7899,26 +7896,25 @@ 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(); - nsresult rv = - RecreateFramesForContent(insertion.mParentFrame->GetContent(), false, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(insertion.mParentFrame->GetContent(), false, + REMOVE_FOR_RECONSTRUCTION, nullptr); LAYOUT_PHASE_TEMP_REENTER(); - return rv; + return; } // Don't construct kids of leaves if (insertion.mParentFrame->IsLeaf()) { // Clear lazy bits so we don't try to construct again. ClearLazyBits(aStartChild, aEndChild); - return NS_OK; + return; } if (insertion.mParentFrame->IsFrameOfType(nsIFrame::eMathML)) { LAYOUT_PHASE_TEMP_EXIT(); - nsresult rv = RecreateFramesForContent(insertion.mParentFrame->GetContent(), false, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(insertion.mParentFrame->GetContent(), false, + REMOVE_FOR_RECONSTRUCTION, nullptr); LAYOUT_PHASE_TEMP_REENTER(); - return rv; + return; } nsFrameConstructorState state(mPresShell, @@ -7995,7 +7991,7 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, IssueSingleInsertNofications(aContainer, aStartChild, aEndChild, aAllowLazyConstruction); LAYOUT_PHASE_TEMP_REENTER(); - return NS_OK; + return; } container = insertion.mParentFrame->GetContent(); @@ -8063,7 +8059,7 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, if (WipeContainingBlock(state, containingBlock, insertion.mParentFrame, items, isAppend, prevSibling)) { LAYOUT_PHASE_TEMP_REENTER(); - return NS_OK; + return; } LAYOUT_PHASE_TEMP_REENTER(); @@ -8235,10 +8231,10 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, } #endif - return NS_OK; + return; } -nsresult +void nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, nsIContent* aChild, nsIContent* aOldNextSibling, @@ -8286,7 +8282,6 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, } #endif - nsresult rv = NS_OK; nsIFrame* childFrame = aChild->GetPrimaryFrame(); if (!childFrame || childFrame->GetContent() != aChild) { // XXXbz the GetContent() != aChild check is needed due to bug 135040. @@ -8314,7 +8309,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, // XXX Perhaps even only those that belong to the aChild sub-tree? RecreateFramesForContent(ancestor, false, aFlags, aDestroyedFramesFor); LAYOUT_PHASE_TEMP_REENTER(); - return NS_OK; + return; } } @@ -8322,11 +8317,10 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, for (nsIContent* c = iter.GetNextChild(); c; c = iter.GetNextChild()) { if (c->GetPrimaryFrame() || GetDisplayContentsStyleFor(c)) { LAYOUT_PHASE_TEMP_EXIT(); - rv = ContentRemoved(aChild, c, nullptr, aFlags, aDidReconstruct, aDestroyedFramesFor); + ContentRemoved(aChild, c, nullptr, aFlags, aDidReconstruct, aDestroyedFramesFor); LAYOUT_PHASE_TEMP_REENTER(); - NS_ENSURE_SUCCESS(rv, rv); if (aFlags != REMOVE_DESTROY_FRAMES && *aDidReconstruct) { - return rv; + return; } } } @@ -8339,7 +8333,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, if (aFlags == REMOVE_DESTROY_FRAMES) { CaptureStateForFramesOf(aChild, mTempFrameTreeState); } - return NS_OK; + return; } #endif // MOZ_XUL @@ -8377,10 +8371,9 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, nsIContent* bindingParent = aContainer->GetBindingParent(); *aDidReconstruct = true; LAYOUT_PHASE_TEMP_EXIT(); - nsresult rv = RecreateFramesForContent(bindingParent, false, - aFlags, aDestroyedFramesFor); + RecreateFramesForContent(bindingParent, false, aFlags, aDestroyedFramesFor); LAYOUT_PHASE_TEMP_REENTER(); - return rv; + return; } if (aFlags == REMOVE_DESTROY_FRAMES) { @@ -8393,14 +8386,14 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, // See whether we need to remove more than just childFrame LAYOUT_PHASE_TEMP_EXIT(); nsIContent* container; - if (MaybeRecreateContainerForFrameRemoval(childFrame, aFlags, &rv, &container)) { + if (MaybeRecreateContainerForFrameRemoval(childFrame, aFlags, &container)) { LAYOUT_PHASE_TEMP_REENTER(); MOZ_ASSERT(container); *aDidReconstruct = true; if (aDestroyedFramesFor) { *aDestroyedFramesFor = container; } - return rv; + return; } LAYOUT_PHASE_TEMP_REENTER(); @@ -8413,10 +8406,10 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, // Just reframe the parent, since framesets are weird like that. *aDidReconstruct = true; LAYOUT_PHASE_TEMP_EXIT(); - nsresult rv = RecreateFramesForContent(parentFrame->GetContent(), false, - aFlags, aDestroyedFramesFor); + RecreateFramesForContent(parentFrame->GetContent(), false, + aFlags, aDestroyedFramesFor); LAYOUT_PHASE_TEMP_REENTER(); - return rv; + return; } // If we're a child of MathML, then we should reframe the MathML content. @@ -8427,10 +8420,10 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, if (possibleMathMLAncestor->IsFrameOfType(nsIFrame::eMathML)) { *aDidReconstruct = true; LAYOUT_PHASE_TEMP_EXIT(); - nsresult rv = RecreateFramesForContent(possibleMathMLAncestor->GetContent(), - false, aFlags, aDestroyedFramesFor); + RecreateFramesForContent(possibleMathMLAncestor->GetContent(), + false, aFlags, aDestroyedFramesFor); LAYOUT_PHASE_TEMP_REENTER(); - return rv; + return; } // Undo XUL wrapping if it's no longer needed. @@ -8444,10 +8437,10 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, !AnyKidsNeedBlockParent(childFrame->GetNextSibling())) { *aDidReconstruct = true; LAYOUT_PHASE_TEMP_EXIT(); - nsresult rv = RecreateFramesForContent(grandparentFrame->GetContent(), true, - aFlags, aDestroyedFramesFor); + RecreateFramesForContent(grandparentFrame->GetContent(), true, + aFlags, aDestroyedFramesFor); LAYOUT_PHASE_TEMP_REENTER(); - return rv; + return; } #ifdef ACCESSIBILITY @@ -8491,7 +8484,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, // XXXbz the GetContent() != aChild check is needed due to bug 135040. // Remove it once that's fixed. ClearUndisplayedContentIn(aChild, aContainer); - return NS_OK; + return; } parentFrame = childFrame->GetParent(); parentType = parentFrame->GetType(); @@ -8581,7 +8574,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, #endif } - return rv; + return; } /** @@ -8643,12 +8636,11 @@ nsCSSFrameConstructor::EnsureFrameForTextNode(nsGenericDOMDataNode* aContent) return aContent->GetPrimaryFrame(); } -nsresult +void nsCSSFrameConstructor::CharacterDataChanged(nsIContent* aContent, CharacterDataChangeInfo* aInfo) { AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC); - nsresult rv = NS_OK; if ((aContent->HasFlag(NS_CREATE_FRAME_IF_NON_WHITESPACE) && !aContent->TextIsOnlyWhitespace()) || @@ -8660,10 +8652,10 @@ nsCSSFrameConstructor::CharacterDataChanged(nsIContent* aContent, "Bit should never be set on generated content"); #endif LAYOUT_PHASE_TEMP_EXIT(); - nsresult rv = RecreateFramesForContent(aContent, false, - REMOVE_FOR_RECONSTRUCTION, nullptr); + RecreateFramesForContent(aContent, false, + REMOVE_FOR_RECONSTRUCTION, nullptr); LAYOUT_PHASE_TEMP_REENTER(); - return rv; + return; } // Find the child frame @@ -8711,8 +8703,6 @@ nsCSSFrameConstructor::CharacterDataChanged(nsIContent* aContent, RecoverLetterFrames(block); } } - - return rv; } void @@ -9371,12 +9361,10 @@ FindPreviousNonWhitespaceSibling(nsIFrame* aFrame) bool nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, RemoveFlags aFlags, - nsresult* aResult, nsIContent** aDestroyedFramesFor) { NS_PRECONDITION(aFrame, "Must have a frame"); NS_PRECONDITION(aFrame->GetParent(), "Frame shouldn't be root"); - NS_PRECONDITION(aResult, "Null out param?"); NS_PRECONDITION(aFrame == aFrame->FirstContinuation(), "aFrame not the result of GetPrimaryFrame()?"); @@ -9394,7 +9382,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, } #endif - *aResult = ReframeContainingBlock(aFrame, aFlags, aDestroyedFramesFor); + ReframeContainingBlock(aFrame, aFlags, aDestroyedFramesFor); return true; } @@ -9403,8 +9391,8 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, 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 - *aResult = RecreateFramesForContent(aFrame->GetParent()->GetContent(), false, - aFlags, aDestroyedFramesFor); + RecreateFramesForContent(aFrame->GetParent()->GetContent(), false, + aFlags, aDestroyedFramesFor); return true; } @@ -9417,9 +9405,9 @@ 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. - *aResult = RecreateFramesForContent(aFrame->GetParent()->GetContent(), - false, REMOVE_FOR_RECONSTRUCTION, - aDestroyedFramesFor); + RecreateFramesForContent(aFrame->GetParent()->GetContent(), + false, REMOVE_FOR_RECONSTRUCTION, + aDestroyedFramesFor); return true; } } @@ -9451,8 +9439,8 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, 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 - *aResult = RecreateFramesForContent(parent->GetContent(), true, aFlags, - aDestroyedFramesFor); + RecreateFramesForContent(parent->GetContent(), true, aFlags, + aDestroyedFramesFor); return true; } } @@ -9481,8 +9469,8 @@ 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. - *aResult = RecreateFramesForContent(parent->GetContent(), true, aFlags, - aDestroyedFramesFor); + RecreateFramesForContent(parent->GetContent(), true, aFlags, + aDestroyedFramesFor); return true; } } @@ -9498,8 +9486,8 @@ 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. - *aResult = RecreateFramesForContent(parent->GetContent(), true, aFlags, - aDestroyedFramesFor); + RecreateFramesForContent(parent->GetContent(), true, aFlags, + aDestroyedFramesFor); return true; } @@ -9522,8 +9510,8 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, } #endif // DEBUG // Recreate frames for the flex container (the removed frame's parent) - *aResult = RecreateFramesForContent(parent->GetContent(), true, aFlags, - aDestroyedFramesFor); + RecreateFramesForContent(parent->GetContent(), true, aFlags, + aDestroyedFramesFor); return true; } @@ -9541,8 +9529,8 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, } #endif // DEBUG // Recreate frames for the flex container (the removed frame's grandparent) - *aResult = RecreateFramesForContent(parent->GetParent()->GetContent(), true, - aFlags, aDestroyedFramesFor); + RecreateFramesForContent(parent->GetParent()->GetContent(), true, + aFlags, aDestroyedFramesFor); return true; } @@ -9550,7 +9538,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, if (aFrame->GetType() == nsGkAtoms::popupSetFrame) { nsIRootBox* rootBox = nsIRootBox::GetRootBox(mPresShell); if (rootBox && rootBox->GetPopupSetFrame() == aFrame) { - *aResult = ReconstructDocElementHierarchy(); + ReconstructDocElementHierarchy(); return true; } } @@ -9562,8 +9550,8 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, !inFlowFrame->GetNextSibling() && ((parent->GetPrevContinuation() && !parent->GetPrevInFlow()) || (parent->GetNextContinuation() && !parent->GetNextInFlow()))) { - *aResult = RecreateFramesForContent(parent->GetContent(), true, aFlags, - aDestroyedFramesFor); + RecreateFramesForContent(parent->GetContent(), true, aFlags, + aDestroyedFramesFor); return true; } @@ -9599,11 +9587,11 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, } #endif - *aResult = ReframeContainingBlock(parent, aFlags, aDestroyedFramesFor); + ReframeContainingBlock(parent, aFlags, aDestroyedFramesFor); return true; } -nsresult +void nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, bool aAsyncInsert, RemoveFlags aFlags, @@ -9616,7 +9604,9 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, // anyway). // Rebuilding the frame tree can have bad effects, especially if it's the // frame tree for chrome (see bug 157322). - NS_ENSURE_TRUE(aContent->GetComposedDoc(), NS_ERROR_FAILURE); + if (NS_WARN_IF(!aContent->GetComposedDoc())) { + return; + } // Is the frame ib-split? If so, we need to reframe the containing // block *here*, rather than trying to remove and re-insert the @@ -9680,15 +9670,14 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, } } - nsresult rv = NS_OK; nsIContent* container; - if (frame && MaybeRecreateContainerForFrameRemoval(frame, aFlags, &rv, + if (frame && MaybeRecreateContainerForFrameRemoval(frame, aFlags, &container)) { MOZ_ASSERT(container); if (aDestroyedFramesFor) { *aDestroyedFramesFor = container; } - return rv; + return; } nsINode* containerNode = aContent->GetParentNode(); @@ -9708,11 +9697,8 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, nullptr : aContent->GetNextSibling(); const bool reconstruct = aFlags != REMOVE_DESTROY_FRAMES; RemoveFlags flags = reconstruct ? REMOVE_FOR_RECONSTRUCTION : aFlags; - rv = ContentRemoved(container, aContent, nextSibling, flags, - &didReconstruct, aDestroyedFramesFor); - if (NS_FAILED(rv)) { - return rv; - } + 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 @@ -9722,12 +9708,10 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, RestyleManager()->PostRestyleEvent( aContent->AsElement(), nsRestyleHint(0), nsChangeHint_ReconstructFrame); } else { - rv = ContentInserted(container, aContent, mTempFrameTreeState, false); + ContentInserted(container, aContent, mTempFrameTreeState, false); } } } - - return rv; } void @@ -11123,7 +11107,7 @@ nsCSSFrameConstructor::AppendFirstLineFrames( // Special routine to handle inserting a new frame into a block // frame's child list. Takes care of placing the new frame into the // right place when first-line style is present. -nsresult +void nsCSSFrameConstructor::InsertFirstLineFrames( nsFrameConstructorState& aState, nsIContent* aContent, @@ -11132,7 +11116,6 @@ nsCSSFrameConstructor::InsertFirstLineFrames( nsIFrame* aPrevSibling, nsFrameItems& aFrameItems) { - nsresult rv = NS_OK; // XXXbz If you make this method actually do something, check to // make sure that the caller is passing what you expect. In // particular, which content is aContent? And audit the rest of @@ -11262,7 +11245,6 @@ nsCSSFrameConstructor::InsertFirstLineFrames( } #endif - return rv; } //---------------------------------------------------------------------- @@ -11586,7 +11568,7 @@ FindFirstLetterFrame(nsIFrame* aFrame, nsIFrame::ChildListID aListID) return nullptr; } -nsresult +void nsCSSFrameConstructor::RemoveFloatingFirstLetterFrames( nsIPresShell* aPresShell, nsIFrame* aBlockFrame) @@ -11598,7 +11580,7 @@ nsCSSFrameConstructor::RemoveFloatingFirstLetterFrames( floatFrame = ::FindFirstLetterFrame(aBlockFrame, nsIFrame::kPushedFloatsList); if (!floatFrame) { - return NS_OK; + return; } } @@ -11606,19 +11588,19 @@ nsCSSFrameConstructor::RemoveFloatingFirstLetterFrames( // destroyed when we destroy the letter frame). nsIFrame* textFrame = floatFrame->PrincipalChildList().FirstChild(); if (!textFrame) { - return NS_OK; + return; } // Discover the placeholder frame for the letter frame nsPlaceholderFrame* placeholderFrame = GetPlaceholderFrameFor(floatFrame); if (!placeholderFrame) { // Somethings really wrong - return NS_OK; + return; } nsContainerFrame* parentFrame = placeholderFrame->GetParent(); if (!parentFrame) { // Somethings really wrong - return NS_OK; + return; } // Create a new text frame with the right style context that maps @@ -11627,7 +11609,7 @@ nsCSSFrameConstructor::RemoveFloatingFirstLetterFrames( nsStyleContext* parentSC = parentFrame->StyleContext(); nsIContent* textContent = textFrame->GetContent(); if (!textContent) { - return NS_OK; + return; } RefPtr newSC = aPresShell->StyleSet()-> ResolveStyleForText(textContent, parentSC); @@ -11672,11 +11654,9 @@ nsCSSFrameConstructor::RemoveFloatingFirstLetterFrames( if (offsetsNeedFixing) { prevSibling->RemoveStateBits(TEXT_OFFSETS_NEED_FIXING); } - - return NS_OK; } -nsresult +void nsCSSFrameConstructor::RemoveFirstLetterFrames(nsIPresShell* aPresShell, nsContainerFrame* aFrame, nsContainerFrame* aBlockFrame, @@ -11749,11 +11729,9 @@ nsCSSFrameConstructor::RemoveFirstLetterFrames(nsIPresShell* aPresShell, prevSibling = kid; kid = kid->GetNextSibling(); } - - return NS_OK; } -nsresult +void nsCSSFrameConstructor::RemoveLetterFrames(nsIPresShell* aPresShell, nsContainerFrame* aBlockFrame) { @@ -11762,20 +11740,16 @@ nsCSSFrameConstructor::RemoveLetterFrames(nsIPresShell* aPresShell, nsContainerFrame* continuation = aBlockFrame; bool stopLooking = false; - nsresult rv; do { - rv = RemoveFloatingFirstLetterFrames(aPresShell, continuation); - if (NS_SUCCEEDED(rv)) { - rv = RemoveFirstLetterFrames(aPresShell, - continuation, aBlockFrame, &stopLooking); - } + RemoveFloatingFirstLetterFrames(aPresShell, continuation); + RemoveFirstLetterFrames(aPresShell, continuation, aBlockFrame, + &stopLooking); if (stopLooking) { break; } continuation = static_cast(continuation->GetNextContinuation()); } while (continuation); - return rv; } // Fixup the letter frame situation for the given block @@ -11818,7 +11792,7 @@ nsCSSFrameConstructor::RecoverLetterFrames(nsContainerFrame* aBlockFrame) // listbox Widget Routines -nsresult +void nsCSSFrameConstructor::CreateListBoxContent(nsContainerFrame* aParentFrame, nsIFrame* aPrevFrame, nsIContent* aChild, @@ -11826,8 +11800,6 @@ nsCSSFrameConstructor::CreateListBoxContent(nsContainerFrame* aParentFrame, bool aIsAppend) { #ifdef MOZ_XUL - nsresult rv = NS_OK; - // Construct a new frame if (nullptr != aParentFrame) { nsFrameItems frameItems; @@ -11848,7 +11820,7 @@ nsCSSFrameConstructor::CreateListBoxContent(nsContainerFrame* aParentFrame, if (StyleDisplay::None == display->mDisplay) { *aNewFrame = nullptr; - return NS_OK; + return; } BeginUpdate(); @@ -11867,9 +11839,9 @@ nsCSSFrameConstructor::CreateListBoxContent(nsContainerFrame* aParentFrame, if (newFrame) { // Notify the parent frame if (aIsAppend) - rv = ((nsListBoxBodyFrame*)aParentFrame)->ListBoxAppendFrames(frameItems); + ((nsListBoxBodyFrame*)aParentFrame)->ListBoxAppendFrames(frameItems); else - rv = ((nsListBoxBodyFrame*)aParentFrame)->ListBoxInsertFrames(aPrevFrame, frameItems); + ((nsListBoxBodyFrame*)aParentFrame)->ListBoxInsertFrames(aPrevFrame, frameItems); } EndUpdate(); @@ -11884,10 +11856,6 @@ nsCSSFrameConstructor::CreateListBoxContent(nsContainerFrame* aParentFrame, } #endif } - - return rv; -#else - return NS_ERROR_FAILURE; #endif } @@ -12659,7 +12627,7 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, return true; } -nsresult +void nsCSSFrameConstructor::ReframeContainingBlock(nsIFrame* aFrame, RemoveFlags aFlags, nsIContent** aDestroyedFramesFor) @@ -12682,7 +12650,7 @@ nsCSSFrameConstructor::ReframeContainingBlock(nsIFrame* aFrame, // don't ReframeContainingBlock, this will result in a crash // if we remove a tree that's in reflow - see bug 121368 for testcase NS_ERROR("Atemptted to nsCSSFrameConstructor::ReframeContainingBlock during a Reflow!!!"); - return NS_OK; + return; } // Get the first "normal" ancestor of the target frame. @@ -12714,7 +12682,7 @@ nsCSSFrameConstructor::ReframeContainingBlock(nsIFrame* aFrame, true, aFlags, nullptr); } -nsresult +void nsCSSFrameConstructor::GenerateChildFrames(nsContainerFrame* aFrame) { { @@ -12748,8 +12716,6 @@ nsCSSFrameConstructor::GenerateChildFrames(nsContainerFrame* aFrame) // call XBL constructors after the frames are created mPresShell->GetDocument()->BindingManager()->ProcessAttachedQueue(); - - return NS_OK; } ////////////////////////////////////////////////////////// diff --git a/layout/base/nsCSSFrameConstructor.h b/layout/base/nsCSSFrameConstructor.h index db0d92566..655519e0f 100644 --- a/layout/base/nsCSSFrameConstructor.h +++ b/layout/base/nsCSSFrameConstructor.h @@ -48,7 +48,7 @@ class FlattenedChildIterator; } // namespace dom } // namespace mozilla -class nsCSSFrameConstructor : public nsFrameManager +class nsCSSFrameConstructor final : public nsFrameManager { public: typedef mozilla::CSSPseudoElementType CSSPseudoElementType; @@ -60,7 +60,7 @@ public: nsCSSFrameConstructor(nsIDocument* aDocument, nsIPresShell* aPresShell); ~nsCSSFrameConstructor(void) { - NS_ASSERTION(mUpdateCount == 0, "Dying in the middle of our own update?"); + MOZ_ASSERT(mUpdateCount == 0, "Dying in the middle of our own update?"); } // get the alternate text for a content node @@ -78,7 +78,7 @@ public: nsIFrame* ConstructRootFrame(); - nsresult ReconstructDocElementHierarchy(); + void ReconstructDocElementHierarchy(); // Create frames for content nodes that are marked as needing frames. This // should be called before ProcessPendingRestyles. @@ -202,16 +202,16 @@ public: // If aAllowLazyConstruction is true then frame construction of the new // children can be done lazily. - nsresult ContentAppended(nsIContent* aContainer, - nsIContent* aFirstNewContent, - bool aAllowLazyConstruction); + void ContentAppended(nsIContent* aContainer, + nsIContent* aFirstNewContent, + bool aAllowLazyConstruction); // If aAllowLazyConstruction is true then frame construction of the new child // can be done lazily. - nsresult ContentInserted(nsIContent* aContainer, - nsIContent* aChild, - nsILayoutHistoryState* aFrameState, - bool aAllowLazyConstruction); + void ContentInserted(nsIContent* aContainer, + nsIContent* aChild, + nsILayoutHistoryState* aFrameState, + bool aAllowLazyConstruction); // Like ContentInserted but handles inserting the children of aContainer in // the range [aStartChild, aEndChild). aStartChild must be non-null. @@ -219,11 +219,11 @@ public: // aStartChild. If aAllowLazyConstruction is true then frame construction of // the new children can be done lazily. It is only allowed to be true when // inserting a single node. - nsresult ContentRangeInserted(nsIContent* aContainer, - nsIContent* aStartChild, - nsIContent* aEndChild, - nsILayoutHistoryState* aFrameState, - bool aAllowLazyConstruction); + void ContentRangeInserted(nsIContent* aContainer, + nsIContent* aStartChild, + nsIContent* aEndChild, + nsILayoutHistoryState* aFrameState, + bool aAllowLazyConstruction); enum RemoveFlags { REMOVE_CONTENT, REMOVE_FOR_RECONSTRUCTION, REMOVE_DESTROY_FRAMES }; @@ -243,15 +243,15 @@ public: * only when aFlags == REMOVE_DESTROY_FRAMES, otherwise it will only be * captured if we reconstructed frames for an ancestor. */ - nsresult 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, + nsIContent** aDestroyedFramesFor = nullptr); - nsresult CharacterDataChanged(nsIContent* aContent, - CharacterDataChangeInfo* aInfo); + void CharacterDataChanged(nsIContent* aContent, + CharacterDataChangeInfo* aInfo); // If aContent is a text node that has been optimized away due to being // whitespace next to a block boundary (or for some other reason), stop @@ -260,8 +260,8 @@ public: // Returns the frame for aContent if there is one. nsIFrame* EnsureFrameForTextNode(nsGenericDOMDataNode* aContent); - // generate the child frames and process bindings - nsresult GenerateChildFrames(nsContainerFrame* aFrame); + // Generate the child frames and process bindings + void GenerateChildFrames(nsContainerFrame* aFrame); // Should be called when a frame is going to be destroyed and // WillDestroyFrameTree hasn't been called yet. @@ -303,11 +303,11 @@ public: */ InsertionPoint GetInsertionPoint(nsIContent* aContainer, nsIContent* aChild); - nsresult CreateListBoxContent(nsContainerFrame* aParentFrame, - nsIFrame* aPrevFrame, - nsIContent* aChild, - nsIFrame** aResult, - bool aIsAppend); + void CreateListBoxContent(nsContainerFrame* aParentFrame, + nsIFrame* aPrevFrame, + nsIContent* aChild, + nsIFrame** aResult, + bool aIsAppend); // GetInitialContainingBlock() is deprecated in favor of GetRootElementFrame(); // nsIFrame* GetInitialContainingBlock() { return mRootElementFrame; } @@ -419,14 +419,14 @@ private: * @param [out] aNewContent the content node we create * @param [out] aNewFrame the new frame we create */ - nsresult CreateAttributeContent(nsIContent* aParentContent, - nsIFrame* aParentFrame, - int32_t aAttrNamespace, - nsIAtom* aAttrName, - nsStyleContext* aStyleContext, - nsCOMArray& aGeneratedContent, - nsIContent** aNewContent, - nsIFrame** aNewFrame); + void CreateAttributeContent(nsIContent* aParentContent, + nsIFrame* aParentFrame, + int32_t aAttrNamespace, + nsIAtom* aAttrName, + nsStyleContext* aStyleContext, + nsCOMArray& aGeneratedContent, + nsIContent** aNewContent, + nsIFrame** aNewFrame); /** * Create a text node containing the given string. If aText is non-null @@ -464,11 +464,11 @@ private: // aParentFrame. aPrevSibling must be the frame after which aFrameList is to // be placed on aParentFrame's principal child list. It may be null if // aFrameList is being added at the beginning of the child list. - nsresult AppendFramesToParent(nsFrameConstructorState& aState, - nsContainerFrame* aParentFrame, - nsFrameItems& aFrameList, - nsIFrame* aPrevSibling, - bool aIsRecursiveCall = false); + void AppendFramesToParent(nsFrameConstructorState& aState, + nsContainerFrame* aParentFrame, + nsFrameItems& aFrameList, + nsIFrame* aPrevSibling, + bool aIsRecursiveCall = false); // BEGIN TABLE SECTION /** @@ -1692,7 +1692,7 @@ private: // InitializeSelectFrame puts scrollFrame in aFrameItems if aBuildCombobox is false // aBuildCombobox indicates if we are building a combobox that has a dropdown // popup widget or not. - nsresult + void InitializeSelectFrame(nsFrameConstructorState& aState, nsContainerFrame* aScrollFrame, nsContainerFrame* aScrolledFrame, @@ -1721,7 +1721,7 @@ private: * @param aDestroyedFramesFor if non-null, it will contain the content that * was actually reframed - it may be different than aContent. */ - nsresult + void RecreateFramesForContent(nsIContent* aContent, bool aAsyncInsert, RemoveFlags aFlags, @@ -1739,7 +1739,6 @@ private: // content that was reframed. bool MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, RemoveFlags aFlags, - nsresult* aResult, nsIContent** aDestroyedFramesFor); nsIFrame* CreateContinuingOuterTableFrame(nsIPresShell* aPresShell, @@ -1864,9 +1863,9 @@ private: bool aIsAppend, nsIFrame* aPrevSibling); - nsresult ReframeContainingBlock(nsIFrame* aFrame, - RemoveFlags aFlags, - nsIContent** aReframeContent); + void ReframeContainingBlock(nsIFrame* aFrame, + RemoveFlags aFlags, + nsIContent** aReframeContent); //---------------------------------------- @@ -1920,18 +1919,18 @@ private: void RecoverLetterFrames(nsContainerFrame* aBlockFrame); // - nsresult RemoveLetterFrames(nsIPresShell* aPresShell, - nsContainerFrame* aBlockFrame); + void RemoveLetterFrames(nsIPresShell* aPresShell, + nsContainerFrame* aBlockFrame); // Recursive helper for RemoveLetterFrames - nsresult RemoveFirstLetterFrames(nsIPresShell* aPresShell, - nsContainerFrame* aFrame, - nsContainerFrame* aBlockFrame, - bool* aStopLooking); + void RemoveFirstLetterFrames(nsIPresShell* aPresShell, + nsContainerFrame* aFrame, + nsContainerFrame* aBlockFrame, + bool* aStopLooking); // Special remove method for those pesky floating first-letter frames - nsresult RemoveFloatingFirstLetterFrames(nsIPresShell* aPresShell, - nsIFrame* aBlockFrame); + void RemoveFloatingFirstLetterFrames(nsIPresShell* aPresShell, + nsIFrame* aBlockFrame); // Capture state for the frame tree rooted at the frame associated with the // content object, aContent @@ -1962,12 +1961,12 @@ private: nsContainerFrame* aBlockFrame, nsFrameItems& aFrameItems); - nsresult InsertFirstLineFrames(nsFrameConstructorState& aState, - nsIContent* aContent, - nsIFrame* aBlockFrame, - nsContainerFrame** aParentFrame, - nsIFrame* aPrevSibling, - nsFrameItems& aFrameItems); + void InsertFirstLineFrames(nsFrameConstructorState& aState, + nsIContent* aContent, + nsIFrame* aBlockFrame, + nsContainerFrame** aParentFrame, + nsIFrame* aPrevSibling, + nsFrameItems& aFrameItems); /** * Find the right frame to use for aContent when looking for sibling diff --git a/layout/base/nsFrameManager.cpp b/layout/base/nsFrameManager.cpp index 97b022da8..6bcc715b9 100644 --- a/layout/base/nsFrameManager.cpp +++ b/layout/base/nsFrameManager.cpp @@ -38,13 +38,8 @@ #include "nsIStatefulFrame.h" #include "nsContainerFrame.h" - #ifdef DEBUG - //#define DEBUG_UNDISPLAYED_MAP - //#define DEBUG_DISPLAY_CONTENTS_MAP - #else - #undef DEBUG_UNDISPLAYED_MAP - #undef DEBUG_DISPLAY_CONTENTS_MAP - #endif +// #define DEBUG_UNDISPLAYED_MAP +// #define DEBUG_DISPLAY_CONTENTS_MAP using namespace mozilla; using namespace mozilla::dom; @@ -87,40 +82,49 @@ nsFrameManagerBase::nsFrameManagerBase() //---------------------------------------------------------------------- -// XXXldb This seems too complicated for what I think it's doing, and it -// should also be using PLDHashTable rather than plhash to use less memory. +/** + * The undisplayed map is a class that maps a parent content node to the + * undisplayed content children, and their style contexts. + * + * The linked list of nodes holds strong references to the style contexts and + * the content. + */ +class nsFrameManagerBase::UndisplayedMap : + private nsClassHashtable, + LinkedList> +{ + typedef nsClassHashtable, LinkedList> base_type; -class nsFrameManagerBase::UndisplayedMap { public: - explicit UndisplayedMap(uint32_t aNumBuckets = 16); - ~UndisplayedMap(void); + UndisplayedMap(); + ~UndisplayedMap(); UndisplayedNode* GetFirstNode(nsIContent* aParentContent); - nsresult AddNodeFor(nsIContent* aParentContent, - nsIContent* aChild, nsStyleContext* aStyle); + void AddNodeFor(nsIContent* aParentContent, + nsIContent* aChild, + nsStyleContext* aStyle); - void RemoveNodeFor(nsIContent* aParentContent, - UndisplayedNode* aNode); + void RemoveNodeFor(nsIContent* aParentContent, UndisplayedNode* aNode); void RemoveNodesFor(nsIContent* aParentContent); - UndisplayedNode* UnlinkNodesFor(nsIContent* aParentContent); + + nsAutoPtr> + UnlinkNodesFor(nsIContent* aParentContent); // Removes all entries from the hash table - void Clear(void); + void Clear(); protected: + LinkedList* GetListFor(nsIContent** aParentContent); + LinkedList* GetOrCreateListFor(nsIContent** aParentContent); + void AppendNodeFor(UndisplayedNode* aNode, nsIContent* aParentContent); /** - * Gets the entry for the provided parent content. If the content - * is a element, |**aParentContent| is set to - * the parent of the children element. + * Get the applicable parent for the map lookup. This is almost always the + * provided argument, except if it's a element, in which case + * it's the parent of the children element. */ - PLHashEntry** GetEntryFor(nsIContent** aParentContent); - void AppendNodeFor(UndisplayedNode* aNode, - nsIContent* aParentContent); - - PLHashTable* mTable; - PLHashEntry** mLastLookup; + nsIContent* GetApplicableParent(nsIContent* aParent); }; //---------------------------------------------------------------------- @@ -145,7 +149,6 @@ nsFrameManager::Destroy() mRootFrame->Destroy(); mRootFrame = nullptr; } - delete mUndisplayedMap; mUndisplayedMap = nullptr; delete mDisplayContentsMap; @@ -215,7 +218,7 @@ nsFrameManager::GetStyleContextInMap(UndisplayedMap* aMap, nsIContent* aContent) nsIContent* parent = aContent->GetParentElementCrossingShadowRoot(); MOZ_ASSERT(parent || !aContent->GetParent(), "no non-elements"); for (UndisplayedNode* node = aMap->GetFirstNode(parent); - node; node = node->mNext) { + node; node = node->getNext()) { if (node->mContent == aContent) return node->mStyle; } @@ -241,16 +244,16 @@ nsFrameManager::SetStyleContextInMap(UndisplayedMap* aMap, nsIContent* aContent, nsStyleContext* aStyleContext) { - NS_PRECONDITION(!aStyleContext->GetPseudo(), - "Should only have actual elements here"); + MOZ_ASSERT(!aStyleContext->GetPseudo(), + "Should only have actual elements here"); #if defined(DEBUG_UNDISPLAYED_MAP) || defined(DEBUG_DISPLAY_BOX_CONTENTS_MAP) static int i = 0; printf("SetStyleContextInMap(%d): p=%p \n", i++, (void *)aContent); #endif - NS_ASSERTION(!GetStyleContextInMap(aMap, aContent), - "Already have an entry for aContent"); + MOZ_ASSERT(!GetStyleContextInMap(aMap, aContent), + "Already have an entry for aContent"); nsIContent* parent = aContent->GetParentElementCrossingShadowRoot(); MOZ_ASSERT(parent || !aContent->GetParent(), "no non-elements"); @@ -287,7 +290,7 @@ nsFrameManager::ChangeStyleContextInMap(UndisplayedMap* aMap, #endif for (UndisplayedNode* node = aMap->GetFirstNode(aContent->GetParent()); - node; node = node->mNext) { + node; node = node->getNext()) { if (node->mContent == aContent) { node->mStyle = aStyleContext; return; @@ -306,25 +309,25 @@ nsFrameManager::ClearUndisplayedContentIn(nsIContent* aContent, printf("ClearUndisplayedContent(%d): content=%p parent=%p --> ", i++, (void *)aContent, (void*)aParentContent); #endif - if (mUndisplayedMap) { - UndisplayedNode* node = mUndisplayedMap->GetFirstNode(aParentContent); - while (node) { - if (node->mContent == aContent) { - mUndisplayedMap->RemoveNodeFor(aParentContent, node); + if (!mUndisplayedMap) { + return; + } + + for (UndisplayedNode* node = mUndisplayedMap->GetFirstNode(aParentContent); + node; node = node->getNext()) { + if (node->mContent == aContent) { + mUndisplayedMap->RemoveNodeFor(aParentContent, node); #ifdef DEBUG_UNDISPLAYED_MAP - printf( "REMOVED!\n"); -#endif -#ifdef DEBUG - // make sure that there are no more entries for the same content - nsStyleContext *context = GetUndisplayedContent(aContent); - NS_ASSERTION(context == nullptr, "Found more undisplayed content data after removal"); + printf( "REMOVED!\n"); #endif - return; - } - node = node->mNext; + // make sure that there are no more entries for the same content + MOZ_ASSERT(!GetUndisplayedContent(aContent), + "Found more undisplayed content data after removal"); + return; } } + #ifdef DEBUG_UNDISPLAYED_MAP printf( "not found.\n"); #endif @@ -380,26 +383,25 @@ nsFrameManager::ClearDisplayContentsIn(nsIContent* aContent, static int i = 0; printf("ClearDisplayContents(%d): content=%p parent=%p --> ", i++, (void *)aContent, (void*)aParentContent); #endif - - if (mDisplayContentsMap) { - UndisplayedNode* node = mDisplayContentsMap->GetFirstNode(aParentContent); - while (node) { - if (node->mContent == aContent) { - mDisplayContentsMap->RemoveNodeFor(aParentContent, node); + + if (!mDisplayContentsMap) { + return; + } + + for (UndisplayedNode* node = mDisplayContentsMap->GetFirstNode(aParentContent); + node; node = node->getNext()) { + if (node->mContent == aContent) { + mDisplayContentsMap->RemoveNodeFor(aParentContent, node); #ifdef DEBUG_DISPLAY_CONTENTS_MAP - printf( "REMOVED!\n"); + printf( "REMOVED!\n"); #endif -#ifdef DEBUG - // make sure that there are no more entries for the same content - nsStyleContext* context = GetDisplayContentsStyleFor(aContent); - NS_ASSERTION(context == nullptr, "Found more entries for aContent after removal"); -#endif - ClearAllDisplayContentsIn(aContent); - ClearAllUndisplayedContentIn(aContent); - return; - } - node = node->mNext; + // make sure that there are no more entries for the same content + MOZ_ASSERT(!GetDisplayContentsStyleFor(aContent), + "Found more entries for aContent after removal"); + ClearAllDisplayContentsIn(aContent); + ClearAllUndisplayedContentIn(aContent); + return; } } #ifdef DEBUG_DISPLAY_CONTENTS_MAP @@ -416,14 +418,14 @@ nsFrameManager::ClearAllDisplayContentsIn(nsIContent* aParentContent) #endif if (mDisplayContentsMap) { - UndisplayedNode* cur = mDisplayContentsMap->UnlinkNodesFor(aParentContent); - while (cur) { - UndisplayedNode* next = cur->mNext; - cur->mNext = nullptr; - ClearAllDisplayContentsIn(cur->mContent); - ClearAllUndisplayedContentIn(cur->mContent); - delete cur; - cur = next; + nsAutoPtr> list = + mDisplayContentsMap->UnlinkNodesFor(aParentContent); + if (list) { + while (UndisplayedNode* node = list->popFirst()) { + ClearAllDisplayContentsIn(node->mContent); + ClearAllUndisplayedContentIn(node->mContent); + delete node; + } } } @@ -654,182 +656,132 @@ nsFrameManager::RestoreFrameState(nsIFrame* aFrame, //---------------------------------------------------------------------- -static PLHashNumber -HashKey(void* key) -{ - return NS_PTR_TO_INT32(key); -} - -static int -CompareKeys(void* key1, void* key2) -{ - return key1 == key2; -} - -//---------------------------------------------------------------------- - -nsFrameManagerBase::UndisplayedMap::UndisplayedMap(uint32_t aNumBuckets) +nsFrameManagerBase::UndisplayedMap::UndisplayedMap() { MOZ_COUNT_CTOR(nsFrameManagerBase::UndisplayedMap); - mTable = PL_NewHashTable(aNumBuckets, (PLHashFunction)HashKey, - (PLHashComparator)CompareKeys, - (PLHashComparator)nullptr, - nullptr, nullptr); - mLastLookup = nullptr; } nsFrameManagerBase::UndisplayedMap::~UndisplayedMap(void) { MOZ_COUNT_DTOR(nsFrameManagerBase::UndisplayedMap); Clear(); - PL_HashTableDestroy(mTable); } -PLHashEntry** -nsFrameManagerBase::UndisplayedMap::GetEntryFor(nsIContent** aParentContent) +void +nsFrameManagerBase::UndisplayedMap::Clear() { - nsIContent* parentContent = *aParentContent; - - if (mLastLookup && (parentContent == (*mLastLookup)->key)) { - return mLastLookup; + for (auto iter = Iter(); !iter.Done(); iter.Next()) { + auto* list = iter.UserData(); + while (auto* node = list->popFirst()) { + delete node; + } + iter.Remove(); } +} +nsIContent* +nsFrameManagerBase::UndisplayedMap::GetApplicableParent(nsIContent* aParent) +{ // In the case of XBL default content, elements do not get a // frame causing a mismatch between the content tree and the frame tree. // |GetEntryFor| is sometimes called with the content tree parent (which may // be a element) but the parent in the frame tree would be the // insertion parent (parent of the element). Here the children // elements are normalized to the insertion parent to correct for the mismatch. - if (parentContent && nsContentUtils::IsContentInsertionPoint(parentContent)) { - parentContent = parentContent->GetParent(); - // Change the caller's pointer for the parent content to be the insertion parent. - *aParentContent = parentContent; + if (aParent && nsContentUtils::IsContentInsertionPoint(aParent)) { + return aParent->GetParent(); } - PLHashNumber hashCode = NS_PTR_TO_INT32(parentContent); - PLHashEntry** entry = PL_HashTableRawLookup(mTable, hashCode, parentContent); - if (*entry) { - mLastLookup = entry; - } - return entry; + return aParent; } -UndisplayedNode* -nsFrameManagerBase::UndisplayedMap::GetFirstNode(nsIContent* aParentContent) +LinkedList* +nsFrameManagerBase::UndisplayedMap::GetListFor(nsIContent** aParent) { - PLHashEntry** entry = GetEntryFor(&aParentContent); - if (*entry) { - return (UndisplayedNode*)((*entry)->value); + *aParent = GetApplicableParent(*aParent); + + LinkedList* list; + if (Get(*aParent, &list)) { + return list; } + return nullptr; } +LinkedList* +nsFrameManagerBase::UndisplayedMap::GetOrCreateListFor(nsIContent** aParent) +{ + *aParent = GetApplicableParent(*aParent); + return LookupOrAdd(*aParent); +} + + +UndisplayedNode* +nsFrameManagerBase::UndisplayedMap::GetFirstNode(nsIContent* aParentContent) +{ + auto* list = GetListFor(&aParentContent); + return list ? list->getFirst() : nullptr; +} + void nsFrameManagerBase::UndisplayedMap::AppendNodeFor(UndisplayedNode* aNode, nsIContent* aParentContent) { - PLHashEntry** entry = GetEntryFor(&aParentContent); - if (*entry) { - UndisplayedNode* node = (UndisplayedNode*)((*entry)->value); - while (node->mNext) { - if (node->mContent == aNode->mContent) { - // We actually need to check this in optimized builds because - // there are some callers that do this. See bug 118014, bug - // 136704, etc. - NS_NOTREACHED("node in map twice"); - delete aNode; - return; - } - node = node->mNext; - } - node->mNext = aNode; - } - else { - PLHashNumber hashCode = NS_PTR_TO_INT32(aParentContent); - PL_HashTableRawAdd(mTable, entry, hashCode, aParentContent, aNode); - mLastLookup = nullptr; // hashtable may have shifted bucket out from under us + LinkedList* list = GetOrCreateListFor(&aParentContent); + +#ifdef DEBUG + for (UndisplayedNode* node = list->getFirst(); node; node = node->getNext()) { + // NOTE: In the original code there was a work around for this case, I want + // to check it still happens before hacking around it the same way. + MOZ_ASSERT(node->mContent != aNode->mContent, + "Duplicated content in undisplayed list!"); } +#endif + + list->insertBack(aNode); } -nsresult +void nsFrameManagerBase::UndisplayedMap::AddNodeFor(nsIContent* aParentContent, - nsIContent* aChild, + nsIContent* aChild, nsStyleContext* aStyle) { UndisplayedNode* node = new UndisplayedNode(aChild, aStyle); - AppendNodeFor(node, aParentContent); - return NS_OK; } void nsFrameManagerBase::UndisplayedMap::RemoveNodeFor(nsIContent* aParentContent, UndisplayedNode* aNode) { - PLHashEntry** entry = GetEntryFor(&aParentContent); - NS_ASSERTION(*entry, "content not in map"); - if (*entry) { - if ((UndisplayedNode*)((*entry)->value) == aNode) { // first node - if (aNode->mNext) { - (*entry)->value = aNode->mNext; - aNode->mNext = nullptr; - } - else { - PL_HashTableRawRemove(mTable, entry, *entry); - mLastLookup = nullptr; // hashtable may have shifted bucket out from under us - } - } - else { - UndisplayedNode* node = (UndisplayedNode*)((*entry)->value); - while (node->mNext) { - if (node->mNext == aNode) { - node->mNext = aNode->mNext; - aNode->mNext = nullptr; - break; - } - node = node->mNext; - } - } - } +#ifdef DEBUG + auto list = GetListFor(&aParentContent); + MOZ_ASSERT(list, "content not in map"); + aNode->removeFrom(*list); +#else + aNode->remove(); +#endif delete aNode; } -UndisplayedNode* +nsAutoPtr> nsFrameManagerBase::UndisplayedMap::UnlinkNodesFor(nsIContent* aParentContent) { - PLHashEntry** entry = GetEntryFor(&aParentContent); - NS_ASSERTION(entry, "content not in map"); - if (*entry) { - UndisplayedNode* node = (UndisplayedNode*)((*entry)->value); - NS_ASSERTION(node, "null node for non-null entry in UndisplayedMap"); - PL_HashTableRawRemove(mTable, entry, *entry); - mLastLookup = nullptr; // hashtable may have shifted bucket out from under us - return node; - } - return nullptr; + nsAutoPtr> list; + RemoveAndForget(GetApplicableParent(aParentContent), list); + return list; } void nsFrameManagerBase::UndisplayedMap::RemoveNodesFor(nsIContent* aParentContent) { - delete UnlinkNodesFor(aParentContent); -} - -static int -RemoveUndisplayedEntry(PLHashEntry* he, int i, void* arg) -{ - UndisplayedNode* node = (UndisplayedNode*)(he->value); - delete node; - // Remove and free this entry and continue enumerating - return HT_ENUMERATE_REMOVE | HT_ENUMERATE_NEXT; -} - -void -nsFrameManagerBase::UndisplayedMap::Clear(void) -{ - mLastLookup = nullptr; - PL_HashTableEnumerateEntries(mTable, RemoveUndisplayedEntry, 0); + nsAutoPtr> list = UnlinkNodesFor(aParentContent); + if (list) { + while (auto* node = list->popFirst()) { + delete node; + } + } } uint32_t nsFrameManagerBase::sGlobalGenerationNumber; diff --git a/layout/base/nsFrameManager.h b/layout/base/nsFrameManager.h index 1b9148314..bca4a581a 100644 --- a/layout/base/nsFrameManager.h +++ b/layout/base/nsFrameManager.h @@ -33,32 +33,19 @@ namespace mozilla { * Node in a linked list, containing the style for an element that * does not have a frame but whose parent does have a frame. */ -struct UndisplayedNode { +struct UndisplayedNode : public LinkedListElement +{ UndisplayedNode(nsIContent* aContent, nsStyleContext* aStyle) - : mContent(aContent), - mStyle(aStyle), - mNext(nullptr) + : mContent(aContent) + , mStyle(aStyle) { MOZ_COUNT_CTOR(mozilla::UndisplayedNode); } - ~UndisplayedNode() - { - MOZ_COUNT_DTOR(mozilla::UndisplayedNode); - - // Delete mNext iteratively to avoid blowing up the stack (bug 460461). - UndisplayedNode* cur = mNext; - while (cur) { - UndisplayedNode* next = cur->mNext; - cur->mNext = nullptr; - delete cur; - cur = next; - } - } + ~UndisplayedNode() { MOZ_COUNT_DTOR(mozilla::UndisplayedNode); } - nsCOMPtr mContent; - RefPtr mStyle; - UndisplayedNode* mNext; + nsCOMPtr mContent; + RefPtr mStyle; }; } // namespace mozilla diff --git a/layout/base/nsIPresShell.h b/layout/base/nsIPresShell.h index 865f5534c..f89639b3e 100644 --- a/layout/base/nsIPresShell.h +++ b/layout/base/nsIPresShell.h @@ -977,7 +977,7 @@ public: /** * Reconstruct frames for all elements in the document */ - virtual nsresult ReconstructFrames() = 0; + virtual void ReconstructFrames() = 0; /** * Notify that a content node's state has changed diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp index 264b52b18..b190622c7 100644 --- a/layout/base/nsPresShell.cpp +++ b/layout/base/nsPresShell.cpp @@ -4444,14 +4444,14 @@ PresShell::NotifyCounterStylesAreDirty() mFrameConstructor->EndUpdate(); } -nsresult -PresShell::ReconstructFrames(void) +void +PresShell::ReconstructFrames() { NS_PRECONDITION(!mFrameConstructor->GetRootFrame() || mDidInitialize, "Must not have root frame before initial reflow"); if (!mDidInitialize || mIsDestroying) { // Nothing to do here - return NS_OK; + return; } nsCOMPtr kungFuDeathGrip(this); @@ -4461,16 +4461,14 @@ PresShell::ReconstructFrames(void) mDocument->FlushPendingNotifications(Flush_ContentAndNotify); if (mIsDestroying) { - return NS_OK; + return; } nsAutoCauseReflowNotifier crNotifier(this); mFrameConstructor->BeginUpdate(); - nsresult rv = mFrameConstructor->ReconstructDocElementHierarchy(); + mFrameConstructor->ReconstructDocElementHierarchy(); VERIFY_STYLE_TREE; mFrameConstructor->EndUpdate(); - - return rv; } void diff --git a/layout/base/nsPresShell.h b/layout/base/nsPresShell.h index f20370d73..f276273fa 100644 --- a/layout/base/nsPresShell.h +++ b/layout/base/nsPresShell.h @@ -200,7 +200,7 @@ public: virtual void NotifyCounterStylesAreDirty() override; - virtual nsresult ReconstructFrames(void) override; + virtual void ReconstructFrames(void) override; virtual void Freeze() override; virtual void Thaw() override; virtual void FireOrClearDelayedEvents(bool aFireEvents) override; -- cgit v1.2.3