Bug 852501 part 18. Make ConstructDocElementFrame return an nsIFrame*. r=dholbert
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 19 Mar 2013 21:47:53 -0400
changeset 125548 a1237c5af4aaba8c8916a17013decb9059e491d1
parent 125547 64bd4a734021ca9e486e16c99348fc0439960e0f
child 125549 5b4e1e56815ee405d5e93f85fbf2f4795171e61d
push id24459
push useremorley@mozilla.com
push dateWed, 20 Mar 2013 11:46:36 +0000
treeherdermozilla-central@1d6fe70c79c5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs852501
milestone22.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 852501 part 18. Make ConstructDocElementFrame return an nsIFrame*. r=dholbert
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -2249,30 +2249,27 @@ nsCSSFrameConstructor::PropagateScrollTo
   if (CheckOverflow(presContext, bodyStyle->StyleDisplay())) {
     // tell caller we stole the overflow style from the body element
     return bodyElement;
   }
 
   return nullptr;
 }
 
-nsresult
+nsIFrame*
 nsCSSFrameConstructor::ConstructDocElementFrame(Element*                 aDocElement,
-                                                nsILayoutHistoryState*   aFrameState,
-                                                nsIFrame**               aNewFrame)
+                                                nsILayoutHistoryState*   aFrameState)
 {
   NS_PRECONDITION(mFixedContainingBlock,
                   "No viewport?  Someone forgot to call ConstructRootFrame!");
   NS_PRECONDITION(mFixedContainingBlock == GetRootFrame(),
                   "Unexpected mFixedContainingBlock");
   NS_PRECONDITION(!mDocElementContainingBlock,
                   "Shouldn't have a doc element containing block here");
 
-  *aNewFrame = nullptr;
-
   // Make sure to call PropagateScrollToViewport before
   // SetUpDocElementContainingBlock, since it sets up our scrollbar state
   // properly.
 #ifdef DEBUG
   nsIContent* propagatedScrollFrom =
 #endif
     PropagateScrollToViewport();
 
@@ -2307,25 +2304,26 @@ nsCSSFrameConstructor::ConstructDocEleme
 
   // Ensure that our XBL bindings are installed.
   if (display->mBinding) {
     // Get the XBL loader.
     nsresult rv;
     bool resolveStyle;
     
     nsXBLService* xblService = nsXBLService::GetInstance();
-    if (!xblService)
-      return NS_ERROR_FAILURE;
+    if (!xblService) {
+      return nullptr;
+    }
 
     nsRefPtr<nsXBLBinding> binding;
     rv = xblService->LoadBindings(aDocElement, display->mBinding->GetURI(),
                                   display->mBinding->mOriginPrincipal,
                                   getter_AddRefs(binding), &resolveStyle);
     if (NS_FAILED(rv) && rv != NS_ERROR_XBL_BLOCKED)
-      return NS_OK; // Binding will load asynchronously.
+      return nullptr; // Binding will load asynchronously.
 
     if (binding) {
       // For backwards compat, keep firing the root's constructor
       // after all of its kids' constructors.  So tell the binding
       // manager about it right now.
       mDocument->BindingManager()->AddToAttachedQueue(binding);
     }
 
@@ -2342,17 +2340,17 @@ nsCSSFrameConstructor::ConstructDocEleme
   NS_ASSERTION(!display->IsScrollableOverflow() || 
                state.mPresContext->IsPaginated() ||
                propagatedScrollFrom == aDocElement,
                "Scrollbars should have been propagated to the viewport");
 #endif
 
   if (MOZ_UNLIKELY(display->mDisplay == NS_STYLE_DISPLAY_NONE)) {
     SetUndisplayedContent(aDocElement, styleContext);
-    return NS_OK;
+    return nullptr;
   }
 
   TreeMatchContext::AutoAncestorPusher
     ancestorPusher(true, state.mTreeMatchContext, aDocElement);
 
   // Make sure to start any background image loads for the root element now.
   styleContext->StartBackgroundImageLoads();
 
@@ -2364,32 +2362,33 @@ nsCSSFrameConstructor::ConstructDocEleme
     state.PushAbsoluteContainingBlock(mDocElementContainingBlock,
                                       absoluteSaveState);
   }
 
   // The rules from CSS 2.1, section 9.2.4, have already been applied
   // by the style system, so we can assume that display->mDisplay is
   // either NONE, BLOCK, or TABLE.
 
-  // contentFrame is the primary frame for the root element. *aNewFrame
+  // contentFrame is the primary frame for the root element. newFrame
   // is the frame that will be the child of the initial containing block.
   // These are usually the same frame but they can be different, in
   // particular if the root frame is positioned, in which case
-  // contentFrame is the out-of-flow frame and *aNewFrame is the
+  // contentFrame is the out-of-flow frame and newFrame is the
   // placeholder.
   nsIFrame* contentFrame;
+  nsIFrame* newFrame;
   bool processChildren = false;
 
   // Check whether we need to build a XUL box or SVG root frame
 #ifdef MOZ_XUL
   if (aDocElement->IsXUL()) {
     contentFrame = NS_NewDocElementBoxFrame(mPresShell, styleContext);
     InitAndRestoreFrame(state, aDocElement, mDocElementContainingBlock, nullptr,
                         contentFrame);
-    *aNewFrame = contentFrame;
+    newFrame = contentFrame;
     processChildren = true;
   }
   else
 #endif
   if (aDocElement->IsSVG()) {
     if (aDocElement->Tag() == nsGkAtoms::svg) {
       // We're going to call the right function ourselves, so no need to give a
       // function to this FrameConstructionData.
@@ -2404,22 +2403,20 @@ nsCSSFrameConstructor::ConstructDocEleme
       FrameConstructionItem item(&rootSVGData, aDocElement,
                                  aDocElement->Tag(), kNameSpaceID_SVG,
                                  nullptr, extraRef.forget(), true);
 
       nsFrameItems frameItems;
       contentFrame = ConstructOuterSVG(state, item, mDocElementContainingBlock,
                                        styleContext->StyleDisplay(),
                                        frameItems);
-      if (!contentFrame || frameItems.IsEmpty())
-        return NS_ERROR_FAILURE;
-      *aNewFrame = frameItems.FirstChild();
+      newFrame = frameItems.FirstChild();
       NS_ASSERTION(frameItems.OnlyChild(), "multiple root element frames");
     } else {
-      return NS_ERROR_FAILURE;
+      return nullptr;
     }
   } else {
     bool docElemIsTable = (display->mDisplay == NS_STYLE_DISPLAY_TABLE);
     if (docElemIsTable) {
       // We're going to call the right function ourselves, so no need to give a
       // function to this FrameConstructionData.
 
       // XXXbz on the other hand, if we converted this whole function to
@@ -2433,39 +2430,36 @@ nsCSSFrameConstructor::ConstructDocEleme
                                  aDocElement->Tag(), kNameSpaceID_None,
                                  nullptr, extraRef.forget(), true);
 
       nsFrameItems frameItems;
       // if the document is a table then just populate it.
       contentFrame = ConstructTable(state, item, mDocElementContainingBlock,
                                     styleContext->StyleDisplay(),
                                     frameItems);
-      if (!contentFrame || frameItems.IsEmpty())
-        return NS_ERROR_FAILURE;
-      *aNewFrame = frameItems.FirstChild();
+      newFrame = frameItems.FirstChild();
       NS_ASSERTION(frameItems.OnlyChild(), "multiple root element frames");
     } else {
       contentFrame = NS_NewBlockFormattingContext(mPresShell, styleContext);
-      if (!contentFrame)
-        return NS_ERROR_OUT_OF_MEMORY;
       nsFrameItems frameItems;
       // Use a null PendingBinding, since our binding is not in fact pending.
       ConstructBlock(state, display, aDocElement,
                      state.GetGeometricParent(display,
                                               mDocElementContainingBlock),
                      mDocElementContainingBlock, styleContext,
                      &contentFrame, frameItems,
                      display->IsPositioned(contentFrame), nullptr);
-      if (frameItems.IsEmpty())
-        return NS_OK;
-      *aNewFrame = frameItems.FirstChild();
+      newFrame = frameItems.FirstChild();
       NS_ASSERTION(frameItems.OnlyChild(), "multiple root element frames");
     }
   }
 
+  MOZ_ASSERT(newFrame);
+  MOZ_ASSERT(contentFrame);
+
   // set the primary frame
   aDocElement->SetPrimaryFrame(contentFrame);
 
   NS_ASSERTION(processChildren ? !mRootElementFrame :
                  mRootElementFrame == contentFrame,
                "unexpected mRootElementFrame");
   mRootElementFrame = contentFrame;
 
@@ -2489,19 +2483,19 @@ nsCSSFrameConstructor::ConstructDocEleme
     // Use a null PendingBinding, since our binding is not in fact pending.
     ProcessChildren(state, aDocElement, styleContext, contentFrame, true,
                     childItems, false, nullptr);
 
     // Set the initial child lists
     contentFrame->SetInitialChildList(kPrincipalList, childItems);
   }
 
-  SetInitialSingleChild(mDocElementContainingBlock, *aNewFrame);
-
-  return NS_OK;
+  SetInitialSingleChild(mDocElementContainingBlock, newFrame);
+
+  return newFrame;
 }
 
 
 nsIFrame*
 nsCSSFrameConstructor::ConstructRootFrame()
 {
   AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC);
 
@@ -2540,17 +2534,17 @@ nsCSSFrameConstructor::ConstructRootFram
   mFixedContainingBlock = viewportFrame;
   // Make it an absolute container for fixed-pos elements
   mFixedContainingBlock->AddStateBits(NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN);
   mFixedContainingBlock->MarkAsAbsoluteContainingBlock();
 
   return viewportFrame;
 }
 
-nsresult
+void
 nsCSSFrameConstructor::SetUpDocElementContainingBlock(nsIContent* aDocElement)
 {
   NS_PRECONDITION(aDocElement, "No element?");
   NS_PRECONDITION(!aDocElement->GetParent(), "Not root content?");
   NS_PRECONDITION(aDocElement->GetCurrentDoc(), "Not in a document?");
   NS_PRECONDITION(aDocElement->GetCurrentDoc()->GetRootElement() ==
                   aDocElement, "Not the root of the document?");
 
@@ -2757,18 +2751,16 @@ nsCSSFrameConstructor::SetUpDocElementCo
   }
 
   if (viewportFrame->GetStateBits() & NS_FRAME_FIRST_REFLOW) {
     SetInitialSingleChild(viewportFrame, newFrame);
   } else {
     nsFrameList newFrameList(newFrame, newFrame);
     viewportFrame->AppendFrames(kPrincipalList, newFrameList);
   }
-
-  return NS_OK;
 }
 
 nsIFrame*
 nsCSSFrameConstructor::ConstructPageFrame(nsIPresShell*  aPresShell,
                                           nsPresContext* aPresContext,
                                           nsIFrame*      aParentFrame,
                                           nsIFrame*      aPrevPageFrame,
                                           nsIFrame*&     aCanvasFrame)
@@ -6848,18 +6840,16 @@ nsCSSFrameConstructor::ContentRangeInser
     // XXX the GetContent() != child check is needed due to bug 135040.
     // Remove it once that's fixed.  
     NS_ASSERTION(!child->GetPrimaryFrame() ||
                  child->GetPrimaryFrame()->GetContent() != child,
                  "asked to construct a frame for a node that already has a frame");
   }
 #endif
 
-  nsresult rv = NS_OK;
-
   bool isSingleInsert = (aStartChild->GetNextSibling() == aEndChild);
   NS_ASSERTION(isSingleInsert || !aAllowLazyConstruction,
                "range insert shouldn't be lazy");
   NS_ASSERTION(isSingleInsert || aEndChild,
                "range should not include all nodes after aStartChild");
 
 #ifdef MOZ_XUL
   if (aContainer && IsXULListBox(aContainer)) {
@@ -6895,20 +6885,20 @@ nsCSSFrameConstructor::ContentRangeInser
       // Not the root element; just bail out
       return NS_OK;
     }
 
     NS_PRECONDITION(nullptr == mRootElementFrame,
                     "root element frame already created");
 
     // Create frames for the document element and its child elements
-    nsIFrame*               docElementFrame;
-    rv = ConstructDocElementFrame(docElement, aFrameState, &docElementFrame);
-
-    if (NS_SUCCEEDED(rv) && docElementFrame) {
+    nsIFrame* docElementFrame =
+      ConstructDocElementFrame(docElement, aFrameState);
+
+    if (docElementFrame) {
       InvalidateCanvasIfNeeded(mPresShell, aStartChild);
 #ifdef DEBUG
       if (gReallyNoisyContentUpdates) {
         printf("nsCSSFrameConstructor::ContentRangeInserted: resulting frame "
                "model:\n");
         mFixedContainingBlock->List(stdout, 0);
       }
 #endif
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -432,26 +432,25 @@ private:
   // may suppress the construction of white-space-only text frames
   // must be skipped for these items and items around them.
   void AddFrameConstructionItems(nsFrameConstructorState& aState,
                                  nsIContent*              aContent,
                                  bool                     aSuppressWhiteSpaceOptimizations,
                                  nsIFrame*                aParentFrame,
                                  FrameConstructionItemList& aItems);
 
-  // Construct the frames for the document element.  This must always return a
-  // singe new frame (which may, of course, have a bunch of kids).
-  // XXXbz no need to return a frame here, imo.
-  nsresult ConstructDocElementFrame(Element*                 aDocElement,
-                                    nsILayoutHistoryState*   aFrameState,
-                                    nsIFrame**               aNewFrame);
+  // Construct the frames for the document element.  This can return null if the
+  // document element is display:none, or if the document element has a
+  // not-yet-loaded XBL binding, or if it's an SVG element that's not <svg>.
+  nsIFrame* ConstructDocElementFrame(Element*                 aDocElement,
+                                     nsILayoutHistoryState*   aFrameState);
 
   // Set up our mDocElementContainingBlock correctly for the given root
   // content.
-  nsresult SetUpDocElementContainingBlock(nsIContent* aDocElement);
+  void SetUpDocElementContainingBlock(nsIContent* aDocElement);
 
   /**
    * CreateAttributeContent creates a single content/frame combination for an
    * |attr(foo)| generated content.
    *
    * @param aParentContent the parent content for the generated content
    * @param aParentFrame the parent frame for the generated frame
    * @param aAttrNamespace the namespace of the attribute in question