Keep block pagination struts after layout, and store them before any type of break.

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

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 667 matching lines @@
     if (child.isLayoutMultiColumnSpannerPlaceholder()) {
         // The actual column-span:all element is positioned by this placeholder child.
         positionSpannerDescendant(toLayoutMultiColumnSpannerPlaceholder(child));
     }
 }
 
 LayoutUnit LayoutBlockFlow::adjustBlockChildForPagination(LayoutUnit logicalTop, LayoutBox& child, bool atBeforeSideOfBlock)
 {
     LayoutBlockFlow* childBlockFlow = child.isLayoutBlockFlow() ? toLayoutBlockFlow(&child) : 0;
 
+    // Calculate the pagination strut for this child. A strut may come from three sources:
+    // 1. The first piece of content inside the child doesn't fit in the current page or column
+    // 2. A forced break before the child
+    // 3. The child itself is unsplittable and doesn't fit in the current page or column.
+    //
+    // No matter which source, if we need to insert a strut, it should always take us to the exact
+    // top of the next page or column, or be zero.
+
+    // We're now going to calculate the child's final pagination strut. We may end up propagating
+    // it to its containing block (|this|), so reset it first.
+    child.setPaginationStrut(LayoutUnit());

[leviw_travelin_and_unemployed, 2015/10/06 13:17:59]

We should probably have a resetPaginationStrut() method :)

[mstensho (USE GERRIT), 2015/10/06 14:01:45]

Done.

+
+    // The first piece of content inside the child may have set a strut during layout. Currently,
+    // only block flows support strut propagation, but this may (and should) change in the future.

[leviw_travelin_and_unemployed, 2015/10/06 13:17:59]

Seems bug-worthy for more context.

[mstensho (USE GERRIT), 2015/10/06 14:01:45]

Done.

+    LayoutUnit strutFromContent = childBlockFlow ? childBlockFlow->paginationStrutPropagatedFromChild() : LayoutUnit();
+    LayoutUnit logicalTopWithContentStrut = logicalTop + strutFromContent;
+
     // If the object has a page or column break value of "before", then we should shift to the top of the next page.
-    LayoutUnit newLogicalTop = applyBeforeBreak(child, logicalTop);
+    LayoutUnit logicalTopAfterForcedBreak = applyBeforeBreak(child, logicalTop);
 
     // For replaced elements and scrolled elements, we want to shift them to the next page if they don't fit on the current one.
-    LayoutUnit logicalTopBeforeUnsplittableAdjustment = newLogicalTop;
-    LayoutUnit logicalTopAfterUnsplittableAdjustment = adjustForUnsplittableChild(child, newLogicalTop);
+    LayoutUnit logicalTopAfterUnsplittable = adjustForUnsplittableChild(child, logicalTop);
+    LayoutUnit childLogicalHeight = child.logicalHeight();
+    bool neededBreakForUnsplittable = logicalTopAfterUnsplittable != logicalTop;
+    if (neededBreakForUnsplittable)
+        setPageBreak(logicalTop, childLogicalHeight - (logicalTopAfterUnsplittable - logicalTop));
 
-    LayoutUnit paginationStrut = 0;
-    LayoutUnit unsplittableAdjustmentDelta = logicalTopAfterUnsplittableAdjustment - logicalTopBeforeUnsplittableAdjustment;
-    LayoutUnit childLogicalHeight = child.logicalHeight();
-    if (unsplittableAdjustmentDelta) {
-        setPageBreak(newLogicalTop, childLogicalHeight - unsplittableAdjustmentDelta);
-        paginationStrut = unsplittableAdjustmentDelta;
-    } else if (childBlockFlow && childBlockFlow->paginationStrutPropagatedFromChild()) {
-        paginationStrut = childBlockFlow->paginationStrutPropagatedFromChild();
-    }
+    // Some sanity checks: No matter what the reason is for pushing the child to the next page or
+    // column, the amount should be the same.
+    ASSERT(!strutFromContent || logicalTopAfterForcedBreak == logicalTop || logicalTopAfterForcedBreak == logicalTopWithContentStrut);
+    ASSERT(!strutFromContent || logicalTopAfterUnsplittable == logicalTop || logicalTopAfterUnsplittable == logicalTopWithContentStrut);
+    ASSERT(logicalTopAfterUnsplittable == logicalTop || logicalTopAfterForcedBreak == logicalTop || logicalTopAfterUnsplittable == logicalTopAfterForcedBreak);
 
-    if (paginationStrut) {
+    LayoutUnit logicalTopAfterPagination = std::max(logicalTopWithContentStrut, std::max(logicalTopAfterForcedBreak, logicalTopAfterUnsplittable));
+    LayoutUnit newLogicalTop = logicalTop;
+    if (LayoutUnit paginationStrut = logicalTopAfterPagination - logicalTop) {
+        ASSERT(paginationStrut > 0);
         // We are willing to propagate out to our parent block as long as we were at the top of the block prior
         // to collapsing our margins, and as long as we didn't clear or move as a result of other pagination.
-        if (atBeforeSideOfBlock && logicalTop == newLogicalTop && allowsPaginationStrut()) {
+        if (atBeforeSideOfBlock && logicalTopAfterForcedBreak == logicalTop && allowsPaginationStrut()) {
             // FIXME: Should really check if we're exceeding the page height before propagating the strut, but we don't
             // have all the information to do so (the strut only has the remaining amount to push). Gecko gets this wrong too
             // and pushes to the next page anyway, so not too concerned about it.
             paginationStrut += logicalTop;
             if (isFloating())
                 paginationStrut += marginBefore(); // Floats' margins do not collapse with page or column boundaries.
             setPaginationStrutPropagatedFromChild(paginationStrut);
             if (childBlockFlow)
                 childBlockFlow->setPaginationStrutPropagatedFromChild(LayoutUnit());
         } else {
+            child.setPaginationStrut(paginationStrut);
             newLogicalTop += paginationStrut;
         }
     }
 
-    if (!unsplittableAdjustmentDelta) {
+    if (!neededBreakForUnsplittable) {
         if (LayoutUnit pageLogicalHeight = pageLogicalHeightForOffset(newLogicalTop)) {
             LayoutUnit remainingLogicalHeight = pageRemainingLogicalHeightForOffset(newLogicalTop, AssociateWithLatterPage);
             LayoutUnit spaceShortage = childLogicalHeight - remainingLogicalHeight;
             if (spaceShortage > 0) {
                 // If the child crosses a column boundary, report a break, in case nothing inside it
                 // has already done so. The column balancer needs to know how much it has to stretch
                 // the columns to make more content fit. If no breaks are reported (but do occur),
                 // the balancer will have no clue. Only measure the space after the last column
                 // boundary, in case it crosses more than one.
                 LayoutUnit spaceShortageInLastColumn = intMod(spaceShortage, pageLogicalHeight);
@@ skipping 1668 matching lines @@
 
         SubtreeLayoutScope layoutScope(*childBox);
         LayoutState* layoutState = view()->layoutState();
         bool isPaginated = layoutState->isPaginated();
         if (isPaginated && !childBox->needsLayout())
             childBox->markForPaginationRelayoutIfNeeded(layoutScope);
 
         childBox->layoutIfNeeded();
 
         if (isPaginated) {
-            LayoutUnit newLogicalTop = floatLogicalLocation.y();
+            // If we are unsplittable and don't fit, move to the next page or column if that helps
+            // the situation.
+            LayoutUnit newLogicalTop = adjustForUnsplittableChild(*childBox, floatLogicalLocation.y());
+            LayoutUnit strutFromUnsplittable = newLogicalTop - floatLogicalLocation.y();
+            LayoutUnit finalStrut;
+            if (strutFromUnsplittable)

[leviw_travelin_and_unemployed, 2015/10/06 13:17:59]

You switched the order of this logic from the old version. Why? The old version
would only call adjustForUnsplittableChild in the else clause, which should be
cheaper, no? That method seems expensive.

[mstensho (USE GERRIT), 2015/10/06 14:01:45]

Seems that I had no good reason for making this change. Switching back.

+                finalStrut = strutFromUnsplittable;
+            else if (LayoutBlockFlow* childBlockFlow = childBox->isLayoutBlockFlow() ? toLayoutBlockFlow(childBox) : nullptr)
+                finalStrut = childBlockFlow->paginationStrutPropagatedFromChild();
 
-            LayoutBlockFlow* childBlockFlow = childBox->isLayoutBlockFlow() ? toLayoutBlockFlow(childBox) : 0;
-            if (childBlockFlow && childBlockFlow->paginationStrutPropagatedFromChild()) {
-                // Some content inside this float has determined that we need to move to the next
-                // page or column.
-                newLogicalTop += childBlockFlow->paginationStrutPropagatedFromChild();
-            } else {
-                // Now that we know the final height, check if we are unsplittable, and if we don't
-                // fit at the current position, but would fit at the top of the next page or
-                // column, move there.
-                newLogicalTop = adjustForUnsplittableChild(*childBox, newLogicalTop);
-            }
-
-            if (newLogicalTop != floatLogicalLocation.y()) {
-                floatingObject.setPaginationStrut(newLogicalTop - floatLogicalLocation.y());
-
-                floatLogicalLocation = computeLogicalLocationForFloat(floatingObject, newLogicalTop);
+            floatingObject.setPaginationStrut(finalStrut);
+            childBox->setPaginationStrut(finalStrut);
+            if (finalStrut) {
+                floatLogicalLocation = computeLogicalLocationForFloat(floatingObject, floatLogicalLocation.y() + finalStrut);
                 setLogicalLeftForFloat(floatingObject, floatLogicalLocation.x());
 
                 setLogicalLeftForChild(*childBox, floatLogicalLocation.x() + childLogicalLeftMargin);
                 setLogicalTopForChild(*childBox, floatLogicalLocation.y() + marginBeforeForChild(*childBox));
 
                 if (childBox->isLayoutBlock())
                     childBox->setChildNeedsLayout(MarkOnlyThis);
                 childBox->layoutIfNeeded();
             }
         }
@@ skipping 646 matching lines @@
     FrameView* frameView = document().view();
     LayoutUnit top = (style()->position() == FixedPosition) ? 0 : frameView->scrollOffset().height();
     int visibleHeight = frameView->visibleContentRect(IncludeScrollbars).height();
     if (size().height() < visibleHeight)
         top += (visibleHeight - size().height()) / 2;
     setY(top);
     dialog->setCentered(top);
 }
 
 } // namespace blink