Bug 1574046 Part 5 - Prevent BlockReflowInput::ClearFloat from returning nscoord_MAX. r=dbaron

This effectively makes ClearFloat() act like it always has
DONT_CLEAR_PUSHED_FLOATS. Thus, the flag is no longer needed.

We need to add an early return condition when the block cannot fit in
ReflowBlockFrame(). Because we now don't return nscoord_MAX when floats
are pushed or split, the `availSpace.BSize(wm)` might equal to zero
rather than negative in this case. It's needed by
testing/web-platform/tests/css/CSS2/floats-clear/floats-clear-multicol-003.html
and layout/reftests/pagination/float-clear-003-print.html

Differential Revision: https://phabricator.services.mozilla.com/D74540

layout/generic/BlockReflowInput.cpp

@@ -711,8 +711,13 @@ bool BlockReflowInput::FlowAndPlaceFloat(nsIFrame* aFloat) {
   if (StyleClear::None != floatDisplay->mBreakType) {
     // XXXldb Does this handle vertical margins correctly?
     auto [bCoord, result] = ClearFloats(mBCoord, floatDisplay->mBreakType);
+    if (result == ClearFloatsResult::FloatsPushedOrSplit) {
+      PushFloatPastBreak(aFloat);
+      return false;
+    }
     mBCoord = bCoord;
   }
+
   // Get the band of available space with respect to margin box.
   nsFlowAreaRect floatAvailableSpace =
       GetFloatAvailableSpaceForPlacingFloat(mBCoord);
@@ -1052,7 +1057,7 @@ void BlockReflowInput::PlaceBelowCurrentLineFloats(nsLineBox* aLine) {
 
 std::tuple<nscoord, BlockReflowInput::ClearFloatsResult>
 BlockReflowInput::ClearFloats(nscoord aBCoord, StyleClear aBreakType,
-                              nsIFrame* aReplacedBlock, uint32_t aFlags) {
+                              nsIFrame* aReplacedBlock) {
 #ifdef DEBUG
   if (nsBlockFrame::gNoisyReflow) {
     nsFrame::IndentBy(stdout, nsBlockFrame::gNoiseIndent);
@@ -1073,15 +1078,11 @@ BlockReflowInput::ClearFloats(nscoord aBCoord, StyleClear aBreakType,
   nscoord newBCoord = aBCoord;
 
   if (aBreakType != StyleClear::None) {
-    if (!(aFlags & nsFloatManager::DONT_CLEAR_PUSHED_FLOATS) &&
-        FloatManager()->ClearContinues(aBreakType)) {
-      // FIXME bug 1574046. This makes no sense! we set aState.mBCoord from this
-      // result which will eventually make our parent's size nscoord_MAX!
-      // This is wallpapered over in nsBlockFrame::ComputeFinalSize for now...
-      return {nscoord_MAX, ClearFloatsResult::FloatsPushedOrSplit};
-    }
-
     newBCoord = FloatManager()->ClearFloats(newBCoord, aBreakType);
+
+    if (FloatManager()->ClearContinues(aBreakType)) {
+      return {newBCoord, ClearFloatsResult::FloatsPushedOrSplit};
+    }
   }
 
   if (aReplacedBlock) {

layout/generic/BlockReflowInput.h

@@ -149,7 +149,7 @@ class BlockReflowInput {
   };
   std::tuple<nscoord, ClearFloatsResult> ClearFloats(
       nscoord aBCoord, mozilla::StyleClear aBreakType,
-      nsIFrame* aReplacedBlock = nullptr, uint32_t aFlags = 0);
+      nsIFrame* aReplacedBlock = nullptr);
 
   nsFloatManager* FloatManager() const {
     MOZ_ASSERT(mReflowInput.mFloatManager,

layout/generic/nsBlockFrame.cpp

@@ -1853,10 +1853,11 @@ void nsBlockFrame::ComputeFinalSize(const ReflowInput& aReflowInput,
   if (aState.mFlags.mBlockNeedsFloatManager) {
     // Include the float manager's state to properly account for the
     // block-end margin of any floated elements; e.g., inside a table cell.
-    auto [floatHeight, result] =
-        aState.ClearFloats(blockEndEdgeOfChildren, StyleClear::Both, nullptr,
-                           nsFloatManager::DONT_CLEAR_PUSHED_FLOATS);
-    blockEndEdgeOfChildren = std::max(blockEndEdgeOfChildren, floatHeight);
+    //
+    // Note: The block coordinate returned by ClearFloats is always greater than
+    // or equal to blockEndEdgeOfChildren.
+    std::tie(blockEndEdgeOfChildren, std::ignore) =
+        aState.ClearFloats(blockEndEdgeOfChildren, StyleClear::Both);
   }
 
   if (NS_UNCONSTRAINEDSIZE != aReflowInput.ComputedBSize()) {
@@ -1911,12 +1912,6 @@ void nsBlockFrame::ComputeFinalSize(const ReflowInput& aReflowInput,
   } else {
     NS_ASSERTION(aReflowInput.AvailableBSize() != NS_UNCONSTRAINEDSIZE,
                  "Shouldn't be incomplete if availableBSize is UNCONSTRAINED.");
-    if (aState.mBCoord == nscoord_MAX) {
-      // FIXME bug 1574046.
-      // This should never happen, but it does. nsFloatManager::ClearFloats
-      // returns |nscoord_MAX| when DONT_CLEAR_PUSHED_FLOATS is false.
-      blockEndEdgeOfChildren = aState.mBCoord = aReflowInput.AvailableBSize();
-    }
     nscoord bSize = std::max(aState.mBCoord, aReflowInput.AvailableBSize());
     if (aReflowInput.AvailableBSize() == NS_UNCONSTRAINEDSIZE) {
       // This should never happen, but it does. See bug 414255
@@ -3514,6 +3509,7 @@ void nsBlockFrame::ReflowBlockFrame(BlockReflowInput& aState,
     nscoord bStartMargin = 0;
     bool mayNeedRetry = false;
     bool clearedFloats = false;
+    bool clearedPushedOrSplitFloat = false;
     if (applyBStartMargin) {
       // Precompute the blocks block-start margin value so that we can get the
       // correct available space (there might be a float that's
@@ -3598,6 +3594,8 @@ void nsBlockFrame::ReflowBlockFrame(BlockReflowInput& aState,
         aState.mBCoord = clearBCoord;
 
         clearedFloats = result != ClearFloatsResult::BCoordNoChange;
+        clearedPushedOrSplitFloat =
+            result == ClearFloatsResult::FloatsPushedOrSplit;
 
         // Compute clearance. It's the amount we need to add to the block-start
         // border-edge of the frame, after applying collapsed margins
@@ -3636,17 +3634,13 @@ void nsBlockFrame::ReflowBlockFrame(BlockReflowInput& aState,
     // margins at the top of the page as we ought to, it wouldn't be
     // needed.
     if ((!aState.mReflowInput.mFlags.mIsTopOfPage || clearedFloats) &&
-        availSpace.BSize(wm) < 0) {
+        (availSpace.BSize(wm) < 0 || clearedPushedOrSplitFloat)) {
       // We know already that this child block won't fit on this
       // page/column due to the block-start margin or the clearance.  So we
       // need to get out of here now.  (If we don't, most blocks will handle
       // things fine, and report break-before, but zero-height blocks
       // won't, and will thus make their parent overly-large and force
       // *it* to be pushed in its entirety.)
-      // Doing this means that we also don't need to worry about the
-      // |availSpace.BSize(wm) += bStartMargin| below interacting with
-      // pushed floats (which force nscoord_MAX clearance) to cause a
-      // constrained block size to turn into an unconstrained one.
       aState.mBCoord = startingBCoord;
       aState.mPrevBEndMargin = incomingMargin;
       *aKeepReflowGoing = false;

layout/generic/nsFloatManager.h

@@ -311,11 +311,6 @@ class nsFloatManager {
    *
    * Both aBCoord and the result are relative to the current translation.
    */
-  enum {
-    // Tell ClearFloats not to push to nscoord_MAX when floats have been
-    // pushed to the next page/column.
-    DONT_CLEAR_PUSHED_FLOATS = (1 << 0)
-  };
   nscoord ClearFloats(nscoord aBCoord, mozilla::StyleClear aBreakType) const;
 
   /**