Split positionAndLayoutFloat() off positionNewFloats().

third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp

Index: third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp b/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
index c3510f973e22afc515e8188f6e701bb74f3f47fb..683eca70140ad26acf44ed55af6cfdef28c7bb2a 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
@@ -3637,118 +3637,127 @@ bool LayoutBlockFlow::positionNewFloats(LineWidth* width) {
     if (floatingObject.layoutObject()->containingBlock() != this)
       continue;
 
-    LayoutBox* childBox = floatingObject.layoutObject();
-
-    // FIXME Investigate if this can be removed. crbug.com/370006
-    childBox->setMayNeedPaintInvalidation();
-
-    LayoutUnit childLogicalLeftMargin = style()->isLeftToRightDirection()
-                                            ? marginStartForChild(*childBox)
-                                            : marginEndForChild(*childBox);
-    if (childBox->style()->clear() & ClearLeft)
-      logicalTop = std::max(lowestFloatLogicalBottom(FloatingObject::FloatLeft),
-                            logicalTop);
-    if (childBox->style()->clear() & ClearRight)
-      logicalTop = std::max(
-          lowestFloatLogicalBottom(FloatingObject::FloatRight), logicalTop);
-
-    bool isPaginated = view()->layoutState()->isPaginated();
-    if (isPaginated && !childrenInline()) {
-      // Forced breaks are inserted at class A break points. Floats may be
-      // affected by a break-after value on the previous in-flow sibling.
-      if (LayoutBox* previousInFlowBox = childBox->previousInFlowSiblingBox())
-        logicalTop =
-            applyForcedBreak(logicalTop, previousInFlowBox->breakAfter());
-    }
+    positionAndLayoutFloat(floatingObject, logicalTop);

[mstensho (USE GERRIT), 2016/11/16 10:18:59]

Oops! Forgot to actually update logicalTop here. positionAndLayoutFloat()
returns a new logical top, but we currently just ignore it.

[mstensho (USE GERRIT), 2016/11/16 10:28:13]

See crbug.com/665804

 
-    LayoutPoint floatLogicalLocation =
-        computeLogicalLocationForFloat(floatingObject, logicalTop);
+    m_floatingObjects->addPlacedObject(floatingObject);
 
-    setLogicalLeftForFloat(floatingObject, floatLogicalLocation.x());
+    if (width)
+      width->shrinkAvailableWidthForNewFloatIfNeeded(floatingObject);
+  }
+  return true;
+}
 
-    setLogicalLeftForChild(*childBox,
-                           floatLogicalLocation.x() + childLogicalLeftMargin);
-    setLogicalTopForChild(
-        *childBox, floatLogicalLocation.y() + marginBeforeForChild(*childBox));
+LayoutUnit LayoutBlockFlow::positionAndLayoutFloat(
+    FloatingObject& floatingObject,
+    LayoutUnit logicalTop) {
+  LayoutBox& childBox = *floatingObject.layoutObject();
 
-    SubtreeLayoutScope layoutScope(*childBox);
-    if (isPaginated && !childBox->needsLayout())
-      markChildForPaginationRelayoutIfNeeded(*childBox, layoutScope);
+  // FIXME Investigate if this can be removed. crbug.com/370006
+  childBox.setMayNeedPaintInvalidation();
 
-    childBox->layoutIfNeeded();
+  LayoutUnit childLogicalLeftMargin = style()->isLeftToRightDirection()
+                                          ? marginStartForChild(childBox)
+                                          : marginEndForChild(childBox);
+  if (childBox.style()->clear() & ClearLeft) {
+    logicalTop = std::max(lowestFloatLogicalBottom(FloatingObject::FloatLeft),
+                          logicalTop);
+  }
+  if (childBox.style()->clear() & ClearRight) {
+    logicalTop = std::max(lowestFloatLogicalBottom(FloatingObject::FloatRight),
+                          logicalTop);
+  }
 
-    if (isPaginated) {
-      LayoutBlockFlow* childBlockFlow =
-          childBox->isLayoutBlockFlow() ? toLayoutBlockFlow(childBox) : nullptr;
-      // The first piece of content inside the child may have set a strut during
-      // layout.
-      LayoutUnit strut =
-          childBlockFlow ? childBlockFlow->paginationStrutPropagatedFromChild()
-                         : LayoutUnit();
+  bool isPaginated = view()->layoutState()->isPaginated();
+  if (isPaginated && !childrenInline()) {
+    // Forced breaks are inserted at class A break points. Floats may be
+    // affected by a break-after value on the previous in-flow sibling.
+    if (LayoutBox* previousInFlowBox = childBox.previousInFlowSiblingBox()) {
+      logicalTop =
+          applyForcedBreak(logicalTop, previousInFlowBox->breakAfter());
+    }
+  }
 
-      LayoutUnit marginBefore = marginBeforeForChild(*childBox);
-      if (marginBefore > LayoutUnit()) {
-        // Avoid breaking inside the top margin of a float.
-        if (strut) {
-          // If we already had decided to break, just add the margin. The strut
-          // so far only accounts for pushing the top border edge to the next
-          // fragmentainer. We need to push the margin over as well, because
-          // there's no break opportunity between margin and border.
-          strut += marginBefore;
-        } else {
-          // Even if we didn't break before the border box to the next
-          // fragmentainer, we need to check if we can fit the margin before
-          // it.
-          LayoutUnit marginEdge = childBox->logicalTop() - marginBefore;
-          if (LayoutUnit pageHeight = pageLogicalHeightForOffset(marginEdge)) {
-            LayoutUnit remainingSpace = pageRemainingLogicalHeightForOffset(
-                marginEdge, AssociateWithLatterPage);
-            if (remainingSpace <= marginBefore)
-              strut += remainingSpace;
-          }
-        }
-      }
-      if (!strut) {
-        // If we are unsplittable and don't fit, move to the next page or column
-        // if that helps the situation.
-        strut =
-            adjustForUnsplittableChild(*childBox, floatLogicalLocation.y()) -
-            floatLogicalLocation.y();
-      }
+  LayoutPoint floatLogicalLocation =
+      computeLogicalLocationForFloat(floatingObject, logicalTop);
 
-      childBox->setPaginationStrut(strut);
+  setLogicalLeftForFloat(floatingObject, floatLogicalLocation.x());
+
+  setLogicalLeftForChild(childBox,
+                         floatLogicalLocation.x() + childLogicalLeftMargin);
+  setLogicalTopForChild(
+      childBox, floatLogicalLocation.y() + marginBeforeForChild(childBox));
+
+  SubtreeLayoutScope layoutScope(childBox);
+  if (isPaginated && !childBox.needsLayout())
+    markChildForPaginationRelayoutIfNeeded(childBox, layoutScope);
+
+  childBox.layoutIfNeeded();
+
+  if (isPaginated) {
+    LayoutBlockFlow* childBlockFlow =
+        childBox.isLayoutBlockFlow() ? toLayoutBlockFlow(&childBox) : nullptr;
+    // The first piece of content inside the child may have set a strut during
+    // layout.
+    LayoutUnit strut =
+        childBlockFlow ? childBlockFlow->paginationStrutPropagatedFromChild()
+                       : LayoutUnit();
+
+    LayoutUnit marginBefore = marginBeforeForChild(childBox);
+    if (marginBefore > LayoutUnit()) {
+      // Avoid breaking inside the top margin of a float.
       if (strut) {
-        floatLogicalLocation = computeLogicalLocationForFloat(
-            floatingObject, floatLogicalLocation.y() + strut);
-        setLogicalLeftForFloat(floatingObject, floatLogicalLocation.x());
-
-        setLogicalLeftForChild(
-            *childBox, floatLogicalLocation.x() + childLogicalLeftMargin);
-        setLogicalTopForChild(*childBox, floatLogicalLocation.y() +
-                                             marginBeforeForChild(*childBox));
-
-        if (childBox->isLayoutBlock())
-          childBox->setChildNeedsLayout(MarkOnlyThis);
-        childBox->layoutIfNeeded();
+        // If we already had decided to break, just add the margin. The strut
+        // so far only accounts for pushing the top border edge to the next
+        // fragmentainer. We need to push the margin over as well, because
+        // there's no break opportunity between margin and border.
+        strut += marginBefore;
+      } else {
+        // Even if we didn't break before the border box to the next
+        // fragmentainer, we need to check if we can fit the margin before
+        // it.
+        LayoutUnit marginEdge = childBox.logicalTop() - marginBefore;
+        if (LayoutUnit pageHeight = pageLogicalHeightForOffset(marginEdge)) {
+          LayoutUnit remainingSpace = pageRemainingLogicalHeightForOffset(
+              marginEdge, AssociateWithLatterPage);
+          if (remainingSpace <= marginBefore)
+            strut += remainingSpace;
+        }
       }
     }
+    if (!strut) {
+      // If we are unsplittable and don't fit, move to the next page or column
+      // if that helps the situation.
+      strut = adjustForUnsplittableChild(childBox, floatLogicalLocation.y()) -
+              floatLogicalLocation.y();
+    }
 
-    setLogicalTopForFloat(floatingObject, floatLogicalLocation.y());
+    childBox.setPaginationStrut(strut);
+    if (strut) {
+      floatLogicalLocation = computeLogicalLocationForFloat(
+          floatingObject, floatLogicalLocation.y() + strut);
+      setLogicalLeftForFloat(floatingObject, floatLogicalLocation.x());
 
-    setLogicalHeightForFloat(floatingObject,
-                             logicalHeightForChild(*childBox) +
-                                 marginBeforeForChild(*childBox) +
-                                 marginAfterForChild(*childBox));
+      setLogicalLeftForChild(childBox,
+                             floatLogicalLocation.x() + childLogicalLeftMargin);
+      setLogicalTopForChild(
+          childBox, floatLogicalLocation.y() + marginBeforeForChild(childBox));
 
-    m_floatingObjects->addPlacedObject(floatingObject);
+      if (childBox.isLayoutBlock())
+        childBox.setChildNeedsLayout(MarkOnlyThis);
+      childBox.layoutIfNeeded();
+    }
+  }
 
-    if (ShapeOutsideInfo* shapeOutside = childBox->shapeOutsideInfo())
-      shapeOutside->setReferenceBoxLogicalSize(logicalSizeForChild(*childBox));
+  setLogicalTopForFloat(floatingObject, floatLogicalLocation.y());
 
-    if (width)
-      width->shrinkAvailableWidthForNewFloatIfNeeded(floatingObject);
-  }
-  return true;
+  setLogicalHeightForFloat(floatingObject, logicalHeightForChild(childBox) +
+                                               marginBeforeForChild(childBox) +
+                                               marginAfterForChild(childBox));
+
+  if (ShapeOutsideInfo* shapeOutside = childBox.shapeOutsideInfo())
+    shapeOutside->setReferenceBoxLogicalSize(logicalSizeForChild(childBox));
+
+  return floatLogicalLocation.y();
 }
 
 bool LayoutBlockFlow::hasOverhangingFloat(LayoutBox* layoutBox) {