Manage float continuations to-be-pulled by the next-in-flow better by keeping them in a separate frame list until they're actually pulled. (Bug 563584, patch 13) r=roc
authorL. David Baron <dbaron@dbaron.org>
Thu, 05 Aug 2010 21:59:19 -0700
changeset 48990 69b9b34abe5825d176c04be037bd0dcb80770cd9
parent 48989 c3f25dd3c232f3aaa97b71d96f20e8493bfbec3e
child 48991 1ff1f54dc043c3779f26878b689a5ce45fea232b
push id14884
push userdbaron@mozilla.com
push dateFri, 06 Aug 2010 05:01:26 +0000
treeherderautoland@8ab7ef79b673 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs563584
milestone2.0b4pre
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
Manage float continuations to-be-pulled by the next-in-flow better by keeping them in a separate frame list until they're actually pulled. (Bug 563584, patch 13) r=roc
content/base/src/nsGkAtomList.h
layout/generic/nsBlockFrame.cpp
layout/generic/nsBlockFrame.h
layout/generic/nsBlockReflowState.cpp
layout/generic/nsBlockReflowState.h
--- a/content/base/src/nsGkAtomList.h
+++ b/content/base/src/nsGkAtomList.h
@@ -374,16 +374,17 @@ GK_ATOM(figcaption, "figcaption")
 GK_ATOM(figure, "figure")
 GK_ATOM(fixed, "fixed")
 GK_ATOM(fixedList, "Fixed-list")
 GK_ATOM(flags, "flags")
 GK_ATOM(flex, "flex")
 GK_ATOM(flexgroup, "flexgroup")
 GK_ATOM(floating, "floating")
 GK_ATOM(floatList, "Float-list")
+GK_ATOM(floatContinuationsList, "FloatContinuations-list")
 GK_ATOM(floor, "floor")
 GK_ATOM(focus, "focus")
 GK_ATOM(following, "following")
 GK_ATOM(followingSibling, "following-sibling")
 GK_ATOM(font, "font")
 GK_ATOM(fontWeight, "font-weight")
 GK_ATOM(fontpicker, "fontpicker")
 GK_ATOM(footer, "footer")
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -559,30 +559,35 @@ nsBlockFrame::GetChildList(nsIAtom* aLis
     return overflowLines ? nsFrameList(overflowLines->front()->mFirstChild,
                                        overflowLines->back()->LastChild())
                          : nsFrameList::EmptyList();
   }
   else if (aListName == nsGkAtoms::overflowOutOfFlowList) {
     const nsFrameList* list = GetOverflowOutOfFlows();
     return list ? *list : nsFrameList::EmptyList();
   }
+  else if (aListName == nsGkAtoms::floatContinuationsList) {
+    const nsFrameList* list = GetFloatContinuations();
+    return list ? *list : nsFrameList::EmptyList();
+  }
   else if (aListName == nsGkAtoms::floatList) {
     return mFloats;
   }
   else if (aListName == nsGkAtoms::bulletList) {
     return HaveOutsideBullet() ? nsFrameList(mBullet, mBullet)
                                : nsFrameList::EmptyList();
   }
   return nsContainerFrame::GetChildList(aListName);
 }
 
 #define NS_BLOCK_FRAME_OVERFLOW_OOF_LIST_INDEX  (NS_CONTAINER_LIST_COUNT_INCL_OC + 0)
 #define NS_BLOCK_FRAME_FLOAT_LIST_INDEX         (NS_CONTAINER_LIST_COUNT_INCL_OC + 1)
 #define NS_BLOCK_FRAME_BULLET_LIST_INDEX        (NS_CONTAINER_LIST_COUNT_INCL_OC + 2)
 #define NS_BLOCK_FRAME_ABSOLUTE_LIST_INDEX      (NS_CONTAINER_LIST_COUNT_INCL_OC + 3)
+#define NS_BLOCK_FRAME_FLOAT_CONTINUATIONS_LIST_INDEX (NS_CONTAINER_LIST_COUNT_INCL_OC + 4)
 // If adding/removing lists, don't forget to update the count in nsBlockFrame.h
 
 nsIAtom*
 nsBlockFrame::GetAdditionalChildListName(PRInt32 aIndex) const
 {
   if (aIndex < NS_CONTAINER_LIST_COUNT_INCL_OC)
     return nsContainerFrame::GetAdditionalChildListName(aIndex);
 
@@ -590,16 +595,18 @@ nsBlockFrame::GetAdditionalChildListName
   case NS_BLOCK_FRAME_FLOAT_LIST_INDEX:
     return nsGkAtoms::floatList;
   case NS_BLOCK_FRAME_BULLET_LIST_INDEX:
     return nsGkAtoms::bulletList;
   case NS_BLOCK_FRAME_OVERFLOW_OOF_LIST_INDEX:
     return nsGkAtoms::overflowOutOfFlowList;
   case NS_BLOCK_FRAME_ABSOLUTE_LIST_INDEX:
     return nsGkAtoms::absoluteList;
+  case NS_BLOCK_FRAME_FLOAT_CONTINUATIONS_LIST_INDEX:
+    return nsGkAtoms::floatContinuationsList;
   default:
     return nsnull;
   }
 }
 
 /* virtual */ PRBool
 nsBlockFrame::IsContainingBlock() const
 {
@@ -1008,21 +1015,16 @@ nsBlockFrame::Reflow(nsPresContext*     
   // Now reflow...
   rv = ReflowDirtyLines(state);
   NS_ASSERTION(NS_SUCCEEDED(rv), "reflow dirty lines failed");
   if (NS_FAILED(rv)) return rv;
 
   NS_MergeReflowStatusInto(&state.mReflowStatus, ocStatus);
   NS_MergeReflowStatusInto(&state.mReflowStatus, fcStatus);
 
-  // Put continued floats at the end of mFloats
-  if (state.mFloatContinuations.NotEmpty()) {
-    mFloats.AppendFrames(nsnull, state.mFloatContinuations);
-  }
-
   // If we end in a BR with clear and affected floats continue,
   // we need to continue, too.
   if (NS_UNCONSTRAINEDSIZE != aReflowState.availableHeight &&
       NS_FRAME_IS_COMPLETE(state.mReflowStatus) &&
       state.mFloatManager->ClearContinues(FindTrailingClear())) {
     NS_FRAME_SET_INCOMPLETE(state.mReflowStatus);
   }
 
@@ -4446,60 +4448,26 @@ nsBlockFrame::DrainOverflowLines(nsBlock
 }
 
 // This function assumes our prev-in-flow has completed reflow and its
 // mFloats may contain frames at the end of its float list, marked with
 // NS_FRAME_IS_FLOAT_CONTINUATION, that should be pulled to this block.
 void
 nsBlockFrame::DrainFloatContinuations(nsBlockReflowState& aState)
 {
-  // Cache any continuations of our own floats that we're still holding onto
-  // so they're out of the way. This should only happen if we're re-Reflow'd
-  // before our next-in-flow gets a chance to pull these continuations.
-  // However, if it's a "continuation" that's not actually a continuation,
-  // put it back on the floats list.
-  // FIXME: This is not compatible with doing float breaking in dynamic
-  // situations, since in those situations we could have current
-  // continuations at the end of our float list that were actually
-  // continuations from a previous frame to this one.  (However, it's
-  // not clear to me that we really need this code in the first place;
-  // the best solution might just be to remove it.)
-  nsIFrame *f = mFloats.LastChild();
-  if (f && (f->GetStateBits() & NS_FRAME_IS_FLOAT_CONTINUATION)) {
-    do {
-      f = f->GetPrevSibling();
-    } while (f && (f->GetStateBits() & NS_FRAME_IS_FLOAT_CONTINUATION));
-    aState.SetupFloatContinuationList();
-    // RemoveFramesAfter(nsnull) removes the whole list
-    nsFrameList floatContinuations = mFloats.RemoveFramesAfter(f);
-    while (floatContinuations.NotEmpty()) {
-      nsIFrame *f = floatContinuations.RemoveFirstChild();
-      if (f->GetPrevContinuation()) {
-        aState.mFloatContinuations.AppendFrame(nsnull, f);
-      } else {
-        f->RemoveStateBits(NS_FRAME_IS_FLOAT_CONTINUATION);
-        mFloats.AppendFrame(nsnull, f);
-      }
-    }
-  }
-
   // Take any continuations we need to take from our prev-in-flow.
   nsBlockFrame* prevBlock = static_cast<nsBlockFrame*>(GetPrevInFlow());
   if (!prevBlock)
     return;
-  f = prevBlock->mFloats.LastChild();
-  if (f && (f->GetStateBits() & NS_FRAME_IS_FLOAT_CONTINUATION)) {
-    do {
-      ReparentFrame(f, prevBlock, this);
-      f = f->GetPrevSibling();
-    } while (f && (f->GetStateBits() & NS_FRAME_IS_FLOAT_CONTINUATION));
-
-    // RemoveFramesAfter(nsnull) removes the whole list
-    nsFrameList floatContinuations = prevBlock->mFloats.RemoveFramesAfter(f);
-    mFloats.InsertFrames(nsnull, nsnull, floatContinuations);
+  nsFrameList *list = prevBlock->RemoveFloatContinuations();
+  if (list) {
+    if (list->NotEmpty()) {
+      mFloats.InsertFrames(nsnull, nsnull, *list);
+    }
+    delete list;
   }
 
 #ifdef DEBUG
   for (nsIFrame* f = mFloats.FirstChild(); f ; f = f->GetNextSibling()) {
     for (nsIFrame* c = f->GetFirstInFlow(); c ; c = c->GetNextInFlow()) {
       NS_ASSERTION(c == f || c->GetParent() != this || !mFloats.ContainsFrame(c),
                    "Two floats with same parent in same floats list, expect weird errors.");
     }
@@ -4591,16 +4559,56 @@ nsBlockFrame::SetOverflowOutOfFlows(cons
   }
   else {
     SetPropTableFrames(PresContext(), new nsFrameList(aList),
                        OverflowOutOfFlowsProperty());
     AddStateBits(NS_BLOCK_HAS_OVERFLOW_OUT_OF_FLOWS);
   }
 }
 
+nsFrameList*
+nsBlockFrame::GetFloatContinuations() const
+{
+  if (!(GetStateBits() & NS_BLOCK_HAS_FLOAT_CONTINUATIONS)) {
+    return nsnull;
+  }
+  nsFrameList* result =
+    static_cast<nsFrameList*>(Properties().Get(FloatContinuationProperty()));
+  NS_ASSERTION(result, "value should always be non-empty when state set");
+  return result;
+}
+
+nsFrameList*
+nsBlockFrame::EnsureFloatContinuations()
+{
+  nsFrameList *result = GetFloatContinuations();
+  if (result)
+    return result;
+
+  result = new nsFrameList;
+  Properties().Set(FloatContinuationProperty(), result);
+  AddStateBits(NS_BLOCK_HAS_FLOAT_CONTINUATIONS);
+
+  return result;
+}
+
+nsFrameList*
+nsBlockFrame::RemoveFloatContinuations()
+{
+  if (!(GetStateBits() & NS_BLOCK_HAS_FLOAT_CONTINUATIONS)) {
+    return nsnull;
+  }
+
+  nsFrameList *result =
+    static_cast<nsFrameList*>(Properties().Remove(FloatContinuationProperty()));
+  RemoveStateBits(NS_BLOCK_HAS_FLOAT_CONTINUATIONS);
+  NS_ASSERTION(result, "value should always be non-empty when state set");
+  return result;
+}
+
 //////////////////////////////////////////////////////////////////////
 // Frame list manipulation routines
 
 NS_IMETHODIMP
 nsBlockFrame::AppendFrames(nsIAtom*  aListName,
                            nsFrameList& aFrameList)
 {
   if (aFrameList.IsEmpty()) {
@@ -5410,18 +5418,17 @@ nsBlockFrame::StealFrame(nsPresContext* 
                          PRBool         aForceNormal)
 {
   NS_PRECONDITION(aPresContext && aChild, "null pointer");
 
   if ((aChild->GetStateBits() & NS_FRAME_OUT_OF_FLOW) &&
       aChild->GetStyleDisplay()->IsFloating()) {
     PRBool removed = mFloats.RemoveFrameIfPresent(aChild);
     if (!removed) {
-      nsFrameList* list = GetPropTableFrames(aPresContext,
-                                             FloatContinuationProperty());
+      nsFrameList* list = GetFloatContinuations();
       if (list) {
         removed = list->RemoveFrameIfPresent(aChild);
       }
     }
     return removed ? NS_OK : NS_ERROR_UNEXPECTED;
   }
 
   if ((aChild->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER)
@@ -6676,18 +6683,17 @@ void nsBlockFrame::CollectFloats(nsIFram
     // Don't descend into float containing blocks.
     if (!aFrame->IsFloatContainingBlock()) {
       nsIFrame *outOfFlowFrame =
         aFrame->GetType() == nsGkAtoms::placeholderFrame ?
           nsLayoutUtils::GetFloatFromPlaceholder(aFrame) : nsnull;
       if (outOfFlowFrame) {
         if (outOfFlowFrame->GetStateBits() & NS_FRAME_IS_FLOAT_CONTINUATION) {
           if (outOfFlowFrame->GetParent() == this) {
-            nsFrameList* list = GetPropTableFrames(PresContext(),
-                                                   FloatContinuationProperty());
+            nsFrameList* list = GetFloatContinuations();
             if (!list || !list->RemoveFrameIfPresent(outOfFlowFrame)) {
               mFloats.RemoveFrame(outOfFlowFrame);
             }
             aList.AppendFrame(nsnull, outOfFlowFrame);
           }
         } else {
           // Make sure that its parent is us. Otherwise we don't want
           // to mess around with it because it belongs to someone
--- a/layout/generic/nsBlockFrame.h
+++ b/layout/generic/nsBlockFrame.h
@@ -79,40 +79,30 @@ class nsBlockInFlowLineIterator;
 class nsBulletFrame;
 class nsLineBox;
 class nsFirstLineFrame;
 class nsIntervalSet;
 /**
  * Child list name indices
  * @see #GetAdditionalChildListName()
  */
-#define NS_BLOCK_LIST_COUNT  (NS_CONTAINER_LIST_COUNT_INCL_OC + 4)
+#define NS_BLOCK_LIST_COUNT  (NS_CONTAINER_LIST_COUNT_INCL_OC + 5)
 
 /**
  * Some invariants:
  * -- The overflow out-of-flows list contains the out-of-
  * flow frames whose placeholders are in the overflow list.
  * -- A given piece of content has at most one placeholder
  * frame in a block's normal child list.
- * -- While a block is being reflowed, it may have a FloatContinuationProperty
- * frame property that points to an nsFrameList in its
- * nsBlockReflowState. This list contains continuations for
+ * -- While a block is being reflowed, and from then until
+ * its next-in-flow is reflowed it may have a
+ * FloatContinuationProperty frame property that points to
+ * an nsFrameList. This list contains continuations for
  * floats whose prev-in-flow is in the block's regular float
- * list. The list is always empty/nonexistent after the
- * block has been reflowed.  (However, after it has been
- * reflowed and before the continuations are moved to the
- * next block, they are temporarily at the end of the
- * block's float list.  FIXME: This temporary storage
- * situation is not compatible with doing float breaking in
- * dynamic cases, since we can't distinguish unflushed
- * temporary storage (floats being transferred from the
- * frame) in a case where we need a second reflow from
- * frames previously transferred to the frame; fixing this
- * would require an additional frame list for this temporary
- * storage.)
+ * list.
  * -- In all these frame lists, if there are two frames for
  * the same content appearing in the list, then the frames
  * appear with the prev-in-flow before the next-in-flow.
  * -- While reflowing a block, its overflow line list
  * will usually be empty but in some cases will have lines
  * (while we reflow the block at its shrink-wrap width).
  * In this case any new overflowing content must be
  * prepended to the overflow lines.
@@ -121,16 +111,17 @@ class nsIntervalSet;
 // see nsHTMLParts.h for the public block state bits
 
 /**
  * Something in the block has changed that requires Bidi resolution to be
  * performed on the block. This flag must be either set on all blocks in a 
  * continuation chain or none of them.
  */
 #define NS_BLOCK_NEEDS_BIDI_RESOLUTION      NS_FRAME_STATE_BIT(20)
+#define NS_BLOCK_HAS_FLOAT_CONTINUATIONS    NS_FRAME_STATE_BIT(21)
 #define NS_BLOCK_HAS_LINE_CURSOR            NS_FRAME_STATE_BIT(24)
 #define NS_BLOCK_HAS_OVERFLOW_LINES         NS_FRAME_STATE_BIT(25)
 #define NS_BLOCK_HAS_OVERFLOW_OUT_OF_FLOWS  NS_FRAME_STATE_BIT(26)
 
 // Set on any block that has descendant frames in the normal
 // flow with 'clear' set to something other than 'none'
 // (including <BR CLEAR="..."> frames)
 #define NS_BLOCK_HAS_CLEAR_CHILDREN         NS_FRAME_STATE_BIT(27)
@@ -161,17 +152,20 @@ public:
   const_line_iterator end_lines() const { return mLines.end(); }
   reverse_line_iterator rbegin_lines() { return mLines.rbegin(); }
   reverse_line_iterator rend_lines() { return mLines.rend(); }
   const_reverse_line_iterator rbegin_lines() const { return mLines.rbegin(); }
   const_reverse_line_iterator rend_lines() const { return mLines.rend(); }
 
   friend nsIFrame* NS_NewBlockFrame(nsIPresShell* aPresShell, nsStyleContext* aContext, PRUint32 aFlags);
 
-  NS_DECLARE_FRAME_PROPERTY(FloatContinuationProperty, nsnull)
+  // This is a child list too, but we let nsBlockReflowState get to it
+  // directly too.
+  NS_DECLARE_FRAME_PROPERTY(FloatContinuationProperty,
+                            nsContainerFrame::DestroyFrameList)
 
   // nsQueryFrame
   NS_DECL_QUERYFRAME
 
   // nsIFrame
   NS_IMETHOD Init(nsIContent*      aContent,
                   nsIFrame*        aParent,
                   nsIFrame*        aPrevInFlow);
@@ -723,16 +717,24 @@ protected:
     nsFrameList* const mPropValue;
     nsBlockFrame* const mBlock;
   };
   friend struct nsAutoOOFFrameList;
 
   nsFrameList* GetOverflowOutOfFlows() const;
   void SetOverflowOutOfFlows(const nsFrameList& aList, nsFrameList* aPropValue);
 
+  // Get the float continuations list
+  nsFrameList* GetFloatContinuations() const;
+  // Get the float continuations list, or if there is not currently one,
+  // make a new empty one.
+  nsFrameList* EnsureFloatContinuations();
+  // Remove and return the float continuations list.
+  nsFrameList* RemoveFloatContinuations();
+
 #ifdef NS_DEBUG
   void VerifyLines(PRBool aFinalCheckOK);
   void VerifyOverflowSituation();
   PRInt32 GetDepth() const;
 #endif
 
   nscoord mMinWidth, mPrefWidth;
 
--- a/layout/generic/nsBlockReflowState.cpp
+++ b/layout/generic/nsBlockReflowState.cpp
@@ -66,16 +66,17 @@ nsBlockReflowState::nsBlockReflowState(c
                                        nsBlockFrame* aFrame,
                                        const nsHTMLReflowMetrics& aMetrics,
                                        PRBool aTopMarginRoot,
                                        PRBool aBottomMarginRoot,
                                        PRBool aBlockNeedsFloatManager)
   : mBlock(aFrame),
     mPresContext(aPresContext),
     mReflowState(aReflowState),
+    mFloatContinuations(nsnull),
     mOverflowTracker(nsnull),
     mPrevBottomMargin(),
     mLineNumber(0),
     mFlags(0),
     mFloatBreakType(NS_STYLE_CLEAR_NONE)
 {
   SetFlag(BRS_ISFIRSTINFLOW, aFrame->GetPrevInFlow() == nsnull);
   SetFlag(BRS_ISOVERFLOWCONTAINER,
@@ -144,30 +145,22 @@ nsBlockReflowState::nsBlockReflowState(c
   mPrevChild = nsnull;
   mCurrentLine = aFrame->end_lines();
 
   mMinLineHeight = aReflowState.CalcLineHeight();
 }
 
 nsBlockReflowState::~nsBlockReflowState()
 {
-  NS_ASSERTION(mFloatContinuations.IsEmpty(),
-               "Leaking float continuation frames");
-
   // Restore the coordinate system, unless the float manager is null,
   // which means it was just destroyed.
   if (mFloatManager) {
     const nsMargin& borderPadding = BorderPadding();
     mFloatManager->Translate(-borderPadding.left, -borderPadding.top);
   }
-
-  if (GetFlag(BRS_PROPTABLE_FLOATCLIST)) {
-    mPresContext->PropertyTable()->
-      Delete(mBlock, nsBlockFrame::FloatContinuationProperty());
-  }
 }
 
 nsLineBox*
 nsBlockReflowState::NewLineBox(nsIFrame* aFrame,
                                PRInt32 aCount,
                                PRBool aIsBlock)
 {
   return NS_NewLineBox(mPresContext->PresShell(), aFrame, aCount, aIsBlock);
@@ -429,20 +422,26 @@ nsBlockReflowState::ReconstructMarginAbo
       break;
     }
   }
 }
 
 void
 nsBlockReflowState::SetupFloatContinuationList()
 {
+  NS_ABORT_IF_FALSE(!GetFlag(BRS_PROPTABLE_FLOATCLIST) == !mFloatContinuations,
+                    "flag mismatch");
   if (!GetFlag(BRS_PROPTABLE_FLOATCLIST)) {
-    mPresContext->PropertyTable()->
-      Set(mBlock, nsBlockFrame::FloatContinuationProperty(),
-          &mFloatContinuations);
+    // If we're being re-Reflow'd without our next-in-flow having been
+    // reflowed, some float continuations from our previous reflow might
+    // still be on our float continuations list.  However, that's
+    // actually fine, since they'll all end up being stolen and
+    // reordered into the correct order again.
+    // FIXME: Check this!
+    mFloatContinuations = mBlock->EnsureFloatContinuations();
     SetFlag(BRS_PROPTABLE_FLOATCLIST, PR_TRUE);
   }
 }
 
 /**
  * Restore information about floats into the float manager for an
  * incremental reflow, and simultaneously push the floats by
  * |aDeltaY|, which is the amount |aLine| was pushed relative to its
--- a/layout/generic/nsBlockReflowState.h
+++ b/layout/generic/nsBlockReflowState.h
@@ -219,27 +219,27 @@ public:
   // The content area to reflow child frames within. The x/y
   // coordinates are known to be mBorderPadding.left and
   // mBorderPadding.top. The width/height may be NS_UNCONSTRAINEDSIZE
   // if the container reflowing this frame has given the frame an
   // unconstrained area.
   nsSize mContentArea;
 
   // Continuation out-of-flow float frames that need to move to our
-  // next in flow are placed here during reflow. At the end of reflow
-  // they move to the end of the mFloats list.
-  nsFrameList mFloatContinuations;
+  // next in flow are placed here during reflow.  It's a pointer to
+  // a frame list stored in the block's property table.
+  nsFrameList *mFloatContinuations;
   // This method makes sure float continuations are accessible to
   // StealFrame. Call it before adding any frames to mFloatContinuations.
   void SetupFloatContinuationList();
   // Use this method to append to mFloatContinuations.
   void AppendFloatContinuation(nsIFrame* aFloatCont) {
     SetupFloatContinuationList();
     aFloatCont->AddStateBits(NS_FRAME_IS_FLOAT_CONTINUATION);
-    mFloatContinuations.AppendFrame(mBlock, aFloatCont);
+    mFloatContinuations->AppendFrame(mBlock, aFloatCont);
   }
 
   // Track child overflow continuations.
   nsOverflowContinuationTracker* mOverflowTracker;
 
   //----------------------------------------
 
   // This state is "running" state updated by the reflow of each line