Bug 1574046 Part 3 - Make BlockReflowInput::ClearFloats() return ClearFloatsResults. r=dbaron
authorTing-Yu Lin <tlin@mozilla.com>
Wed, 03 Jun 2020 00:14:55 +0000
changeset 535983 b217eaadc9d95c656d4f7cf53ca92d979765503d
parent 535982 4140c68594e37736cad363e832d76ba22b7db8e9
child 535984 2490a3ca2481334d32efb292eaffa81aa61aeddf
push id37515
push usernerli@mozilla.com
push dateWed, 17 Jun 2020 14:49:45 +0000
treeherdermozilla-central@1e3e996bb9a1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1574046
milestone79.0a1
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
Bug 1574046 Part 3 - Make BlockReflowInput::ClearFloats() return ClearFloatsResults. r=dbaron This change doesn't change the behavior yet. It just changes all the callers by having them catch the ClearFloatsResults. Some of the callers will be revised in next patch by utilizing the returned results. Some of the return values are not being used, and may produce warnings, they will be suppressed in the next patch, too. Differential Revision: https://phabricator.services.mozilla.com/D74538
layout/generic/BlockReflowInput.cpp
layout/generic/BlockReflowInput.h
layout/generic/nsBlockFrame.cpp
--- a/layout/generic/BlockReflowInput.cpp
+++ b/layout/generic/BlockReflowInput.cpp
@@ -705,17 +705,18 @@ bool BlockReflowInput::FlowAndPlaceFloat
   // ``above'' another float that preceded it in the flow.
   mBCoord = std::max(FloatManager()->GetLowestFloatTop(), mBCoord);
 
   // 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 (StyleClear::None != floatDisplay->mBreakType) {
     // XXXldb Does this handle vertical margins correctly?
-    mBCoord = ClearFloats(mBCoord, floatDisplay->mBreakType);
+    auto [bCoord, result] = ClearFloats(mBCoord, floatDisplay->mBreakType);
+    mBCoord = bCoord;
   }
   // Get the band of available space with respect to margin box.
   nsFlowAreaRect floatAvailableSpace =
       GetFloatAvailableSpaceForPlacingFloat(mBCoord);
   LogicalRect adjustedAvailableSpace = mBlock->AdjustFloatAvailableSpace(
       *this, floatAvailableSpace.mRect, aFloat);
 
   NS_ASSERTION(aFloat->GetParent() == mBlock, "Float frame has wrong parent");
@@ -1044,48 +1045,48 @@ void BlockReflowInput::PlaceBelowCurrent
       delete fc;
       aLine->SetHadFloatPushed();
     }
     fc = next;
   }
   aLine->AppendFloats(mBelowCurrentLineFloats);
 }
 
-nscoord BlockReflowInput::ClearFloats(nscoord aBCoord, StyleClear aBreakType,
-                                      nsIFrame* aReplacedBlock,
-                                      uint32_t aFlags) {
+std::tuple<nscoord, BlockReflowInput::ClearFloatsResult>
+BlockReflowInput::ClearFloats(nscoord aBCoord, StyleClear aBreakType,
+                              nsIFrame* aReplacedBlock, uint32_t aFlags) {
 #ifdef DEBUG
   if (nsBlockFrame::gNoisyReflow) {
     nsFrame::IndentBy(stdout, nsBlockFrame::gNoiseIndent);
     printf("clear floats: in: aBCoord=%d\n", aBCoord);
   }
 #endif
 
 #ifdef NOISY_FLOAT_CLEARING
   printf("BlockReflowInput::ClearFloats: aBCoord=%d breakType=%s\n", aBCoord,
          nsLineBox::BreakTypeToString(aBreakType));
   FloatManager()->List(stdout);
 #endif
 
   if (!FloatManager()->HasAnyFloats()) {
-    return aBCoord;
+    return {aBCoord, ClearFloatsResult::BCoordNoChange};
   }
 
   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...
-      newBCoord = nscoord_MAX;
-    } else {
-      newBCoord = FloatManager()->ClearFloats(newBCoord, aBreakType);
+      return {nscoord_MAX, ClearFloatsResult::FloatsPushedOrSplit};
     }
+
+    newBCoord = FloatManager()->ClearFloats(newBCoord, aBreakType);
   }
 
   if (aReplacedBlock) {
     for (;;) {
       nsFlowAreaRect floatAvailableSpace = GetFloatAvailableSpace(newBCoord);
       if (ReplacedBlockFitsInAvailSpace(aReplacedBlock, floatAvailableSpace)) {
         break;
       }
@@ -1101,10 +1102,13 @@ nscoord BlockReflowInput::ClearFloats(ns
 
 #ifdef DEBUG
   if (nsBlockFrame::gNoisyReflow) {
     nsFrame::IndentBy(stdout, nsBlockFrame::gNoiseIndent);
     printf("clear floats: out: y=%d\n", newBCoord);
   }
 #endif
 
-  return newBCoord;
+  ClearFloatsResult result = newBCoord == aBCoord
+                                 ? ClearFloatsResult::BCoordNoChange
+                                 : ClearFloatsResult::BCoordAdvanced;
+  return {newBCoord, result};
 }
--- a/layout/generic/BlockReflowInput.h
+++ b/layout/generic/BlockReflowInput.h
@@ -4,19 +4,21 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* state used in reflow of block frames */
 
 #ifndef BlockReflowInput_h
 #define BlockReflowInput_h
 
+#include <tuple>
+
+#include "mozilla/ReflowInput.h"
 #include "nsFloatManager.h"
 #include "nsLineBox.h"
-#include "mozilla/ReflowInput.h"
 
 class nsBlockFrame;
 class nsFrameList;
 class nsOverflowContinuationTracker;
 
 namespace mozilla {
 
 // BlockReflowInput contains additional reflow input information that the
@@ -135,18 +137,24 @@ class BlockReflowInput {
 
   bool FlowAndPlaceFloat(nsIFrame* aFloat);
 
   void PlaceBelowCurrentLineFloats(nsLineBox* aLine);
 
   // Returns the first coordinate >= aBCoord that clears the
   // floats indicated by aBreakType and has enough inline size between floats
   // (or no floats remaining) to accomodate aReplacedBlock.
-  nscoord ClearFloats(nscoord aBCoord, mozilla::StyleClear aBreakType,
-                      nsIFrame* aReplacedBlock = nullptr, uint32_t aFlags = 0);
+  enum class ClearFloatsResult : uint8_t {
+    BCoordNoChange,
+    BCoordAdvanced,
+    FloatsPushedOrSplit,
+  };
+  std::tuple<nscoord, ClearFloatsResult> ClearFloats(
+      nscoord aBCoord, mozilla::StyleClear aBreakType,
+      nsIFrame* aReplacedBlock = nullptr, uint32_t aFlags = 0);
 
   nsFloatManager* FloatManager() const {
     MOZ_ASSERT(mReflowInput.mFloatManager,
                "Float manager should be valid during the lifetime of "
                "BlockReflowInput!");
     return mReflowInput.mFloatManager;
   }
 
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -1847,17 +1847,17 @@ void nsBlockFrame::ComputeFinalSize(cons
       blockEndEdgeOfChildren =
           std::min(blockEndEdgeOfChildren + aState.mPrevBEndMargin.get(),
                    aState.mReflowInput.AvailableBSize());
     }
   }
   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.
-    nscoord floatHeight =
+    auto [floatHeight, result] =
         aState.ClearFloats(blockEndEdgeOfChildren, StyleClear::Both, nullptr,
                            nsFloatManager::DONT_CLEAR_PUSHED_FLOATS);
     blockEndEdgeOfChildren = std::max(blockEndEdgeOfChildren, floatHeight);
   }
 
   if (NS_UNCONSTRAINEDSIZE != aReflowInput.ComputedBSize()) {
     finalSize.BSize(wm) =
         ComputeFinalBSize(aReflowInput, aState.mReflowStatus,
@@ -2509,20 +2509,21 @@ void nsBlockFrame::ReflowDirtyLines(Bloc
     // We have to reflow the line if it's a block whose clearance
     // might have changed, so detect that.
     if (!line->IsDirty() &&
         (line->GetBreakTypeBefore() != StyleClear::None || replacedBlock)) {
       nscoord curBCoord = aState.mBCoord;
       // See where we would be after applying any clearance due to
       // BRs.
       if (inlineFloatBreakType != StyleClear::None) {
-        curBCoord = aState.ClearFloats(curBCoord, inlineFloatBreakType);
-      }
-
-      nscoord newBCoord = aState.ClearFloats(
+        std::tie(curBCoord, std::ignore) =
+            aState.ClearFloats(curBCoord, inlineFloatBreakType);
+      }
+
+      auto [newBCoord, result] = aState.ClearFloats(
           curBCoord, line->GetBreakTypeBefore(), replacedBlock);
 
       if (line->HasClearance()) {
         // Reflow the line if it might not have clearance anymore.
         if (newBCoord == curBCoord
             // aState.mBCoord is the clearance point which should be the
             // block-start border-edge of the block frame. If sliding the
             // block by deltaBCoord isn't going to put it in the predicted
@@ -2535,17 +2536,18 @@ void nsBlockFrame::ReflowDirtyLines(Bloc
         if (curBCoord != newBCoord) {
           line->MarkDirty();
         }
       }
     }
 
     // We might have to reflow a line that is after a clearing BR.
     if (inlineFloatBreakType != StyleClear::None) {
-      aState.mBCoord = aState.ClearFloats(aState.mBCoord, inlineFloatBreakType);
+      std::tie(aState.mBCoord, std::ignore) =
+          aState.ClearFloats(aState.mBCoord, inlineFloatBreakType);
       if (aState.mBCoord != line->BStart() + deltaBCoord) {
         // SlideLine is not going to put the line where the clearance
         // put it. Reflow the line to be sure.
         line->MarkDirty();
       }
       inlineFloatBreakType = StyleClear::None;
     }
 
@@ -2804,17 +2806,18 @@ void nsBlockFrame::ReflowDirtyLines(Bloc
       // interrupt, but until we get better at propagating float damage we
       // really do need to do it this way; see comments inside MarkLineDirty.
       MarkLineDirtyForInterrupt(line);
     }
   }
 
   // Handle BR-clearance from the last line of the block
   if (inlineFloatBreakType != StyleClear::None) {
-    aState.mBCoord = aState.ClearFloats(aState.mBCoord, inlineFloatBreakType);
+    std::tie(aState.mBCoord, std::ignore) =
+        aState.ClearFloats(aState.mBCoord, inlineFloatBreakType);
   }
 
   if (needToRecoverState) {
     // Is this expensive?
     aState.ReconstructMarginBefore(line);
 
     // Update aState.mPrevChild as if we had reflowed all of the frames in
     // the last line.
@@ -3474,19 +3477,19 @@ void nsBlockFrame::ReflowBlockFrame(Bloc
 
   // If our block-start margin was counted as part of some parent's block-start
   // margin collapse, and we are being speculatively reflowed assuming this
   // frame DID NOT need clearance, then we need to check that
   // assumption.
   if (!treatWithClearance && !applyBStartMargin && mightClearFloats &&
       aState.mReflowInput.mDiscoveredClearance) {
     nscoord curBCoord = aState.mBCoord + aState.mPrevBEndMargin.get();
-    nscoord clearBCoord =
-        aState.ClearFloats(curBCoord, breakType, replacedBlock);
-    if (clearBCoord != curBCoord) {
+    if (auto [clearBCoord, result] =
+            aState.ClearFloats(curBCoord, breakType, replacedBlock);
+        clearBCoord != curBCoord) {
       // Only record the first frame that requires clearance
       if (!*aState.mReflowInput.mDiscoveredClearance) {
         *aState.mReflowInput.mDiscoveredClearance = frame;
       }
       aState.mPrevChild = frame;
       // Exactly what we do now is flexible since we'll definitely be
       // reflowed.
       return;
@@ -3551,19 +3554,19 @@ void nsBlockFrame::ReflowBlockFrame(Bloc
       if (!treatWithClearance && !clearanceFrame && mightClearFloats) {
         // We don't know if we need clearance and this is the first,
         // optimistic pass.  So determine whether *this block* needs
         // clearance. Note that we do not allow the decision for whether
         // this block has clearance to change on the second pass; that
         // decision is only allowed to be made under the optimistic
         // first pass.
         nscoord curBCoord = aState.mBCoord + aState.mPrevBEndMargin.get();
-        nscoord clearBCoord =
-            aState.ClearFloats(curBCoord, breakType, replacedBlock);
-        if (clearBCoord != curBCoord) {
+        if (auto [clearBCoord, result] =
+                aState.ClearFloats(curBCoord, breakType, replacedBlock);
+            clearBCoord != curBCoord) {
           // Looks like we need clearance and we didn't know about it already.
           // So recompute collapsed margin
           treatWithClearance = true;
           // Remember this decision, needed for incremental reflow
           aLine->SetHasClearance();
 
           // Apply incoming margins
           aState.mBCoord += aState.mPrevBEndMargin.get();
@@ -3580,18 +3583,19 @@ void nsBlockFrame::ReflowBlockFrame(Bloc
       // Temporarily advance the running block-direction value so that the
       // GetFloatAvailableSpace method will return the right available space.
       // This undone as soon as the horizontal margins are computed.
       bStartMargin = aState.mPrevBEndMargin.get();
 
       if (treatWithClearance) {
         nscoord currentBCoord = aState.mBCoord;
         // advance mBCoord to the clear position.
-        aState.mBCoord =
+        auto [clearBCoord, result] =
             aState.ClearFloats(aState.mBCoord, breakType, replacedBlock);
+        aState.mBCoord = clearBCoord;
 
         clearedFloats = aState.mBCoord != currentBCoord;
 
         // Compute clearance. It's the amount we need to add to the block-start
         // border-edge of the frame, after applying collapsed margins
         // from the frame and its children, to get it to line up with
         // the block-end of the floats. The former is
         // currentBCoord + bStartMargin, the latter is the current
@@ -3806,18 +3810,19 @@ void nsBlockFrame::ReflowBlockFrame(Bloc
       if (!aState.ReplacedBlockFitsInAvailSpace(replacedBlock,
                                                 floatAvailableSpace)) {
         // Advance to the next band.
         nscoord newBCoord = aState.mBCoord;
         if (aState.AdvanceToNextBand(floatAvailableSpace.mRect, &newBCoord)) {
           advanced = true;
         }
         // ClearFloats might be able to advance us further once we're there.
-        aState.mBCoord =
+        std::tie(aState.mBCoord, std::ignore) =
             aState.ClearFloats(newBCoord, StyleClear::None, replacedBlock);
+
         // Start over with a new available space rect at the new height.
         floatAvailableSpace = aState.GetFloatAvailableSpaceWithState(
             aState.mBCoord, ShapeType::ShapeOutside, &floatManagerState);
       }
 
       LogicalRect oldAvailSpace(availSpace);
       aState.ComputeBlockAvailSpace(frame, floatAvailableSpace,
                                     replacedBlock != nullptr, availSpace);
@@ -4985,17 +4990,17 @@ bool nsBlockFrame::PlaceLine(BlockReflow
 #endif
     }
     aLine->SetOverflowAreas(lineOverflowAreas);
   }
 
   // Apply break-after clearing if necessary
   // This must stay in sync with |ReflowDirtyLines|.
   if (aLine->HasFloatBreakAfter()) {
-    aState.mBCoord =
+    std::tie(aState.mBCoord, std::ignore) =
         aState.ClearFloats(aState.mBCoord, aLine->GetBreakTypeAfter());
   }
   return true;
 }
 
 void nsBlockFrame::PushLines(BlockReflowInput& aState,
                              nsLineList::iterator aLineBefore) {
   // NOTE: aLineBefore is always a normal line, not an overflow line.
@@ -6694,17 +6699,18 @@ void nsBlockFrame::ReflowPushedFloats(Bl
       // We didn't push |f| so its next-sibling is next.
       next = f->GetNextSibling();
       prev = f;
     }  // else: we did push |f| so |prev|'s new next-sibling is next.
     f = next;
   }
 
   // If there are continued floats, then we may need to continue BR clearance
-  if (0 != aState.ClearFloats(0, StyleClear::Both)) {
+  if (auto [bCoord, result] = aState.ClearFloats(0, StyleClear::Both);
+      0 != bCoord) {
     nsBlockFrame* prevBlock = static_cast<nsBlockFrame*>(GetPrevInFlow());
     if (prevBlock) {
       aState.mFloatBreakType = prevBlock->FindTrailingClear();
     }
   }
 }
 
 void nsBlockFrame::RecoverFloats(nsFloatManager& aFloatManager, WritingMode aWM,