Bug 1291110 Part 4 - Use line BSize to query available space when updating nsLineLayout. r=dbaron
authorTing-Yu Lin <tlin@mozilla.com>
Mon, 22 Aug 2016 19:42:37 +0800
changeset 321891 ae7543286030724b9aa495792c86085265e0f711
parent 321890 5ebd8aebc99f26443b596cb026ab025585a9260d
child 321892 c77ef1e8dc98202a902185f40617612d94872b15
push id21
push usermaklebus@msu.edu
push dateThu, 01 Dec 2016 06:22:08 +0000
reviewersdbaron
bugs1291110
milestone52.0a1
Bug 1291110 Part 4 - Use line BSize to query available space when updating nsLineLayout. r=dbaron In nsBlockFrame::PlaceLine(), we query the float available space by using the line's BSize(), which may cause the line to reflow again due to available space shrunk. The first issue lies in the second reflow. That is, we do not leverage the line's BSize() computed in the first reflow to query the float available space when updating the inline reflow engine in BlockReflowInput::AddFloat(). So some tall inline elements could still overlap the floats as in the first reflow. To solve this, we cache current line's BSize so that it could be used to update the inline reflow engine when redo the line. Another issue is in nsBlockFrame::PlaceLine(). When determined whether the available space is shrunk, we use the float manager's state *before* placing the line. So if current line has floats, they're not considered. To solve this, we use the current set of floats to get the float available space for comparison, and leave the original aFloatAvailableSpace to provide the information when redoing the line. MozReview-Commit-ID: GqqNlphgxYS
layout/generic/BlockReflowInput.cpp
layout/generic/BlockReflowInput.h
layout/generic/nsBlockFrame.cpp
layout/reftests/floats/1291110-1-ref.html
layout/reftests/floats/1291110-1.html
layout/reftests/floats/1291110-2-ref.html
layout/reftests/floats/1291110-2.html
layout/reftests/floats/reftest.list
--- a/layout/generic/BlockReflowInput.cpp
+++ b/layout/generic/BlockReflowInput.cpp
@@ -623,17 +623,23 @@ BlockReflowInput::AddFloat(nsLineLayout*
       (aLineLayout->LineIsEmpty() ||
        mBlock->ComputeFloatISize(*this, floatAvailableSpace, aFloat)
        <= aAvailableISize)) {
     // And then place it
     placed = FlowAndPlaceFloat(aFloat);
     if (placed) {
       // Pass on updated available space to the current inline reflow engine
       WritingMode wm = mReflowInput.GetWritingMode();
-      nsFlowAreaRect floatAvailSpace = GetFloatAvailableSpace(mBCoord);
+      // If we have mLineBSize, we are reflowing the line again due to
+      // LineReflowStatus::RedoMoreFloats. We should use mLineBSize to query the
+      // correct available space.
+      nsFlowAreaRect floatAvailSpace =
+        mLineBSize.isNothing()
+        ? GetFloatAvailableSpace(mBCoord)
+        : GetFloatAvailableSpaceForBSize(mBCoord, mLineBSize.value(), nullptr);
       LogicalRect availSpace(wm, floatAvailSpace.mRect.IStart(wm), mBCoord,
                              floatAvailSpace.mRect.ISize(wm),
                              floatAvailSpace.mRect.BSize(wm));
       aLineLayout->UpdateBand(wm, availSpace, aFloat);
       // Record this float in the current-line list
       mCurrentLineFloats.Append(mFloatCacheFreeList.Alloc(aFloat));
     } else {
       (*aLineLayout->GetLine())->SetHadFloatPushed();
--- a/layout/generic/BlockReflowInput.h
+++ b/layout/generic/BlockReflowInput.h
@@ -371,16 +371,22 @@ public:
 
   Flags mFlags;
 
   StyleClear mFloatBreakType;
 
   // The amount of computed block-direction size "consumed" by previous-in-flows.
   nscoord mConsumedBSize;
 
+  // Cache the current line's BSize if nsBlockFrame::PlaceLine() fails to
+  // place the line. When redoing the line, it will be used to query the
+  // accurate float available space in AddFloat() and
+  // nsBlockFrame::PlaceLine().
+  mozilla::Maybe<nscoord> mLineBSize;
+
 private:
   bool CanPlaceFloat(nscoord aFloatISize,
                      const nsFlowAreaRect& aFloatAvailableSpace);
 
   void PushFloatPastBreak(nsIFrame* aFloat);
 
   void RecoverFloats(nsLineList::iterator aLine, nscoord aDeltaBCoord);
 };
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -3761,16 +3761,17 @@ nsBlockFrame::ReflowInlineFrames(BlockRe
   if (ShouldApplyBStartMargin(aState, aLine, aLine->mFirstChild)) {
     aState.mBCoord += aState.mPrevBEndMargin.get();
   }
   nsFlowAreaRect floatAvailableSpace = aState.GetFloatAvailableSpace();
 
   LineReflowStatus lineReflowStatus;
   do {
     nscoord availableSpaceBSize = 0;
+    aState.mLineBSize.reset();
     do {
       bool allowPullUp = true;
       nsIFrame* forceBreakInFrame = nullptr;
       int32_t forceBreakOffset = -1;
       gfxBreakPriority forceBreakPriority = gfxBreakPriority::eNoBreak;
       do {
         nsFloatManager::SavedState floatManagerState;
         aState.mReflowInput.mFloatManager->PushState(&floatManagerState);
@@ -4464,55 +4465,76 @@ nsBlockFrame::PlaceLine(BlockReflowInput
     ReflowBullet(bullet, aState, metrics, aState.mBCoord);
     NS_ASSERTION(!BulletIsEmpty() || metrics.BSize(wm) == 0,
                  "empty bullet took up space");
     aLineLayout.AddBulletFrame(bullet, metrics);
     addedBullet = true;
   }
   aLineLayout.VerticalAlignLine();
 
-  // We want to compare to the available space that we would have had in
-  // the line's BSize *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).
-  LogicalRect oldFloatAvailableSpace(aFloatAvailableSpace);
+  // We want to consider the floats in the current line when determining
+  // whether the float available space is shrunk. If mLineBSize doesn't
+  // exist, we are in the first pass trying to place the line. Calling
+  // GetFloatAvailableSpace() like we did in BlockReflowInput::AddFloat()
+  // for UpdateBand().
+
+  // floatAvailableSpaceWithOldLineBSize is the float available space with
+  // the old BSize, but including the floats that were added in this line.
+  LogicalRect floatAvailableSpaceWithOldLineBSize =
+    aState.mLineBSize.isNothing()
+    ? aState.GetFloatAvailableSpace(aLine->BStart()).mRect
+    : aState.GetFloatAvailableSpaceForBSize(aLine->BStart(),
+                                            aState.mLineBSize.value(),
+                                            nullptr).mRect;
+
   // As we redo for floats, we can't reduce the amount of BSize we're
   // checking.
   aAvailableSpaceBSize = std::max(aAvailableSpaceBSize, aLine->BSize());
-  aFloatAvailableSpace =
+  LogicalRect floatAvailableSpaceWithLineBSize =
     aState.GetFloatAvailableSpaceForBSize(aLine->BStart(),
                                           aAvailableSpaceBSize,
-                                          aFloatStateBeforeLine).mRect;
-  NS_ASSERTION(aFloatAvailableSpace.BStart(wm) ==
-               oldFloatAvailableSpace.BStart(wm), "yikes");
-  // Restore the BSize to the position of the next band.
-  aFloatAvailableSpace.BSize(wm) = oldFloatAvailableSpace.BSize(wm);
-
-  // Enforce both IStart() and IEnd() never move outwards to prevent
-  // infinite grow-shrink loops.
-  const nscoord iStartDiff =
-    aFloatAvailableSpace.IStart(wm) - oldFloatAvailableSpace.IStart(wm);
-  const nscoord iEndDiff =
-    aFloatAvailableSpace.IEnd(wm) - oldFloatAvailableSpace.IEnd(wm);
-  if (iStartDiff < 0) {
-    aFloatAvailableSpace.IStart(wm) -= iStartDiff;
-    aFloatAvailableSpace.ISize(wm) += iStartDiff;
-  }
-  if (iEndDiff > 0) {
-    aFloatAvailableSpace.ISize(wm) -= iEndDiff;
-  }
+                                          nullptr).mRect;
 
   // If the available space between the floats is smaller now that we
   // know the BSize, return false (and cause another pass with
   // LineReflowStatus::RedoMoreFloats).  We ensure aAvailableSpaceBSize
   // never decreases, which means that we can't reduce the set of floats
   // we intersect, which means that the available space cannot grow.
-  if (AvailableSpaceShrunk(wm, oldFloatAvailableSpace, aFloatAvailableSpace,
-                           false)) {
+  if (AvailableSpaceShrunk(wm, floatAvailableSpaceWithOldLineBSize,
+                           floatAvailableSpaceWithLineBSize, false)) {
+    // Prepare data for redoing the line.
+    aState.mLineBSize = Some(aLine->BSize());
+
+    // Since we want to redo the line, we update aFloatAvailableSpace by
+    // using the aFloatStateBeforeLine, which is the float manager's state
+    // before the line is placed.
+    LogicalRect oldFloatAvailableSpace(aFloatAvailableSpace);
+    aFloatAvailableSpace =
+      aState.GetFloatAvailableSpaceForBSize(aLine->BStart(),
+                                            aAvailableSpaceBSize,
+                                            aFloatStateBeforeLine).mRect;
+    NS_ASSERTION(aFloatAvailableSpace.BStart(wm) ==
+                 oldFloatAvailableSpace.BStart(wm), "yikes");
+    // Restore the BSize to the position of the next band.
+    aFloatAvailableSpace.BSize(wm) = oldFloatAvailableSpace.BSize(wm);
+
+    // Enforce both IStart() and IEnd() never move outwards to prevent
+    // infinite grow-shrink loops.
+    const nscoord iStartDiff =
+      aFloatAvailableSpace.IStart(wm) - oldFloatAvailableSpace.IStart(wm);
+    const nscoord iEndDiff =
+      aFloatAvailableSpace.IEnd(wm) - oldFloatAvailableSpace.IEnd(wm);
+    if (iStartDiff < 0) {
+      aFloatAvailableSpace.IStart(wm) -= iStartDiff;
+      aFloatAvailableSpace.ISize(wm) += iStartDiff;
+    }
+    if (iEndDiff > 0) {
+      aFloatAvailableSpace.ISize(wm) -= iEndDiff;
+    }
+
     return false;
   }
 
 #ifdef DEBUG
   if (!GetParent()->IsCrazySizeAssertSuppressed()) {
     static nscoord lastHeight = 0;
     if (CRAZY_SIZE(aLine->BStart())) {
       lastHeight = aLine->BStart();
new file mode 100644
--- /dev/null
+++ b/layout/reftests/floats/1291110-1-ref.html
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<style>
+.container {
+  position: relative;
+  width: 250px;
+  height: 300px;
+  border: 1px solid blue;
+}
+
+.float1 {
+  position: absolute;
+  width: 100px;
+  height: 50px;
+  background-color: green;
+}
+
+.float2 {
+  position: absolute;
+  top: 50px;
+  width: 180px;
+  height: 50px;
+  background-color: lightgreen;
+}
+
+.box {
+  position: absolute;
+  left: 180px;
+  width: 30px;
+  height: 80px;
+  background-color: blue;
+}
+</style>
+
+<div class="container">
+  <div class="float1"></div>
+  <div class="float2"></div>
+  <div class="box"></div>
+</div>
+<p>The test pass if the inline blue box does not overlap any of the green float boxes.</p>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/floats/1291110-1.html
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<style>
+.container {
+  width: 250px;
+  height: 300px;
+  border: 1px solid blue;
+}
+
+.float1 {
+  float: left;
+  width: 100px;
+  height: 50px;
+  background-color: green;
+}
+
+.float2 {
+  float: left;
+  width: 180px;
+  height: 50px;
+  background-color: lightgreen;
+}
+
+.box {
+  display: inline-block;
+  width: 30px;
+  height: 80px;
+  background-color: blue;
+}
+</style>
+
+<div class="container">
+  <div class="float1"></div>
+  <div class="float2"></div>
+  <div class="box"></div>
+</div>
+<p>The test pass if the inline blue box does not overlap any of the green float boxes.</p>
+
new file mode 100644
--- /dev/null
+++ b/layout/reftests/floats/1291110-2-ref.html
@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<style>
+.container {
+  position: relative;
+  width: 250px;
+  height: 300px;
+  border: 1px solid blue;
+}
+
+.float1 {
+  position: absolute;
+  width: 100px;
+  height: 50px;
+  background-color: green;
+}
+
+.float2 {
+  position: absolute;
+  top: 50px;
+  left: 70px;
+  width: 180px;
+  height: 50px;
+  background-color: lightgreen;
+}
+
+.box {
+  position: absolute;
+  top: 50px;
+  width: 30px;
+  height: 80px;
+  background-color: blue;
+}
+</style>
+
+<div class="container">
+  <div class="float1"></div>
+  <div class="float2"></div>
+  <div class="box"></div>
+</div>
+<p>The test pass if the inline blue box does not overlap any of the green float boxes.</p>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/floats/1291110-2.html
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<style>
+.container {
+  width: 250px;
+  height: 300px;
+  border: 1px solid blue;
+}
+
+.float1 {
+  float: left;
+  width: 100px;
+  height: 50px;
+  background-color: green;
+}
+
+.float2 {
+  float: right;
+  width: 180px;
+  height: 50px;
+  background-color: lightgreen;
+}
+
+.box {
+  display: inline-block;
+  width: 30px;
+  height: 80px;
+  background-color: blue;
+}
+</style>
+
+<div class="container">
+  <div class="float1"></div>
+  <div class="float2"></div>
+  <div class="box"></div>
+</div>
+<p>The test pass if the inline blue box does not overlap any of the green float boxes.</p>
--- a/layout/reftests/floats/reftest.list
+++ b/layout/reftests/floats/reftest.list
@@ -5,30 +5,32 @@ fails == other-float-outside-rule-3-left
 fails == other-float-outside-rule-3-right-2.html other-float-outside-rule-3-right-2-ref.html # bug 616334
 fails == other-float-outside-rule-7-left.html other-float-outside-rule-7-left-ref.html # bug 616334
 fails == other-float-outside-rule-7-right.html other-float-outside-rule-7-right-ref.html # bug 616334
 fuzzy-if(gtkWidget,1,10) == float-outside-block-push.html float-outside-block-push-ref.html # bug 815612
 == relative-float-1.html relative-float-1-ref.html
 == relative-float-2.html relative-float-2-ref.html
 == zero-height-float-base.html zero-height-float-ref.html
 fails == zero-height-float.html zero-height-float-ref.html # bug 81710
-fails == 345369-1.html 345369-1-ref.html
+== 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
 == 429974-1.html 429974-1-ref.html
 == 478834-1.html 478834-1-ref.html
 == 546048-1.html 546048-1-ref.html
 == 775350-1.html 775350-1-ref.html
 == 1114329.html 1114329-ref.html
 == 1236745-1.html 1236745-1-ref.html
 == 1260031-1.html?display:table 1260031-1-ref.html
 == 1260031-1.html?display:table-cell 1260031-1-ref.html
 == 1260031-1.html?overflow:hidden 1260031-1-ref.html
+== 1291110-1.html 1291110-1-ref.html
+== 1291110-2.html 1291110-2-ref.html
 == float-in-rtl-1a.html float-in-rtl-1-ref.html
 fuzzy-if(skiaContent,1,27000) == float-in-rtl-1b.html float-in-rtl-1-ref.html
 fuzzy-if(skiaContent,1,27000) == float-in-rtl-1c.html float-in-rtl-1-ref.html
 fuzzy-if(skiaContent,1,27000) == float-in-rtl-1d.html float-in-rtl-1-ref.html
 == float-in-rtl-2a.html float-in-rtl-2-ref.html
 fuzzy-if(skiaContent,1,12000) == float-in-rtl-2b.html float-in-rtl-2-ref.html
 fuzzy-if(skiaContent,1,12000) == float-in-rtl-2c.html float-in-rtl-2-ref.html
 fuzzy-if(skiaContent,1,12000) == float-in-rtl-2d.html float-in-rtl-2-ref.html