Bug 368863. Reparent floats properly when placeholders move between inline continuations. r+sr=dbaron

layout/generic/nsBlockFrame.cpp

@@ -1605,10 +1605,11 @@ static PRBool LineHasClear(nsLineBox* aLine) {
  */
 void
 nsBlockFrame::ReparentFloats(nsIFrame* aFirstFrame,
-                             nsBlockFrame* aOldParent, PRBool aFromOverflow) {
+                             nsBlockFrame* aOldParent, PRBool aFromOverflow,
+                             PRBool aReparentSiblings) {
   nsFrameList list;
   nsIFrame* tail = nsnull;
-  aOldParent->CollectFloats(aFirstFrame, list, &tail, aFromOverflow);
+  aOldParent->CollectFloats(aFirstFrame, list, &tail, aFromOverflow, aReparentSiblings);
   if (list.NotEmpty()) {
     for (nsIFrame* f = list.FirstChild(); f; f = f->GetNextSibling()) {
       ReparentFrame(f, aOldParent, this);
@@ -1983,7 +1984,7 @@ nsBlockFrame::ReflowDirtyLines(nsBlockReflowState& aState)
       lastFrame->SetNextSibling(nsnull);
 
       // Reparent floats whose placeholders are in the line.
-      ReparentFloats(toMove->mFirstChild, nextInFlow, collectOverflowFloats);
+      ReparentFloats(toMove->mFirstChild, nextInFlow, collectOverflowFloats, PR_TRUE);
 
       // Add line to our line list
       if (aState.mPrevChild) {
@@ -2344,7 +2345,7 @@ nsBlockFrame::PullFrameFrom(nsBlockReflowState& aState,
 
       // The frame might have (or contain) floats that need to be
       // brought over too.
-      ReparentFloats(frame, aFromContainer, aFromOverflowLine);
+      ReparentFloats(frame, aFromContainer, aFromOverflowLine, PR_TRUE);
     }
 
     // Stop pulling because we found a frame to pull
@@ -3983,7 +3984,7 @@ nsBlockFrame::PushLines(nsBlockReflowState&  aState,
     // Remove floats in the lines from mFloats
     nsFrameList floats;
     nsIFrame* tail = nsnull;
-    CollectFloats(overBegin->mFirstChild, floats, &tail, PR_FALSE);
+    CollectFloats(overBegin->mFirstChild, floats, &tail, PR_FALSE, PR_TRUE);
 
     if (floats.NotEmpty()) {
       // Push the floats onto the front of the overflow out-of-flows list
@@ -6199,7 +6200,7 @@ nsBlockFrame::ReflowBullet(nsBlockReflowState& aState,
 // floats from whatever list they might be in. We don't search descendants
 // that are float containing blocks. The floats must be children of 'this'.
 void nsBlockFrame::CollectFloats(nsIFrame* aFrame, nsFrameList& aList, nsIFrame** aTail,
-                                 PRBool aFromOverflow) {
+                                 PRBool aFromOverflow, PRBool aCollectSiblings) {
   while (aFrame) {
     // Don't descend into float containing blocks.
     if (!aFrame->IsFloatContainingBlock()) {
@@ -6221,9 +6222,11 @@ void nsBlockFrame::CollectFloats(nsIFrame* aFrame, nsFrameList& aList, nsIFrame*
         *aTail = outOfFlowFrame;
       }
 
-      CollectFloats(aFrame->GetFirstChild(nsnull), aList, aTail, aFromOverflow);
+      CollectFloats(aFrame->GetFirstChild(nsnull), aList, aTail, aFromOverflow,
+                    PR_TRUE);
     }
-    
+    if (!aCollectSiblings)
+      break;
     aFrame = aFrame->GetNextSibling();
   }
 }

layout/generic/nsBlockFrame.h

@@ -379,6 +379,11 @@ public:
     */
   nsresult DoRemoveFrame(nsIFrame* aDeletedFrame, PRBool aDestroyFrames = PR_TRUE, 
                          PRBool aRemoveOnlyFluidContinuations = PR_TRUE);
+
+  void ReparentFloats(nsIFrame* aFirstFrame,
+                      nsBlockFrame* aOldParent, PRBool aFromOverflow,
+                      PRBool aReparentSiblings);
+
 protected:
 
   /** grab overflow lines from this block's prevInFlow, and make them
@@ -394,7 +399,7 @@ protected:
   line_iterator RemoveFloat(nsIFrame* aFloat);
 
   void CollectFloats(nsIFrame* aFrame, nsFrameList& aList, nsIFrame** aTail,
-                     PRBool aFromOverflow);
+                     PRBool aFromOverflow, PRBool aCollectFromSiblings);
   // Remove a float, abs, rel positioned frame from the appropriate block's list
   static void DoRemoveOutOfFlowFrame(nsIFrame* aFrame);
 
@@ -508,10 +513,6 @@ protected:
   void PushLines(nsBlockReflowState& aState,
                  nsLineList::iterator aLineBefore);
 
-
-  void ReparentFloats(nsIFrame* aFirstFrame,
-                      nsBlockFrame* aOldParent, PRBool aFromOverflow);
-
   void PropagateFloatDamage(nsBlockReflowState& aState,
                             nsLineBox* aLine,
                             nscoord aDeltaY);

layout/generic/nsColumnSetFrame.cpp

@@ -270,7 +270,8 @@ nsColumnSetFrame::ChooseColumnStrategy(const nsHTMLReflowState& aReflowState)
     }
 
     // Compute extra space and divide it among the columns
-    nscoord extraSpace = availContentWidth - (colWidth*numColumns + colGap*(numColumns - 1));
+    nscoord extraSpace =
+      PR_MAX(0, availContentWidth - (colWidth*numColumns + colGap*(numColumns - 1)));
     nscoord extraToColumns = extraSpace/numColumns;
     colWidth += extraToColumns;
     expectedWidthLeftOver = extraSpace - (extraToColumns*numColumns);

layout/generic/nsFrameList.cpp

@@ -247,37 +247,6 @@ nsFrameList::Split(nsIFrame* aAfterFrame, nsIFrame** aNextFrameResult)
   return PR_FALSE;
 }
 
-nsIFrame*
-nsFrameList::PullFrame(nsIFrame* aParent,
-                       nsIFrame* aLastChild,
-                       nsFrameList& aFromList)
-{
-  NS_PRECONDITION(nsnull != aParent, "null ptr");
-
-  nsIFrame* pulledFrame = nsnull;
-  if (nsnull != aParent) {
-    pulledFrame = aFromList.FirstChild();
-    if (nsnull != pulledFrame) {
-      // Take frame off old list
-      aFromList.RemoveFirstChild();
-
-      // Put it on the end of this list
-      if (nsnull == aLastChild) {
-        NS_ASSERTION(nsnull == mFirstChild, "bad aLastChild");
-        mFirstChild = pulledFrame;
-      }
-      else {
-        aLastChild->SetNextSibling(pulledFrame);
-      }
-      pulledFrame->SetParent(aParent);
-    }
-  }
-#ifdef DEBUG
-  CheckForLoops();
-#endif
-  return pulledFrame;
-}
-
 nsIFrame*
 nsFrameList::LastChild() const
 {

layout/generic/nsFrameList.h

@@ -121,10 +121,6 @@ public:
    */
   void SortByContentOrder();
 
-  nsIFrame* PullFrame(nsIFrame* aParent,
-                      nsIFrame* aLastChild,
-                      nsFrameList& aFromList);
-
   nsIFrame* FirstChild() const {
     return mFirstChild;
   }

layout/generic/nsInlineFrame.cpp

@@ -247,6 +247,58 @@ nsInlineFrame::ComputeSize(nsIRenderingContext *aRenderingContext,
   return nsSize(NS_UNCONSTRAINEDSIZE, NS_UNCONSTRAINEDSIZE);
 }
 
+void
+nsInlineFrame::ReparentFloatsForInlineChild(nsIFrame* aOurLineContainer,
+                                            nsIFrame* aFrame,
+                                            PRBool aReparentSiblings)
+{
+  NS_ASSERTION(aOurLineContainer->GetNextContinuation() ||
+               aOurLineContainer->GetPrevContinuation(),
+               "Don't call this when we have no continuation, it's a waste");
+
+  nsIFrame* ancestor = aFrame;
+  nsIFrame* ancestorBlockChild;
+  do {
+    ancestorBlockChild = ancestor;
+    ancestor = ancestor->GetParent();
+    if (!ancestor)
+      return;
+  } while (!ancestor->IsFloatContainingBlock());
+
+  if (ancestor == aOurLineContainer)
+    return;
+
+  nsBlockFrame* ourBlock;
+  nsresult rv = aOurLineContainer->QueryInterface(kBlockFrameCID, (void**)&ourBlock);
+  NS_ASSERTION(NS_SUCCEEDED(rv), "Not a block, but broke vertically?");
+  nsBlockFrame* frameBlock;
+  rv = ancestor->QueryInterface(kBlockFrameCID, (void**)&frameBlock);
+  NS_ASSERTION(NS_SUCCEEDED(rv), "ancestor not a block");
+
+  nsFrameList blockChildren(ancestor->GetFirstChild(nsnull));
+  PRBool isOverflow = !blockChildren.ContainsFrame(ancestorBlockChild);
+
+  while (PR_TRUE) {
+    ourBlock->ReparentFloats(aFrame, frameBlock, isOverflow, PR_FALSE);
+
+    if (!aReparentSiblings)
+      return;
+    nsIFrame* next = aFrame->GetNextSibling();
+    if (!next)
+      return;
+    if (next->GetParent() == aFrame->GetParent()) {
+      aFrame = next;
+      continue;
+    }
+    // This is paranoid and will hardly ever get hit ... but we can't actually
+    // trust that the frames in the sibling chain all have the same parent,
+    // because lazy reparenting may be going on. If we find a different
+    // parent we need to redo our analysis.
+    ReparentFloatsForInlineChild(aOurLineContainer, next, aReparentSiblings);
+    return;
+  }
+}
+
 NS_IMETHODIMP
 nsInlineFrame::Reflow(nsPresContext*          aPresContext,
                       nsHTMLReflowMetrics&     aMetrics,
@@ -261,7 +313,9 @@ nsInlineFrame::Reflow(nsPresContext*          aPresContext,
 
   PRBool  lazilySetParentPointer = PR_FALSE;
 
-  // Check for an overflow list with our prev-in-flow
+  nsIFrame* lineContainer = aReflowState.mLineLayout->GetLineContainerFrame();
+
+   // Check for an overflow list with our prev-in-flow
   nsInlineFrame* prevInFlow = (nsInlineFrame*)GetPrevInFlow();
   if (nsnull != prevInFlow) {
     nsIFrame* prevOverflowFrames = prevInFlow->GetOverflowFrames(aPresContext, PR_TRUE);
@@ -282,8 +336,11 @@ nsInlineFrame::Reflow(nsPresContext*          aPresContext,
         NS_ASSERTION(mFrames.IsEmpty(), "child list is not empty for initial reflow");
         mFrames.SetFrames(prevOverflowFrames);
         lazilySetParentPointer = PR_TRUE;
-
       } else {
+        // Assign all floats to our block if necessary
+        if (lineContainer && lineContainer->GetPrevContinuation()) {
+          ReparentFloatsForInlineChild(lineContainer, prevOverflowFrames, PR_TRUE);
+        }
         // Insert the new frames at the beginning of the child list
         // and set their parent pointer
         mFrames.InsertFrames(this, nsnull, prevOverflowFrames);
@@ -331,6 +388,7 @@ nsInlineFrame::Reflow(nsPresContext*          aPresContext,
   // aReflowState)
   InlineReflowState irs;
   irs.mPrevFrame = nsnull;
+  irs.mLineContainer = lineContainer;
   irs.mNextInFlow = (nsInlineFrame*) GetNextInFlow();
   irs.mSetParentPointer = lazilySetParentPointer;
 
@@ -393,8 +451,22 @@ nsInlineFrame::ReflowFrames(nsPresContext* aPresContext,
 
     // Check if we should lazily set the child frame's parent pointer
     if (irs.mSetParentPointer) {
+      PRBool havePrevBlock =
+        irs.mLineContainer && irs.mLineContainer->GetPrevContinuation();
+      // If our block is the first in flow, then any floats under the pulled
+      // frame must already belong to our block.
+      if (havePrevBlock) {
+        // This has to happen before we update frame's parent; we need to
+        // know frame's ancestry under its old block.
+        // The blockChildren.ContainsFrame check performed by
+        // ReparentFloatsForInlineChild here may be slow, but we can't
+        // easily avoid it because we don't know where 'frame' originally
+        // came from. If we really really have to optimize this we could
+        // cache whether frame->GetParent() is under its containing blocks
+        // overflowList or not.
+        ReparentFloatsForInlineChild(irs.mLineContainer, frame, PR_FALSE);
+      }
       frame->SetParent(this);
-
       // We also need to check if frame has a next-in-flow. If it does, then set
       // its parent frame pointer, too. Otherwise, if we reflow frame and it's
       // complete we'll fail when deleting its next-in-flow which is no longer
@@ -406,6 +478,9 @@ nsInlineFrame::ReflowFrames(nsPresContext* aPresContext,
         // the continuing child frame must be in our child list as well. If
         // not, then something is wrong
         NS_ASSERTION(mFrames.ContainsFrame(nextInFlow), "unexpected flow");
+        if (havePrevBlock) {
+          ReparentFloatsForInlineChild(irs.mLineContainer, nextInFlow, PR_FALSE);
+        }
         nextInFlow->SetParent(this);
         nextInFlow = nextInFlow->GetNextInFlow();
       }
@@ -567,6 +642,10 @@ nsInlineFrame::ReflowInlineFrame(nsPresContext* aPresContext,
         // remaining child frames in our child list with the wrong parent
         // frame pointer...
         if (irs.mSetParentPointer) {
+          if (irs.mLineContainer && irs.mLineContainer->GetPrevContinuation()) {
+            ReparentFloatsForInlineChild(irs.mLineContainer, aFrame->GetNextSibling(),
+                                         PR_TRUE);
+          }
           for (nsIFrame* f = aFrame->GetNextSibling(); f; f = f->GetNextSibling()) {
             f->SetParent(this);
           }
@@ -635,8 +714,19 @@ nsInlineFrame::PullOneFrame(nsPresContext* aPresContext,
   nsIFrame* frame = nsnull;
   nsInlineFrame* nextInFlow = irs.mNextInFlow;
   while (nsnull != nextInFlow) {
-    frame = mFrames.PullFrame(this, irs.mPrevFrame, nextInFlow->mFrames);
+    frame = nextInFlow->mFrames.FirstChild();
     if (nsnull != frame) {
+      // If our block has no next continuation, then any floats belonging to
+      // the pulled frame must belong to our block already. This check ensures
+      // we do no extra work in the common non-vertical-breaking case.
+      if (irs.mLineContainer && irs.mLineContainer->GetNextContinuation()) {
+        // The blockChildren.ContainsFrame check performed by
+        // ReparentFloatsForInlineChild will be fast because frame's ancestor
+        // will be the first child of its containing block.
+        ReparentFloatsForInlineChild(irs.mLineContainer, frame, PR_FALSE);
+      }
+      nextInFlow->mFrames.RemoveFirstChild();
+      mFrames.InsertFrame(this, irs.mPrevFrame, frame);
       isComplete = PR_FALSE;
       nsHTMLContainerFrame::ReparentFrameView(aPresContext, frame, nextInFlow, this);
       break;
@@ -782,7 +872,8 @@ nsFirstLineFrame::StealFramesFrom(nsIFrame* aFrame)
 }
 
 nsIFrame*
-nsFirstLineFrame::PullOneFrame(nsPresContext* aPresContext, InlineReflowState& irs, PRBool* aIsComplete)
+nsFirstLineFrame::PullOneFrame(nsPresContext* aPresContext, InlineReflowState& irs,
+                               PRBool* aIsComplete)
 {
   nsIFrame* frame = nsInlineFrame::PullOneFrame(aPresContext, irs, aIsComplete);
   if (frame && !GetPrevInFlow()) {
@@ -804,6 +895,8 @@ nsFirstLineFrame::Reflow(nsPresContext* aPresContext,
     return NS_ERROR_INVALID_ARG;
   }
 
+  nsIFrame* lineContainer = aReflowState.mLineLayout->GetLineContainerFrame();
+
   // Check for an overflow list with our prev-in-flow
   nsFirstLineFrame* prevInFlow = (nsFirstLineFrame*)GetPrevInFlow();
   if (nsnull != prevInFlow) {
@@ -811,6 +904,10 @@ nsFirstLineFrame::Reflow(nsPresContext* aPresContext,
     if (prevOverflowFrames) {
       nsFrameList frames(prevOverflowFrames);
       
+      // Assign all floats to our block if necessary
+      if (lineContainer && lineContainer->GetPrevContinuation()) {
+        ReparentFloatsForInlineChild(lineContainer, prevOverflowFrames, PR_TRUE);
+      }
       mFrames.InsertFrames(this, nsnull, prevOverflowFrames);
       ReParentChildListStyle(aPresContext, frames, this);
     }
@@ -830,6 +927,7 @@ nsFirstLineFrame::Reflow(nsPresContext* aPresContext,
   // aReflowState)
   InlineReflowState irs;
   irs.mPrevFrame = nsnull;
+  irs.mLineContainer = lineContainer;
   irs.mNextInFlow = (nsInlineFrame*) GetNextInFlow();
 
   nsresult rv;

layout/generic/nsInlineFrame.h

@@ -160,12 +160,14 @@ protected:
   struct InlineReflowState {
     nsIFrame* mPrevFrame;
     nsInlineFrame* mNextInFlow;
+    nsIFrame*      mLineContainer;
     PRPackedBool mSetParentPointer;  // when reflowing child frame first set its
                                      // parent frame pointer
 
     InlineReflowState()  {
       mPrevFrame = nsnull;
       mNextInFlow = nsnull;
+      mLineContainer = nsnull;
       mSetParentPointer = PR_FALSE;
     };
   };
@@ -186,6 +188,15 @@ protected:
                              nsIFrame* aFrame,
                              nsReflowStatus& aStatus);
 
+  /**
+   * Reparent floats whose placeholders are inline descendants of aFrame from
+   * whatever block they're currently parented by to aOurBlock.
+   * @param aReparentSiblings if this is true, we follow aFrame's
+   * GetNextSibling chain reparenting them all
+   */
+  void ReparentFloatsForInlineChild(nsIFrame* aOurBlock, nsIFrame* aFrame,
+                                    PRBool aReparentSiblings);
+
   virtual nsIFrame* PullOneFrame(nsPresContext* aPresContext,
                                  InlineReflowState& rs,
                                  PRBool* aIsComplete);