Fix bug 25888 for inline frames other than bullets: redo line reflow when the line's height pushes it into the way of other floats. (Bug 25888) r+sr=roc
authorL. David Baron <dbaron@dbaron.org>
Wed, 20 May 2009 07:21:34 -0400
changeset 28638 be75e40365ce2dfc034cb17d8e33a4007244a013
parent 28637 efc6ec4a3c35396d0f5172a7057ab616d4901673
child 28639 477546db4a9449f342b430541eb4c36ee85de910
push idunknown
push userunknown
push dateunknown
bugs25888
milestone1.9.2a1pre
Fix bug 25888 for inline frames other than bullets: redo line reflow when the line's height pushes it into the way of other floats. (Bug 25888) r+sr=roc
layout/generic/crashtests/25888-1.html
layout/generic/crashtests/25888-2.html
layout/generic/crashtests/crashtests.list
layout/generic/nsBlockFrame.cpp
layout/generic/nsBlockFrame.h
layout/generic/nsBlockReflowState.cpp
layout/generic/nsBlockReflowState.h
layout/generic/nsFloatManager.h
layout/reftests/bugs/reftest.list
layout/reftests/floats/345369-1-ref.html
layout/reftests/floats/345369-1.html
layout/reftests/floats/345369-2-ref.html
layout/reftests/floats/345369-2.html
layout/reftests/floats/345369-3-ref.html
layout/reftests/floats/345369-3.html
layout/reftests/floats/345369-4-ref.html
layout/reftests/floats/345369-4.html
layout/reftests/floats/345369-5-ref.html
layout/reftests/floats/345369-5.html
layout/reftests/floats/reftest.list
new file mode 100644
--- /dev/null
+++ b/layout/generic/crashtests/25888-1.html
@@ -0,0 +1,6 @@
+<title>Hang while developing patch for bug 25888</title>
+
+<div style="width:500px">
+<div style="float:left;width:600px;height:30px"></div>
+Hello
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/generic/crashtests/25888-2.html
@@ -0,0 +1,8 @@
+<!DOCTYPE HTML>
+<title>Testcase for hang while developing bug 25888 (hit on www.flightaware.com)</title>
+
+<div style="border: solid 1em; width: 500px; height: 500px">
+  <div style="float:right; width: 300px; height: 3px;background:yellow;"></div>
+  <div style="float:left; width: 220px; height: 100px;background:aqua;"></div>
+  hi
+</div>
--- a/layout/generic/crashtests/crashtests.list
+++ b/layout/generic/crashtests/crashtests.list
@@ -1,8 +1,10 @@
+load 25888-1.html
+load 25888-2.html
 load 37757-1.html
 load 225868-1.html
 load 264937-1.html
 load 265867-1.html
 load 265867-2.html
 load 289864-1.html
 load 295292-1.html
 load 295292-2.html
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -778,16 +778,30 @@ nsRect
 nsBlockFrame::ComputeTightBounds(gfxContext* aContext) const
 {
   // be conservative
   if (GetStyleContext()->HasTextDecorations())
     return GetOverflowRect();
   return ComputeSimpleTightBounds(aContext);
 }
 
+static PRBool
+AvailableSpaceShrunk(const nsRect& aOldAvailableSpace,
+                     const nsRect& aNewAvailableSpace)
+{
+  if (aNewAvailableSpace.width == 0) {
+    // Positions are not significant if the width is zero.
+    return aOldAvailableSpace.width != 0;
+  }
+  NS_ASSERTION(aOldAvailableSpace.x <= aNewAvailableSpace.x &&
+               aOldAvailableSpace.XMost() >= aNewAvailableSpace.XMost(),
+               "available space should never grow");
+  return aOldAvailableSpace.width != aNewAvailableSpace.width;
+}
+
 static nsSize
 CalculateContainingBlockSizeForAbsolutes(const nsHTMLReflowState& aReflowState,
                                          nsSize aFrameSize)
 {
   // The issue here is that for a 'height' of 'auto' the reflow state
   // code won't know how to calculate the containing block height
   // because it's calculated bottom up. So we use our own computed
   // size as the dimensions.
@@ -1658,21 +1672,21 @@ nsBlockFrame::PropagateFloatDamage(nsBlo
   if (aDeltaY + aState.mReflowState.mBlockDelta != 0) {
     if (aLine->IsBlock()) {
       // Unconditionally reflow sliding blocks; we only really need to reflow
       // if there's a float impacting this block, but the current float manager
       // makes it difficult to check that.  Therefore, we let the child block
       // decide what it needs to reflow.
       aLine->MarkDirty();
     } else {
-      // Note that this check will become incorrect once bug 25888 is fixed
-      // because we are only checking the top of the line
       PRBool wasImpactedByFloat = aLine->IsImpactedByFloat();
       nsFlowAreaRect floatAvailableSpace =
-        aState.GetFloatAvailableSpace(aLine->mBounds.y + aDeltaY, PR_FALSE);
+        aState.GetFloatAvailableSpaceForHeight(aLine->mBounds.y + aDeltaY,
+                                               aLine->mBounds.height,
+                                               nsnull);
 
 #ifdef REALLY_NOISY_REFLOW
     printf("nsBlockFrame::PropagateFloatDamage %p was = %d, is=%d\n", 
            this, wasImpactedByFloat, floatAvailableSpace.mHasFloats);
 #endif
 
       // Mark the line dirty if it was or is affected by a float
       // We actually only really need to reflow if the amount of impact
@@ -3315,21 +3329,29 @@ nsBlockFrame::ReflowBlockFrame(nsBlockRe
 nsresult
 nsBlockFrame::ReflowInlineFrames(nsBlockReflowState& aState,
                                  line_iterator aLine,
                                  PRBool* aKeepReflowGoing)
 {
   nsresult rv = NS_OK;
   *aKeepReflowGoing = PR_TRUE;
 
+  aLine->SetLineIsImpactedByFloat(PR_FALSE);
+
+  // Setup initial coordinate system for reflowing the inline frames
+  // into. Apply a previous block frame's bottom margin first.
+  if (ShouldApplyTopMargin(aState, aLine)) {
+    aState.mY += aState.mPrevBottomMargin.get();
+  }
+  nsFlowAreaRect floatAvailableSpace = aState.GetFloatAvailableSpace();
+
 #ifdef DEBUG
   PRInt32 spins = 0;
 #endif
   LineReflowStatus lineReflowStatus = LINE_REFLOW_REDO_NEXT_BAND;
-  PRBool movedPastFloat = PR_FALSE;
   do {
     PRBool allowPullUp = PR_TRUE;
     nsIContent* forceBreakInContent = nsnull;
     PRInt32 forceBreakOffset = -1;
     gfxBreakPriority forceBreakPriority = eNoBreak;
     do {
       nsFloatManager::SavedState floatManagerState;
       aState.mReflowState.mFloatManager->PushState(&floatManagerState);
@@ -3344,21 +3366,23 @@ nsBlockFrame::ReflowInlineFrames(nsBlock
       nsLineLayout lineLayout(aState.mPresContext,
                               aState.mReflowState.mFloatManager,
                               &aState.mReflowState, &aLine);
       lineLayout.Init(&aState, aState.mMinLineHeight, aState.mLineNumber);
       if (forceBreakInContent) {
         lineLayout.ForceBreakAtPosition(forceBreakInContent, forceBreakOffset);
       }
       rv = DoReflowInlineFrames(aState, lineLayout, aLine,
+                                floatAvailableSpace, &floatManagerState,
                                 aKeepReflowGoing, &lineReflowStatus,
                                 allowPullUp);
       lineLayout.EndLineReflow();
 
       if (LINE_REFLOW_REDO_NO_PULL == lineReflowStatus ||
+          LINE_REFLOW_REDO_MORE_FLOATS == lineReflowStatus ||
           LINE_REFLOW_REDO_NEXT_BAND == lineReflowStatus) {
         if (lineLayout.NeedsBackup()) {
           NS_ASSERTION(!forceBreakInContent, "Backing up twice; this should never be necessary");
           // If there is no saved break position, then this will set
           // set forceBreakInContent to null and we won't back up, which is
           // correct.
           forceBreakInContent = lineLayout.GetLastOptionalBreakPosition(&forceBreakOffset, &forceBreakPriority);
         } else {
@@ -3378,27 +3402,19 @@ nsBlockFrame::ReflowInlineFrames(nsBlock
         printf(": yikes! spinning on a line over 1000 times!\n");
         NS_ABORT();
       }
 #endif
 
       // Don't allow pullup on a subsequent LINE_REFLOW_REDO_NO_PULL pass
       allowPullUp = PR_FALSE;
     } while (NS_SUCCEEDED(rv) && LINE_REFLOW_REDO_NO_PULL == lineReflowStatus);
-
-    if (LINE_REFLOW_REDO_NEXT_BAND == lineReflowStatus) {
-      movedPastFloat = PR_TRUE;
-    }
-  } while (NS_SUCCEEDED(rv) && LINE_REFLOW_REDO_NEXT_BAND == lineReflowStatus);
-
-  // If we did at least one REDO_FOR_FLOAT, then the line did not fit next to some float.
-  // Mark it as impacted by a float, even if it no longer is next to a float.
-  if (movedPastFloat) {
-    aLine->SetLineIsImpactedByFloat(PR_TRUE);
-  }
+  } while (NS_SUCCEEDED(rv) &&
+           (LINE_REFLOW_REDO_MORE_FLOATS == lineReflowStatus ||
+            LINE_REFLOW_REDO_NEXT_BAND == lineReflowStatus));
 
   return rv;
 }
 
 // If at least one float on the line was complete, not at the top of
 // page, but was truncated, then restore the overflow floats to what
 // they were before and push the line.  The floats that will be removed
 // from the list aren't yet known by the block's next in flow.  
@@ -3412,63 +3428,64 @@ nsBlockFrame::PushTruncatedPlaceholderLi
   PushLines(aState, prevLine);
   aKeepReflowGoing = PR_FALSE;
   NS_FRAME_SET_INCOMPLETE(aState.mReflowStatus);
 }
 
 #ifdef DEBUG
 static const char* LineReflowStatusNames[] = {
   "LINE_REFLOW_OK", "LINE_REFLOW_STOP", "LINE_REFLOW_REDO_NO_PULL",
+  "LINE_REFLOW_REDO_MORE_FLOATS",
   "LINE_REFLOW_REDO_NEXT_BAND", "LINE_REFLOW_TRUNCATED"
 };
 #endif
 
 nsresult
 nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState& aState,
                                    nsLineLayout& aLineLayout,
                                    line_iterator aLine,
+                                   nsFlowAreaRect& aFloatAvailableSpace,
+                                   nsFloatManager::SavedState*
+                                     aFloatStateBeforeLine,
                                    PRBool* aKeepReflowGoing,
                                    LineReflowStatus* aLineReflowStatus,
                                    PRBool aAllowPullUp)
 {
   // Forget all of the floats on the line
   aLine->FreeFloats(aState.mFloatCacheFreeList);
   aState.mFloatCombinedArea.SetRect(0, 0, 0, 0);
 
-  // Setup initial coordinate system for reflowing the inline frames
-  // into. Apply a previous block frame's bottom margin first.
-  if (ShouldApplyTopMargin(aState, aLine)) {
-    aState.mY += aState.mPrevBottomMargin.get();
-  }
-  nsFlowAreaRect floatAvailableSpace = aState.GetFloatAvailableSpace();
-  aLine->SetLineIsImpactedByFloat(floatAvailableSpace.mHasFloats);
+  // We need to set this flag on the line if any of our reflow passes
+  // are impacted by floats.
+  if (aFloatAvailableSpace.mHasFloats)
+    aLine->SetLineIsImpactedByFloat(PR_TRUE);
 #ifdef REALLY_NOISY_REFLOW
   printf("nsBlockFrame::DoReflowInlineFrames %p impacted = %d\n",
-         this, floatAvailableSpace.mHasFloats);
+         this, aFloatAvailableSpace.mHasFloats);
 #endif
 
   const nsMargin& borderPadding = aState.BorderPadding();
-  nscoord x = floatAvailableSpace.mRect.x + borderPadding.left;
-  nscoord availWidth = floatAvailableSpace.mRect.width;
+  nscoord x = aFloatAvailableSpace.mRect.x + borderPadding.left;
+  nscoord availWidth = aFloatAvailableSpace.mRect.width;
   nscoord availHeight;
   if (aState.GetFlag(BRS_UNCONSTRAINEDHEIGHT)) {
     availHeight = NS_UNCONSTRAINEDSIZE;
   }
   else {
     /* XXX get the height right! */
-    availHeight = floatAvailableSpace.mRect.height;
+    availHeight = aFloatAvailableSpace.mRect.height;
   }
 
   // Make sure to enable resize optimization before we call BeginLineReflow
   // because it might get disabled there
   aLine->EnableResizeReflowOptimization();
 
   aLineLayout.BeginLineReflow(x, aState.mY,
                               availWidth, availHeight,
-                              floatAvailableSpace.mHasFloats,
+                              aFloatAvailableSpace.mHasFloats,
                               PR_FALSE /*XXX isTopOfPage*/);
 
   aState.SetFlag(BRS_LINE_LAYOUT_EMPTY, PR_FALSE);
 
   // XXX Unfortunately we need to know this before reflowing the first
   // inline frame in the line. FIX ME.
   if ((0 == aLineLayout.GetLineNumber()) &&
       (NS_BLOCK_HAS_FIRST_LETTER_CHILD & mState) &&
@@ -3481,17 +3498,17 @@ nsBlockFrame::DoReflowInlineFrames(nsBlo
   LineReflowStatus lineReflowStatus = LINE_REFLOW_OK;
   PRInt32 i;
   nsIFrame* frame = aLine->mFirstChild;
 
   // Determine whether this is a line of placeholders for out-of-flow
   // continuations
   PRBool isContinuingPlaceholders = PR_FALSE;
 
-  if (floatAvailableSpace.mHasFloats) {
+  if (aFloatAvailableSpace.mHasFloats) {
     // There is a soft break opportunity at the start of the line, because
     // we can always move this line down below float(s).
     if (aLineLayout.NotifyOptionalBreakPosition(frame->GetContent(), 0, PR_TRUE, eNormalBreak)) {
       lineReflowStatus = LINE_REFLOW_REDO_NEXT_BAND;
     }
   }
 
   // need to repeatedly call GetChildCount here, because the child
@@ -3586,63 +3603,67 @@ nsBlockFrame::DoReflowInlineFrames(nsBlo
 
   if (LINE_REFLOW_REDO_NEXT_BAND == lineReflowStatus) {
     // This happens only when we have a line that is impacted by
     // floats and the first element in the line doesn't fit with
     // the floats.
     //
     // What we do is to advance past the first float we find and
     // then reflow the line all over again.
-    NS_ASSERTION(NS_UNCONSTRAINEDSIZE != floatAvailableSpace.mRect.height,
+    NS_ASSERTION(NS_UNCONSTRAINEDSIZE != aFloatAvailableSpace.mRect.height,
                  "unconstrained height on totally empty line");
 
     // See the analogous code for blocks in nsBlockReflowState::ClearFloats.
-    if (floatAvailableSpace.mRect.height > 0) {
-      NS_ASSERTION(floatAvailableSpace.mHasFloats,
+    if (aFloatAvailableSpace.mRect.height > 0) {
+      NS_ASSERTION(aFloatAvailableSpace.mHasFloats,
                    "redo line on totally empty line with non-empty band...");
-      aState.mY += floatAvailableSpace.mRect.height;
+      // We should never hit this case if we've placed floats on the
+      // line; if we have, then the GetFloatAvailableSpace call is wrong
+      // and needs to happen after the caller pops the space manager
+      // state.
+      aState.mFloatManager->AssertStateMatches(aFloatStateBeforeLine);
+      aState.mY += aFloatAvailableSpace.mRect.height;
+      aFloatAvailableSpace = aState.GetFloatAvailableSpace();
     } else {
       NS_ASSERTION(NS_UNCONSTRAINEDSIZE != aState.mReflowState.availableHeight,
                    "We shouldn't be running out of height here");
       if (NS_UNCONSTRAINEDSIZE == aState.mReflowState.availableHeight) {
         // just move it down a bit to try to get out of this mess
         aState.mY += 1;
+        // We should never hit this case if we've placed floats on the
+        // line; if we have, then the GetFloatAvailableSpace call is wrong
+        // and needs to happen after the caller pops the space manager
+        // state.
+        aState.mFloatManager->AssertStateMatches(aFloatStateBeforeLine);
+        aFloatAvailableSpace = aState.GetFloatAvailableSpace();
       } else {
         // There's nowhere to retry placing the line. Just treat it as if
         // we placed the float but it was truncated so we need this line
         // to go to the next page/column.
         lineReflowStatus = LINE_REFLOW_TRUNCATED;
         // Push the line that didn't fit
         PushTruncatedPlaceholderLine(aState, aLine, *aKeepReflowGoing);
       }
     }
-      
-    // We don't want to advance by the bottom margin anymore (we did it
-    // once at the beginning of this function, which will just be called
-    // again), and we certainly don't want to go back if it's negative
-    // (infinite loop, bug 153429).
-    aState.mPrevBottomMargin.Zero();
 
     // XXX: a small optimization can be done here when paginating:
     // if the new Y coordinate is past the end of the block then
     // push the line and return now instead of later on after we are
     // past the float.
   }
-  else if (LINE_REFLOW_REDO_NO_PULL == lineReflowStatus) {
-    // We don't want to advance by the bottom margin anymore (we did it
-    // once at the beginning of this function, which will just be called
-    // again), and we certainly don't want to go back if it's negative
-    // (infinite loop, bug 153429).
-    aState.mPrevBottomMargin.Zero();
-  }
-  else if (LINE_REFLOW_TRUNCATED != lineReflowStatus) {
+  else if (LINE_REFLOW_TRUNCATED != lineReflowStatus &&
+           LINE_REFLOW_REDO_NO_PULL != lineReflowStatus) {
     // If we are propagating out a break-before status then there is
     // no point in placing the line.
     if (!NS_INLINE_IS_BREAK_BEFORE(aState.mReflowStatus)) {
-      PlaceLine(aState, aLineLayout, aLine, aKeepReflowGoing);
+      if (!PlaceLine(aState, aLineLayout, aLine, aFloatStateBeforeLine,
+                     aFloatAvailableSpace.mRect, aKeepReflowGoing)) {
+        lineReflowStatus = LINE_REFLOW_REDO_MORE_FLOATS;
+        // PlaceLine already called GetAvailableSpaceForHeight for us.
+      }
     }
   }
 #ifdef DEBUG
   if (gNoisyReflow) {
     printf("Line reflow status = %s\n", LineReflowStatusNames[lineReflowStatus]);
   }
 #endif
   *aLineReflowStatus = lineReflowStatus;
@@ -4012,20 +4033,22 @@ nsBlockFrame::ShouldJustifyLine(nsBlockR
     }
     nextInFlow = (nsBlockFrame*) nextInFlow->GetNextInFlow();
   }
 
   // This is the last line - so don't allow justification
   return PR_FALSE;
 }
 
-void
+PRBool
 nsBlockFrame::PlaceLine(nsBlockReflowState& aState,
                         nsLineLayout&       aLineLayout,
                         line_iterator       aLine,
+                        nsFloatManager::SavedState *aFloatStateBeforeLine,
+                        nsRect&             aFloatAvailableSpace,
                         PRBool*             aKeepReflowGoing)
 {
   // Trim extra white-space from the line before placing the frames
   aLineLayout.TrimTrailingWhiteSpace();
 
   // Vertically align the frames on this line.
   //
   // According to the CSS2 spec, section 12.6.1, the "marker" box
@@ -4044,16 +4067,36 @@ nsBlockFrame::PlaceLine(nsBlockReflowSta
         aLine == mLines.begin().next()))) {
     nsHTMLReflowMetrics metrics;
     ReflowBullet(aState, metrics, aState.mY);
     aLineLayout.AddBulletFrame(mBullet, metrics);
     addedBullet = PR_TRUE;
   }
   aLineLayout.VerticalAlignLine();
 
+  // We want to compare to the available space that we would have had in
+  // the line's height *before* we placed any floats in the line itself.
+  // Floats that are in the line are handled during line reflow (and may
+  // result in floats being pushed to below the line or (I HOPE???) in a
+  // reflow with a forced break position).
+  nsRect oldFloatAvailableSpace(aFloatAvailableSpace);
+  aFloatAvailableSpace = 
+    aState.GetFloatAvailableSpaceForHeight(aLine->mBounds.y,
+                                           aLine->mBounds.height,
+                                           aFloatStateBeforeLine).mRect;
+  NS_ASSERTION(aFloatAvailableSpace.y == oldFloatAvailableSpace.y, "yikes");
+  // Restore the height to the position of the next band.
+  aFloatAvailableSpace.height = oldFloatAvailableSpace.height;
+  // If the available space between the floats is smaller now that we
+  // know the height, return false (and cause another pass with
+  // LINE_REFLOW_REDO_MORE_FLOATS).
+  if (AvailableSpaceShrunk(oldFloatAvailableSpace, aFloatAvailableSpace)) {
+    return PR_FALSE;
+  }
+
 #ifdef DEBUG
   {
     static nscoord lastHeight = 0;
     if (CRAZY_HEIGHT(aLine->mBounds.y)) {
       lastHeight = aLine->mBounds.y;
       if (abs(aLine->mBounds.y - lastHeight) > CRAZY_H/10) {
         nsFrame::ListTag(stdout);
         printf(": line=%p y=%d line.bounds.height=%d\n",
@@ -4138,17 +4181,17 @@ nsBlockFrame::PlaceLine(nsBlockReflowSta
     PushLines(aState, aLine.prev());
 
     // Stop reflow and whack the reflow status if reflow hasn't
     // already been stopped.
     if (*aKeepReflowGoing) {
       NS_FRAME_SET_INCOMPLETE(aState.mReflowStatus);
       *aKeepReflowGoing = PR_FALSE;
     }
-    return;
+    return PR_TRUE;
   }
 
   // May be needed below
   PRBool wasAdjacentWIthTop = aState.IsAdjacentWithTop();
 
   aState.mY = newY;
   
   // Add the already placed current-line floats to the line
@@ -4195,16 +4238,17 @@ nsBlockFrame::PlaceLine(nsBlockReflowSta
 #endif
   }
 
   // Apply break-after clearing if necessary
   // This must stay in sync with |ReflowDirtyLines|.
   if (aLine->HasFloatBreakAfter()) {
     aState.mY = aState.ClearFloats(aState.mY, aLine->GetBreakTypeAfter());
   }
+  return PR_TRUE;
 }
 
 void
 nsBlockFrame::PushLines(nsBlockReflowState&  aState,
                         nsLineList::iterator aLineBefore)
 {
   nsLineList::iterator overBegin(aLineBefore.next());
 
--- a/layout/generic/nsBlockFrame.h
+++ b/layout/generic/nsBlockFrame.h
@@ -45,27 +45,32 @@
 #define nsBlockFrame_h___
 
 #include "nsHTMLContainerFrame.h"
 #include "nsHTMLParts.h"
 #include "nsAbsoluteContainingBlock.h"
 #include "nsLineBox.h"
 #include "nsCSSPseudoElements.h"
 #include "nsStyleSet.h"
+#include "nsFloatManager.h"
 
 enum LineReflowStatus {
   // The line was completely reflowed and fit in available width, and we should
   // try to pull up content from the next line if possible.
   LINE_REFLOW_OK,
   // The line was completely reflowed and fit in available width, but we should
   // not try to pull up content from the next line.
   LINE_REFLOW_STOP,
   // We need to reflow the line again at its current vertical position. The
   // new reflow should not try to pull up any frames from the next line.
   LINE_REFLOW_REDO_NO_PULL,
+  // We need to reflow the line again using the floats from its height
+  // this reflow, since its height made it hit floats that were not
+  // adjacent to its top.
+  LINE_REFLOW_REDO_MORE_FLOATS,
   // We need to reflow the line again at a lower vertical postion where there
   // may be more horizontal space due to different float configuration.
   LINE_REFLOW_REDO_NEXT_BAND,
   // The line did not fit in the available vertical space. Try pushing it to
   // the next page or column if it's not the first line on the current page/column.
   LINE_REFLOW_TRUNCATED
 };
 
@@ -474,21 +479,25 @@ protected:
    * @param aLine            the line to reflow.  can contain a single block frame
    *                         or contain 1 or more inline frames.
    * @param aKeepReflowGoing [OUT] indicates whether the caller should continue to reflow more lines
    */
   nsresult ReflowLine(nsBlockReflowState& aState,
                       line_iterator aLine,
                       PRBool* aKeepReflowGoing);
 
-  // Return PR_TRUE if aLine gets pushed.
-  void PlaceLine(nsBlockReflowState& aState,
-                 nsLineLayout&       aLineLayout,
-                 line_iterator       aLine,
-                 PRBool*             aKeepReflowGoing);
+  // Return false if it needs another reflow because of reduced space
+  // between floats that are next to it (but not next to its top), and
+  // return true otherwise.
+  PRBool PlaceLine(nsBlockReflowState& aState,
+                   nsLineLayout&       aLineLayout,
+                   line_iterator       aLine,
+                   nsFloatManager::SavedState* aFloatStateBeforeLine,
+                   nsRect&             aFloatAvailableSpace, /* in-out */
+                   PRBool*             aKeepReflowGoing);
 
   /**
    * Mark |aLine| dirty, and, if necessary because of possible
    * pull-up, mark the previous line dirty as well. Also invalidates textruns
    * on those lines because the text in the lines might have changed due to
    * addition/removal of frames.
    * @param aLine the line to mark dirty
    * @param aLineList the line list containing that line, null means the line
@@ -517,16 +526,19 @@ protected:
 
   nsresult ReflowInlineFrames(nsBlockReflowState& aState,
                               line_iterator aLine,
                               PRBool* aKeepLineGoing);
 
   nsresult DoReflowInlineFrames(nsBlockReflowState& aState,
                                 nsLineLayout& aLineLayout,
                                 line_iterator aLine,
+                                nsFlowAreaRect& aFloatAvailableSpace,
+                                nsFloatManager::SavedState*
+                                  aFloatStateBeforeLine,
                                 PRBool* aKeepReflowGoing,
                                 LineReflowStatus* aLineReflowStatus,
                                 PRBool aAllowPullUp);
 
   nsresult ReflowInlineFrame(nsBlockReflowState& aState,
                              nsLineLayout& aLineLayout,
                              line_iterator aLine,
                              nsIFrame* aFrame,
--- a/layout/generic/nsBlockReflowState.cpp
+++ b/layout/generic/nsBlockReflowState.cpp
@@ -361,16 +361,48 @@ nsBlockReflowState::GetFloatAvailableSpa
     printf("GetAvailableSpace: band=%d,%d,%d,%d hasfloats=%d\n",
            result.mRect.x, result.mRect.y, result.mRect.width,
            result.mRect.height, result.mHasFloats);
   }
 #endif
   return result;
 }
 
+nsFlowAreaRect
+nsBlockReflowState::GetFloatAvailableSpaceForHeight(
+                      nscoord aY, nscoord aHeight,
+                      nsFloatManager::SavedState *aState) const
+{
+#ifdef DEBUG
+  // Verify that the caller setup the coordinate system properly
+  nscoord wx, wy;
+  mFloatManager->GetTranslation(wx, wy);
+  NS_ASSERTION((wx == mFloatManagerX) && (wy == mFloatManagerY),
+               "bad coord system");
+#endif
+
+  nsFlowAreaRect result =
+    mFloatManager->GetFlowArea(aY - BorderPadding().top, 
+                               nsFloatManager::WIDTH_WITHIN_HEIGHT,
+                               aHeight, mContentArea.width, aState);
+  // Keep the width >= 0 for compatibility with nsSpaceManager.
+  if (result.mRect.width < 0)
+    result.mRect.width = 0;
+
+#ifdef DEBUG
+  if (nsBlockFrame::gNoisyReflow) {
+    nsFrame::IndentBy(stdout, nsBlockFrame::gNoiseIndent);
+    printf("GetAvailableSpaceForHeight: space=%d,%d,%d,%d hasfloats=%d\n",
+           result.mRect.x, result.mRect.y, result.mRect.width,
+           result.mRect.height, result.mHasFloats);
+  }
+#endif
+  return result;
+}
+
 /*
  * Reconstruct the vertical margin before the line |aLine| in order to
  * do an incremental reflow that begins with |aLine| without reflowing
  * the line before it.  |aLine| may point to the fencepost at the end of
  * the line list, and it is used this way since we (for now, anyway)
  * always need to recover margins at the end of a block.
  *
  * The reconstruction involves walking backward through the line list to
--- a/layout/generic/nsBlockReflowState.h
+++ b/layout/generic/nsBlockReflowState.h
@@ -95,16 +95,19 @@ public:
     { return GetFloatAvailableSpace(mY, PR_FALSE); }
   nsFlowAreaRect GetFloatAvailableSpace(nscoord aY,
                                         PRBool aRelaxHeightConstraint) const
     { return GetFloatAvailableSpaceWithState(aY, aRelaxHeightConstraint,
                                              nsnull); }
   nsFlowAreaRect
     GetFloatAvailableSpaceWithState(nscoord aY, PRBool aRelaxHeightConstraint,
                                     nsFloatManager::SavedState *aState) const;
+  nsFlowAreaRect
+    GetFloatAvailableSpaceForHeight(nscoord aY, nscoord aHeight,
+                                    nsFloatManager::SavedState *aState) const;
 
   /*
    * The following functions all return PR_TRUE if they were able to
    * place the float, PR_FALSE if the float did not fit in available
    * space.
    */
   PRBool AddFloat(nsLineLayout&       aLineLayout,
                   nsPlaceholderFrame* aPlaceholderFrame,
--- a/layout/generic/nsFloatManager.h
+++ b/layout/generic/nsFloatManager.h
@@ -222,16 +222,23 @@ public:
   /**
    * Return the coordinate of the lowest float matching aBreakType in this
    * float manager. Returns aY if there are no matching floats.
    *
    * Both aY and the result are relative to the current translation.
    */
   nscoord ClearFloats(nscoord aY, PRUint8 aBreakType) const;
 
+  void AssertStateMatches(SavedState *aState) const
+  {
+    NS_ASSERTION(aState->mX == mX && aState->mY == mY &&
+                 aState->mFloatInfoCount == mFloats.Length(),
+                 "float manager state should match saved state");
+  }
+
 #ifdef DEBUG
   /**
    * Dump the state of the float manager out to a file.
    */
   nsresult List(FILE* out) const;
 #endif
 
 private:
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -31,24 +31,24 @@
 == 18217-zorder-1.html 18217-zorder-ref.html
 == 18217-zorder-2.html 18217-zorder-ref.html
 == 18217-zorder-3.html 18217-zorder-ref-inline.html
 == 18217-zorder-4.html 18217-zorder-ref-inline-table.html
 == 18217-zorder-5.html 18217-zorder-ref-inline-table.html
 == 23604-1.html 23604-1-ref.html
 == 23604-2.html 23604-2-ref.html
 != 24998-1.html 24998-1-ref.html
-fails == 25888-1l.html 25888-1l-ref.html # bug 25888
-fails != 25888-1l.html 25888-1l-notref.html # bug 25888
-fails == 25888-1r.html 25888-1r-ref.html # bug 25888
-fails != 25888-1r.html 25888-1r-notref.html # bug 25888
-fails == 25888-2l.html 25888-2l-ref.html # bug 25888
-fails == 25888-2r.html 25888-2r-ref.html # bug 25888
-fails == 25888-3l.html 25888-3l-ref.html # bug 25888
-fails == 25888-3r.html 25888-3r-ref.html # bug 25888
+== 25888-1l.html 25888-1l-ref.html
+!= 25888-1l.html 25888-1l-notref.html
+== 25888-1r.html 25888-1r-ref.html
+!= 25888-1r.html 25888-1r-notref.html
+== 25888-2l.html 25888-2l-ref.html
+== 25888-2r.html 25888-2r-ref.html
+== 25888-3l.html 25888-3l-ref.html
+== 25888-3r.html 25888-3r-ref.html
 == 28811-1a.html 28811-1-ref.html
 == 28811-1b.html 28811-1-ref.html
 == 28811-2a.html 28811-2-ref.html
 == 28811-2b.html 28811-2-ref.html
 == 40596-1a.html 40596-1-ref.html
 != 40596-1b.html 40596-1-ref.html
 == 40596-1c.html 40596-1-ref.html
 != 40596-1d.html 40596-1-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/floats/345369-1-ref.html
@@ -0,0 +1,9 @@
+<title>Ambiguous line width</title>
+
+<div style="background: yellow; width: 10em; height: 10em; position: absolute">
+  <div style="position: absolute; top: 0; left: 0; height: 0.5em; width: 1em; background: blue"></div>
+  <div style="position: absolute; top: 0.5em; left: 0; height: 2em; width: 10em; background: aqua"></div>
+
+  <span style="position: absolute; top: 2.5em; left: 0; height: 1em; width: 4em; background: maroon;"></span>
+
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/floats/345369-1.html
@@ -0,0 +1,9 @@
+<title>Ambiguous line width</title>
+
+<div style="background: yellow; width: 10em; height: 10em; line-height: 1; font-family: sans-serif;">
+  <div style="float: left; height: 0.5em; width: 1em; background: blue"></div>
+  <div style="float: left; height: 2em; width: 10em; background: aqua"></div>
+
+  <span style="display:inline-block; height: 1em; width: 4em; background: maroon; vertical-align: bottom"></span>
+
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/floats/345369-2-ref.html
@@ -0,0 +1,10 @@
+<title>Ambiguous line width</title>
+
+<div style="background: yellow; width: 10em; height: 10em; position: relative">
+  <div style="position: absolute; top: 0; left: 0; height: 2em; width: 1em; background: blue"></div>
+  <div style="position: absolute; top: 2em; left: 0; height: 2em; width: 5em; background: aqua"></div>
+
+  <span style="position: absolute; top: 0; left: 1em; height: 1em; width: 4em; background: maroon; vertical-align: bottom"></span>
+  <span style="position: absolute; top: 1em; left: 5em; height: 3em; width: 4em; background: maroon; vertical-align: bottom"></span>
+
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/floats/345369-2.html
@@ -0,0 +1,12 @@
+<title>Ambiguous line width</title>
+
+<div style="background: yellow; width: 10em; height: 10em; line-height: 1; font-family: sans-serif;">
+  <div style="float: left; height: 2em; width: 1em; background: blue"></div>
+  <div style="float: left; clear: left; height: 2em; width: 5em; background: aqua"></div>
+
+  <!-- small <span style="font-size: 3em">big</span> -->
+
+  <span style="display:inline-block; height: 1em; width: 4em; background: maroon; vertical-align: bottom"></span>
+  <span style="display:inline-block; height: 3em; width: 4em; background: maroon; vertical-align: bottom"></span>
+
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/floats/345369-3-ref.html
@@ -0,0 +1,10 @@
+<title>Ambiguous line width</title>
+
+<div style="background: yellow; width: 10em; height: 10em; position: relative">
+  <div style="position: absolute; top: 0; left: 0; height: 2em; width: 1em; background: blue"></div>
+  <div style="position: absolute; top: 2em; left: 0; height: 2em; width: 5em; background: aqua"></div>
+
+  <span style="position: absolute; top: 0; left: 1em; height: 1em; width: 4em; background: maroon; vertical-align: bottom"></span>
+  <span style="position: absolute; top: 4em; left: 0; height: 3em; width: 6em; background: maroon; vertical-align: bottom"></span>
+
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/floats/345369-3.html
@@ -0,0 +1,10 @@
+<title>Ambiguous line width</title>
+
+<div style="background: yellow; width: 10em; height: 10em; line-height: 1; font-family: sans-serif;">
+  <div style="float: left; height: 2em; width: 1em; background: blue"></div>
+  <div style="float: left; clear: left; height: 2em; width: 5em; background: aqua"></div>
+
+  <span style="display:inline-block; height: 1em; width: 4em; background: maroon; vertical-align: bottom"></span>
+  <span style="display:inline-block; height: 3em; width: 6em; background: maroon; vertical-align: bottom"></span>
+
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/floats/345369-4-ref.html
@@ -0,0 +1,10 @@
+<title>Ambiguous line width</title>
+
+<div style="background: yellow; width: 10em; height: 10em; position: relative">
+  <div style="position: absolute; top: 0; left: 0; height: 2em; width: 2em; background: blue"></div>
+  <div style="position: absolute; top: 2em; left: 0; height: 2em; width: 5em; background: aqua"></div>
+
+  <span style="position: absolute; top: 0; left: 2em; height: 1em; width: 3em; background: maroon; vertical-align: bottom"></span>
+  <span style="position: absolute; top: 1em; left: 5em; height: 3em; width: 5em; background: maroon; vertical-align: bottom"></span>
+
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/floats/345369-4.html
@@ -0,0 +1,10 @@
+<title>Ambiguous line width</title>
+
+<div style="background: yellow; width: 10em; height: 10em; line-height: 1; font-family: sans-serif;">
+  <div style="float: left; height: 2em; width: 2em; background: blue"></div>
+  <div style="float: left; clear: left; height: 2em; width: 5em; background: aqua"></div>
+
+  <span style="display:inline-block; height: 1em; width: 3em; background: maroon; vertical-align: bottom"></span>
+  <span style="display:inline-block; height: 3em; width: 5em; background: maroon; vertical-align: bottom"></span>
+
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/floats/345369-5-ref.html
@@ -0,0 +1,10 @@
+<title>Ambiguous line width</title>
+
+<div style="background: yellow; width: 10em; height: 10em; position: relative">
+  <div style="position: absolute; top: 0; left: 0; height: 2em; width: 2em; background: blue"></div>
+  <div style="position: absolute; top: 2em; left: 0; height: 2em; width: 5em; background: aqua"></div>
+
+  <span style="position: absolute; top: 0; left: 2em; height: 1em; width: 2em; background: maroon; vertical-align: bottom"></span>
+  <span style="position: absolute; top: 4em; left: 0; height: 3em; width: 6em; background: maroon; vertical-align: bottom"></span>
+
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/floats/345369-5.html
@@ -0,0 +1,10 @@
+<title>Ambiguous line width</title>
+
+<div style="background: yellow; width: 10em; height: 10em; line-height: 1; font-family: sans-serif;">
+  <div style="float: left; height: 2em; width: 2em; background: blue"></div>
+  <div style="float: left; clear: left; height: 2em; width: 5em; background: aqua"></div>
+
+  <span style="display:inline-block; height: 1em; width: 2em; background: maroon; vertical-align: bottom"></span>
+  <span style="display:inline-block; height: 3em; width: 6em; background: maroon; vertical-align: bottom"></span>
+
+</div>
--- a/layout/reftests/floats/reftest.list
+++ b/layout/reftests/floats/reftest.list
@@ -2,10 +2,15 @@
 == other-float-outside-rule-3-right.html other-float-outside-rule-3-right-ref.html
 # Maybe the spec should change here.
 fails == other-float-outside-rule-3-left-2.html other-float-outside-rule-3-left-2-ref.html
 fails == other-float-outside-rule-3-right-2.html other-float-outside-rule-3-right-2-ref.html
 # It's possible the spec should change here too.
 fails == other-float-outside-rule-7-left.html other-float-outside-rule-7-left-ref.html
 fails == other-float-outside-rule-7-right.html other-float-outside-rule-7-right-ref.html
 == float-outside-block-push.html float-outside-block-push-ref.html
-fails == zero-height-float-base.html zero-height-float-ref.html
+== zero-height-float-base.html zero-height-float-ref.html
 fails == zero-height-float.html zero-height-float-ref.html
+fails == 345369-1.html 345369-1-ref.html
+fails == 345369-2.html 345369-2-ref.html
+== 345369-3.html 345369-3-ref.html
+== 345369-4.html 345369-4-ref.html
+== 345369-5.html 345369-5-ref.html