Remove aForceFit/aRelaxHeightConstraint concept from float reflow, since we now split floats rather than push them to the next page (and correct propagation of nsHTMLReflowState::mFlags::mIsTopOfPage will force fitting when we need to). (Bug 563584, patch 2) r=roc
authorL. David Baron <dbaron@dbaron.org>
Thu, 05 Aug 2010 21:59:18 -0700
changeset 48978 7974c4944fa5e21d9486861f8cf8bc22f2d6ffd4
parent 48977 a3fd5fc54c46d8b409b02685af457bd44c9c71b1
child 48979 dcf7e5d1b9866512faf0548ea2b467ca98a2372b
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
Remove aForceFit/aRelaxHeightConstraint concept from float reflow, since we now split floats rather than push them to the next page (and correct propagation of nsHTMLReflowState::mFlags::mIsTopOfPage will force fitting when we need to). (Bug 563584, patch 2) r=roc
layout/generic/nsBlockFrame.cpp
layout/generic/nsBlockReflowState.cpp
layout/generic/nsBlockReflowState.h
layout/reftests/bugs/563584-6-columns-ref.html
layout/reftests/bugs/563584-6-columns.html
layout/reftests/bugs/563584-6-printing-ref.html
layout/reftests/bugs/563584-6-printing.html
layout/reftests/bugs/reftest.list
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -4196,30 +4196,26 @@ nsBlockFrame::PlaceLine(nsBlockReflowSta
     // already been stopped.
     if (*aKeepReflowGoing) {
       NS_FRAME_SET_INCOMPLETE(aState.mReflowStatus);
       *aKeepReflowGoing = PR_FALSE;
     }
     return PR_TRUE;
   }
 
-  // May be needed below
-  PRBool wasAdjacentWIthTop = aState.IsAdjacentWithTop();
-
   aState.mY = newY;
   
   // Add the already placed current-line floats to the line
   aLine->AppendFloats(aState.mCurrentLineFloats);
 
   // Any below current line floats to place?
   if (aState.mBelowCurrentLineFloats.NotEmpty()) {
     // Reflow the below-current-line floats, then add them to the
     // lines float list if there aren't any truncated floats.
-    if (aState.PlaceBelowCurrentLineFloats(aState.mBelowCurrentLineFloats,
-                                           wasAdjacentWIthTop)) {
+    if (aState.PlaceBelowCurrentLineFloats(aState.mBelowCurrentLineFloats)) {
       aLine->AppendFloats(aState.mBelowCurrentLineFloats);
     }
     else { 
       // At least one float is truncated, so fix up any placeholders that got split and
       // push the line. XXX It may be better to put the float on the next line, but this
       // is not common enough to justify the complexity. Or maybe it is now...
       PushTruncatedPlaceholderLine(aState, aLine, *aKeepReflowGoing);
     }
@@ -6594,17 +6590,17 @@ nsBlockFrame::ReflowBullet(nsBlockReflow
   mBullet->Reflow(aState.mPresContext, aMetrics, reflowState, status);
 
   // Get the float available space using our saved state from before we
   // started reflowing the block, so that we ignore any floats inside
   // the block.
   // FIXME: aLineTop isn't actually set correctly by some callers, since
   // they reposition the line.
   nsRect floatAvailSpace =
-    aState.GetFloatAvailableSpaceWithState(aLineTop, PR_FALSE,
+    aState.GetFloatAvailableSpaceWithState(aLineTop,
                                            &aState.mFloatManagerStateBefore)
           .mRect;
   // FIXME (bug 25888): need to check the entire region that the first
   // line overlaps, not just the top pixel.
 
   // Place the bullet now.  We want to place the bullet relative to the
   // border-box of the associated block (using the right/left margin of
   // the bullet frame as separation).  However, if a line box would be
--- a/layout/generic/nsBlockReflowState.cpp
+++ b/layout/generic/nsBlockReflowState.cpp
@@ -320,33 +320,32 @@ nsBlockReflowState::ComputeBlockAvailSpa
 
 #ifdef REALLY_NOISY_REFLOW
   printf("  CBAS: result %d %d %d %d\n", aResult.x, aResult.y, aResult.width, aResult.height);
 #endif
 }
 
 nsFlowAreaRect
 nsBlockReflowState::GetFloatAvailableSpaceWithState(
-                      nscoord aY, PRBool aRelaxHeightConstraint,
+                      nscoord aY,
                       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::BAND_FROM_POINT,
-                               aRelaxHeightConstraint ? nscoord_MAX
-                                                      : mContentArea.height,
-                               mContentArea.width, aState);
+                               mContentArea.height, 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("GetAvailableSpace: band=%d,%d,%d,%d hasfloats=%d\n",
@@ -573,38 +572,28 @@ nsBlockReflowState::AddFloat(nsLineLayou
       (mBelowCurrentLineFloats.IsEmpty() &&
        (aLineLayout->LineIsEmpty() ||
         mBlock->ComputeFloatWidth(*this, floatAvailableSpace, aFloat)
         <= aAvailableWidth))) {
     nsFloatManager::SavedState floatManagerState;
     mFloatManager->PushState(&floatManagerState);
 
     // And then place it
-    // force it to fit if we're at the top of the block and we can't
-    // break before this
-    PRBool forceFit = !aLineLayout ||
-                      (IsAdjacentWithTop() && !aLineLayout->LineIsBreakable());
-    placed = FlowAndPlaceFloat(aFloat, aReflowStatus, forceFit);
-    NS_ASSERTION(placed || !forceFit,
-                 "If we asked for force-fit, it should have been placed");
-    if (forceFit || (placed && !NS_FRAME_IS_TRUNCATED(aReflowStatus))) {
+    placed = FlowAndPlaceFloat(aFloat, aReflowStatus);
+    if (placed && !NS_FRAME_IS_TRUNCATED(aReflowStatus)) {
       // Pass on updated available space to the current inline reflow engine
-      nsFlowAreaRect floatAvailSpace =
-        GetFloatAvailableSpace(mY, forceFit);
+      nsFlowAreaRect floatAvailSpace = GetFloatAvailableSpace(mY);
       nsRect availSpace(nsPoint(floatAvailSpace.mRect.x + BorderPadding().left,
                                 mY),
                         floatAvailSpace.mRect.Size());
       if (aLineLayout) {
         aLineLayout->UpdateBand(availSpace, aFloat);
         // Record this float in the current-line list
         mCurrentLineFloats.Append(mFloatCacheFreeList.Alloc(aFloat));
       }
-      // If we can't break here, hide the fact that it's truncated
-      // XXX We can probably do this more cleanly
-      aReflowStatus &= ~NS_FRAME_TRUNCATED;
     }
     else {
       if (placed) {
         mFloatManager->PopState(&floatManagerState);
       } else {
         mFloatManager->AssertStateMatches(&floatManagerState);
       }
       if (IsAdjacentWithTop()) {
@@ -632,18 +621,17 @@ nsBlockReflowState::AddFloat(nsLineLayou
   // Restore coordinate system
   mFloatManager->Translate(dx, dy);
 
   return placed;
 }
 
 PRBool
 nsBlockReflowState::CanPlaceFloat(const nsSize& aFloatSize, PRUint8 aFloats,
-                                  const nsFlowAreaRect& aFloatAvailableSpace,
-                                  PRBool aForceFit)
+                                  const nsFlowAreaRect& aFloatAvailableSpace)
 {
   // If the current Y coordinate is not impacted by any floats
   // then by definition the float fits.
   PRBool result = PR_TRUE;
   if (aFloatAvailableSpace.mHasFloats) {
     // XXX We should allow overflow by up to half a pixel here (bug 21193).
     if (aFloatAvailableSpace.mRect.width < aFloatSize.width) {
       // The available width is too narrow (and it's been impacted by a
@@ -704,17 +692,17 @@ nsBlockReflowState::CanPlaceFloat(const 
       // Get the available space at the new Y coordinate
       if (floatAvailableSpace.mRect.height <= 0) {
         // there is no more available space. We lose.
         result = PR_FALSE;
         break;
       }
 
       mY += floatAvailableSpace.mRect.height;
-      floatAvailableSpace = GetFloatAvailableSpace(mY, aForceFit);
+      floatAvailableSpace = GetFloatAvailableSpace(mY);
 
       if (floatAvailableSpace.mHasFloats) {
         if (xa < floatAvailableSpace.mRect.x ||
             xb > floatAvailableSpace.mRect.XMost()) {
           // The float can't go here.
           result = PR_FALSE;
           break;
         }
@@ -732,18 +720,17 @@ nsBlockReflowState::CanPlaceFloat(const 
     mY = saveY;
   }
 
   return result;
 }
 
 PRBool
 nsBlockReflowState::FlowAndPlaceFloat(nsIFrame*       aFloat,
-                                      nsReflowStatus& aReflowStatus,
-                                      PRBool          aForceFit)
+                                      nsReflowStatus& aReflowStatus)
 {
   aReflowStatus = NS_FRAME_COMPLETE;
   // Save away the Y coordinate before placing the float. We will
   // restore mY at the end after placing the float. This is
   // necessary because any adjustments to mY during the float
   // placement are for the float only, not for any non-floating
   // content.
   nscoord saveY = mY;
@@ -761,17 +748,17 @@ nsBlockReflowState::FlowAndPlaceFloat(ns
   // See if the float should clear any preceding floats...
   // XXX We need to mark this float somehow so that it gets reflowed
   // when floats are inserted before it.
   if (NS_STYLE_CLEAR_NONE != floatDisplay->mBreakType) {
     // XXXldb Does this handle vertical margins correctly?
     mY = ClearFloats(mY, floatDisplay->mBreakType);
   }
     // Get the band of available space
-  nsFlowAreaRect floatAvailableSpace = GetFloatAvailableSpace(mY, aForceFit);
+  nsFlowAreaRect floatAvailableSpace = GetFloatAvailableSpace(mY);
 
   NS_ASSERTION(aFloat->GetParent() == mBlock,
                "Float frame has wrong parent");
 
   // Reflow the float
   nsMargin floatMargin; // computed margin
   mBlock->ReflowFloat(*this, floatAvailableSpace.mRect, aFloat,
                       floatMargin, aReflowStatus);
@@ -799,30 +786,30 @@ nsBlockReflowState::FlowAndPlaceFloat(ns
   // block if possible (CSS2 spec section 9.5.1, see the rule list).
   NS_ASSERTION((NS_STYLE_FLOAT_LEFT == floatDisplay->mFloats) ||
 	       (NS_STYLE_FLOAT_RIGHT == floatDisplay->mFloats),
 	       "invalid float type");
 
   // Can the float fit here?
   PRBool keepFloatOnSameLine = PR_FALSE;
 
-  while (!CanPlaceFloat(floatSize, floatDisplay->mFloats, floatAvailableSpace,
-                        aForceFit)) {
+  while (!CanPlaceFloat(floatSize, floatDisplay->mFloats,
+                        floatAvailableSpace)) {
     if (floatAvailableSpace.mRect.height <= 0) {
       // No space, nowhere to put anything.
       mY = saveY;
       return PR_FALSE;
     }
 
     // Nope. try to advance to the next band.
     if (NS_STYLE_DISPLAY_TABLE != floatDisplay->mDisplay ||
           eCompatibility_NavQuirks != mPresContext->CompatibilityMode() ) {
 
       mY += floatAvailableSpace.mRect.height;
-      floatAvailableSpace = GetFloatAvailableSpace(mY, aForceFit);
+      floatAvailableSpace = GetFloatAvailableSpace(mY);
     } else {
       // This quirk matches the one in nsBlockFrame::ReflowFloat
       // IE handles float tables in a very special way
 
       // see if the previous float is also a table and has "align"
       nsFloatCache* fc = mCurrentLineFloats.Head();
       nsIFrame* prevFrame = nsnull;
       while (fc) {
@@ -851,17 +838,17 @@ nsBlockReflowState::FlowAndPlaceFloat(ns
               break;
             }
           }
         }
       }
 
       // the table does not fit anymore in this line so advance to next band 
       mY += floatAvailableSpace.mRect.height;
-      floatAvailableSpace = GetFloatAvailableSpace(mY, aForceFit);
+      floatAvailableSpace = GetFloatAvailableSpace(mY);
       // reflow the float again now since we have more space
       // XXXldb We really don't need to Reflow in a loop, we just need
       // to ComputeSize in a loop (once ComputeSize depends on
       // availableWidth, which should make this work again).
       mBlock->ReflowFloat(*this, floatAvailableSpace.mRect, aFloat,
                           floatMargin, aReflowStatus);
       // Get the floats bounding box and margin information
       floatSize = aFloat->GetSize() +
@@ -979,36 +966,34 @@ nsBlockReflowState::FlowAndPlaceFloat(ns
 
   return PR_TRUE;
 }
 
 /**
  * Place below-current-line floats.
  */
 PRBool
-nsBlockReflowState::PlaceBelowCurrentLineFloats(nsFloatCacheFreeList& aList, PRBool aForceFit)
+nsBlockReflowState::PlaceBelowCurrentLineFloats(nsFloatCacheFreeList& aList)
 {
   nsFloatCache* fc = aList.Head();
   while (fc) {
     {
 #ifdef DEBUG
       if (nsBlockFrame::gNoisyReflow) {
         nsFrame::IndentBy(stdout, nsBlockFrame::gNoiseIndent);
         printf("placing bcl float: ");
         nsFrame::ListTag(stdout, fc->mFloat);
         printf("\n");
       }
 #endif
       // Place the float
       nsReflowStatus reflowStatus;
-      PRBool placed = FlowAndPlaceFloat(fc->mFloat, reflowStatus, aForceFit);
-      NS_ASSERTION(placed || !aForceFit,
-                   "If we're in force-fit mode, we should have placed the float");
+      PRBool placed = FlowAndPlaceFloat(fc->mFloat, reflowStatus);
 
-      if (!placed || (NS_FRAME_IS_TRUNCATED(reflowStatus) && !aForceFit)) {
+      if (!placed || NS_FRAME_IS_TRUNCATED(reflowStatus)) {
         // return before processing all of the floats, since the line will be pushed.
         return PR_FALSE;
       }
       else if (!NS_FRAME_IS_FULLY_COMPLETE(reflowStatus)) {
         // Create a continuation for the incomplete float
         nsresult rv = mBlock->SplitFloat(*this, fc->mFloat, reflowStatus);
         if (NS_FAILED(rv))
           return PR_FALSE;
@@ -1047,18 +1032,17 @@ nsBlockReflowState::ClearFloats(nscoord 
   nscoord newY = aY;
 
   if (aBreakType != NS_STYLE_CLEAR_NONE) {
     newY = bp.top + mFloatManager->ClearFloats(newY - bp.top, aBreakType);
   }
 
   if (aReplacedBlock) {
     for (;;) {
-      nsFlowAreaRect floatAvailableSpace = 
-        GetFloatAvailableSpace(newY, PR_FALSE);
+      nsFlowAreaRect floatAvailableSpace = GetFloatAvailableSpace(newY);
       nsBlockFrame::ReplacedElementWidthToClear replacedWidth =
         nsBlockFrame::WidthToClearPastFloats(*this, floatAvailableSpace.mRect,
                                              aReplacedBlock);
       if (!floatAvailableSpace.mHasFloats ||
           NS_MAX(floatAvailableSpace.mRect.x, replacedWidth.marginLeft) +
             replacedWidth.borderBoxWidth +
             NS_MAX(mContentArea.width -
                      NS_MIN(mContentArea.width,
--- a/layout/generic/nsBlockReflowState.h
+++ b/layout/generic/nsBlockReflowState.h
@@ -82,23 +82,21 @@ public:
    * for the current y coordinate. The available space is relative to
    * our coordinate system, which is the content box, with (0, 0) in the
    * upper left.
    *
    * Returns whether there are floats present at the given vertical
    * coordinate and within the width of the content rect.
    */
   nsFlowAreaRect GetFloatAvailableSpace() const
-    { return GetFloatAvailableSpace(mY, PR_FALSE); }
-  nsFlowAreaRect GetFloatAvailableSpace(nscoord aY,
-                                        PRBool aRelaxHeightConstraint) const
-    { return GetFloatAvailableSpaceWithState(aY, aRelaxHeightConstraint,
-                                             nsnull); }
+    { return GetFloatAvailableSpace(mY); }
+  nsFlowAreaRect GetFloatAvailableSpace(nscoord aY) const
+    { return GetFloatAvailableSpaceWithState(aY, nsnull); }
   nsFlowAreaRect
-    GetFloatAvailableSpaceWithState(nscoord aY, PRBool aRelaxHeightConstraint,
+    GetFloatAvailableSpaceWithState(nscoord aY,
                                     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
@@ -106,22 +104,20 @@ public:
    * aLineLayout is null when we are reflowing float continuations (because
    * they are not associated with a line box).
    */
   PRBool AddFloat(nsLineLayout*       aLineLayout,
                   nsIFrame*           aFloat,
                   nscoord             aAvailableWidth,
                   nsReflowStatus&     aReflowStatus);
   PRBool CanPlaceFloat(const nsSize& aFloatSize, PRUint8 aFloats,
-                       const nsFlowAreaRect& aFloatAvailableSpace,
-                       PRBool aForceFit);
+                       const nsFlowAreaRect& aFloatAvailableSpace);
   PRBool FlowAndPlaceFloat(nsIFrame*       aFloat,
-                           nsReflowStatus& aReflowStatus,
-                           PRBool          aForceFit);
-  PRBool PlaceBelowCurrentLineFloats(nsFloatCacheFreeList& aFloats, PRBool aForceFit);
+                           nsReflowStatus& aReflowStatus);
+  PRBool PlaceBelowCurrentLineFloats(nsFloatCacheFreeList& aFloats);
 
   // Returns the first coordinate >= aY that clears the
   // floats indicated by aBreakType and has enough width between floats
   // (or no floats remaining) to accomodate aReplacedBlock.
   nscoord ClearFloats(nscoord aY, PRUint8 aBreakType,
                       nsIFrame *aReplacedBlock = nsnull);
 
   PRBool IsAdjacentWithTop() const {
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/563584-6-columns-ref.html
@@ -0,0 +1,7 @@
+<!DOCTYPE HTML>
+<title>Testcase for float breaking</title>
+<body style="margin: 0">
+  <div style="position: absolute; top: 0; left: 0; width: 150px; height: 100px; background: blue"></div>
+  <div style="position: absolute; top: 100px; left: 0; width: 75px; height: 200px; background: aqua"></div>
+  <div style="position: absolute; top: 0; left: 200px; width: 100px; height: 350px; background: fuchsia"></div>
+
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/563584-6-columns.html
@@ -0,0 +1,9 @@
+<!DOCTYPE HTML>
+<title>Testcase for float breaking</title>
+<body style="-moz-column-count: 2; -moz-column-gap: 0; margin: 0; width: 400px; height: 400px">
+<!-- two columns, each 200px wide and 400px tall -->
+  <div style="float: left; width: 150px; height: 100px; background: blue"></div>
+  <div style="float: left; width: 75px; height: 200px; background: aqua"></div>
+  <!-- test to see where this float goes -->
+  <div style="float: left; width: 100px; height: 350px; background: fuchsia"></div>
+
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/563584-6-printing-ref.html
@@ -0,0 +1,18 @@
+<!DOCTYPE HTML>
+<html class="reftest-print">
+<title>Testcase for float breaking (reference)</title>
+<body style="margin: 0">
+<div style="width: 3.5in; height: 0.5in; background: blue"></div>
+<div>
+  <div style="display: inline-block; vertical-align: top; height: 1in; width: 1in; background: aqua"></div
+  ><div style="display: inline-block; vertical-align: top; height: 1in; width: 2in; background: fuchsia"></div>
+</div>
+<div>
+  <div style="display: inline-block; vertical-align: top; height: 0.5in; width: 1in"></div
+  ><div style="display: inline-block; vertical-align: top; height: 0.5in; width: 2in; background: fuchsia"></div>
+</div>
+<div style="page-break-before: always">
+  <!-- FIXME: uncomment this when bug 511551 is fixed
+  <div style="display: inline-block; vertical-align: top; height: 0.25in; width: 1in"></div
+  >--><div style="display: inline-block; vertical-align: top; height: 0.25in; width: 2in; background: fuchsia"></div>
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/563584-6-printing.html
@@ -0,0 +1,9 @@
+<!DOCTYPE HTML>
+<html class="reftest-print">
+<title>Testcase for float breaking</title>
+<body style="margin: 0">
+<div style="float: left; width: 3.5in; height: 0.5in; background: blue"></div>
+<div style="float: left; width: 1in; height: 1in; background: aqua"></div>
+<!-- test to see where this float goes -->
+<div style="float: left; width: 2in; height: 1.75in; background: fuchsia"></div>
+
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1437,16 +1437,18 @@ random-if(!haveTestPlugin) == 546071-1.h
 == 561981-3.html 561981-3-ref.html
 == 561981-4.html 561981-4-ref.html
 == 561981-5.html 561981-5-ref.html
 == 561981-6.html 561981-6-ref.html
 == 561981-7.html 561981-7-ref.html
 == 561981-8.html 561981-8-ref.html
 == 562835-1.html 562835-ref.html
 == 562835-2.html 562835-ref.html
+== 563584-6-columns.html 563584-6-columns-ref.html
+== 563584-6-printing.html 563584-6-printing-ref.html
 == 564054-1.html 564054-1-ref.html
 == 564991-1.html 564991-1-ref.html
 == 565819-1.html 565819-ref.html
 == 565819-2.html 565819-ref.html
 == 569006-1.html 569006-1-ref.html
 == 571281-1a.html 571281-1-ref.html
 == 571281-1b.html 571281-1-ref.html
 == 571281-1c.html 571281-1-ref.html