[LayoutNG] Avoid break between last child and container edge.

If there's no class C breakpoint (i.e. non-zero gap) between the outer
block-end edge of the last child and the inner block-end edge of its
container, we really don't want to break there. We also don't want to
break *inside* the block-end border/padding area.

Only allow breaking here if there are no real preceding breakpoints.

Bug: 829028
Change-Id: If922143923446060702eec5cfb8f7b2fbed06a10
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2264296
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782980}

third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.cc

@@ -2168,10 +2168,8 @@ bool NGBlockLayoutAlgorithm::FinalizeForFragmentation() {
     }
   }
 
-  FinishFragmentation(Node(), ConstraintSpace(), BreakToken(), BorderPadding(),
-                      space_left, &container_builder_);
-
-  return true;
+  return FinishFragmentation(Node(), ConstraintSpace(), BreakToken(),
+                             BorderPadding(), space_left, &container_builder_);
 }
 
 NGBreakStatus NGBlockLayoutAlgorithm::BreakBeforeChildIfNeeded(

third_party/blink/renderer/core/layout/ng/ng_fragmentation_utils.cc

@@ -224,7 +224,7 @@ bool IsNodeFullyGrown(NGBlockNode node,
   return max_block_size == current_total_block_size;
 }
 
-void FinishFragmentation(NGBlockNode node,
+bool FinishFragmentation(NGBlockNode node,
                          const NGConstraintSpace& space,
                          const NGBlockBreakToken* previous_break_token,
                          const NGBoxStrut& border_padding,
@@ -280,7 +280,7 @@ void FinishFragmentation(NGBlockNode node,
     // We don't know how space is available (initial column balancing pass), so
     // we won't break.
     builder->SetIsAtBlockEnd();
-    return;
+    return true;
   }
 
   if (builder->HasChildBreakInside()) {
@@ -328,12 +328,11 @@ void FinishFragmentation(NGBlockNode node,
       if (!builder->HasInflowChildBreakInside())
         builder->SetBreakAppeal(kBreakAppealPerfect);
     }
-    return;
+    return true;
   }
 
   if (desired_block_size > space_left) {
-    // No child inside broke, but we need a break inside this block anyway, due
-    // to its size.
+    // No child inside broke, but we're too tall to fit.
     NGBreakAppeal break_appeal = kBreakAppealPerfect;
     if (!previously_consumed_block_size) {
       // This is the first fragment generated for the node. Avoid breaking
@@ -346,15 +345,30 @@ void FinishFragmentation(NGBlockNode node,
       if (space_left < block_start_unbreakable_space)
         break_appeal = kBreakAppealLastResort;
     }
-    builder->SetBreakAppeal(break_appeal);
     if (space.BlockFragmentationType() == kFragmentColumn &&
         !space.IsInitialColumnBalancingPass())
       builder->PropagateSpaceShortage(desired_block_size - space_left);
-    return;
+    if (desired_block_size <= intrinsic_block_size) {
+      // We only want to break inside if there's a valid class C breakpoint [1].
+      // That is, we need a non-zero gap between the last child (outer block-end
+      // edge) and this container (inner block-end edge). We've just found that
+      // not to be the case. If we have found a better early break, we should
+      // break there. Otherwise mark the break as unappealing, as breaking here
+      // means that we're going to break inside the block-end padding or border,
+      // or right before them. No valid breakpoints there.
+      //
+      // [1] https://www.w3.org/TR/css-break-3/#possible-breaks
+      if (builder->HasEarlyBreak())
+        return false;
+      break_appeal = kBreakAppealLastResort;
+    }
+    builder->SetBreakAppeal(break_appeal);
+    return true;
   }
 
   // The end of the block fits in the current fragmentainer.
   builder->SetIsAtBlockEnd();
+  return true;
 }
 
 NGBreakStatus BreakBeforeChildIfNeeded(const NGConstraintSpace& space,

third_party/blink/renderer/core/layout/ng/ng_fragmentation_utils.h

@@ -131,7 +131,13 @@ bool IsNodeFullyGrown(NGBlockNode,
 // do, since any block-size specified in CSS applies to the entire box,
 // regardless of fragmentation. This function will update the block-size to the
 // actual fragment size, by examining possible breakpoints, if necessary.
-void FinishFragmentation(NGBlockNode node,
+//
+// Return true if successful. If false is returned, it means that we ran out of
+// space at a less-than-ideal location - in this case between the last child and
+// the block-end padding / border. Furthermore, this also means that we know
+// that we have a better earlier breakpoint, so the correct response to 'false'
+// is to abort layout, then relayout and break earlier.
+bool FinishFragmentation(NGBlockNode node,
                          const NGConstraintSpace&,
                          const NGBlockBreakToken* previous_break_token,
                          const NGBoxStrut& border_padding,

third_party/blink/web_tests/TestExpectations

@@ -892,6 +892,10 @@ crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/flowthread-with-floa
 
 crbug.com/1079031 virtual/layout_ng_block_frag/external/wpt/css/css-break/block-end-aligned-abspos-nested.html [ Failure ]
 crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-break/block-end-aligned-abspos-with-overflow.html [ Failure ]
+crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-break/break-at-end-container-edge-000.html [ Pass ]
+crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-break/break-at-end-container-edge-001.html [ Pass ]
+crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-break/break-at-end-container-edge-002.html [ Pass ]
+crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-break/break-at-end-container-edge-004.html [ Pass ]
 crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-break/break-between-avoid-000.html [ Pass ]
 crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-break/break-between-avoid-001.html [ Pass ]
 crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-break/break-between-avoid-003.html [ Pass ]
@@ -3602,6 +3606,10 @@ crbug.com/1034807 [ Mac ] virtual/off-main-thread-css-paint/external/wpt/css/css
 crbug.com/982116 http/tests/devtools/elements/styles-4/styles-new-API.js [ Pass Timeout ]
 
 crbug.com/829028 external/wpt/css/css-break/avoid-border-break.html [ Failure ]
+crbug.com/829028 external/wpt/css/css-break/break-at-end-container-edge-000.html [ Failure ]
+crbug.com/829028 external/wpt/css/css-break/break-at-end-container-edge-001.html [ Failure ]
+crbug.com/829028 external/wpt/css/css-break/break-at-end-container-edge-002.html [ Failure ]
+crbug.com/829028 external/wpt/css/css-break/break-at-end-container-edge-004.html [ Failure ]
 crbug.com/829028 external/wpt/css/css-break/break-between-avoid-000.html [ Failure ]
 crbug.com/829028 external/wpt/css/css-break/break-between-avoid-001.html [ Failure ]
 crbug.com/829028 external/wpt/css/css-break/break-between-avoid-003.html [ Failure ]

third_party/blink/web_tests/external/wpt/css/css-break/break-at-end-container-edge-000.html

+<!DOCTYPE html>
+<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
+<link rel="help" href="https://www.w3.org/TR/css-break-3/#possible-breaks">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<meta name="assert" content="We should break between the two lines (class B breakpoint), rather than between the last line and the container padding">
+<style>
+  .child { display:inline-block; vertical-align:top; width:100%; height:50px; line-height:50px; }
+</style>
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div style="columns:2; column-fill:auto; column-gap:0; width:100px; height:100px; orphans:1; widows:1; background:red;">
+  <div style="padding-bottom:50px; background:green;">
+    <div class="child"></div>
+    <div class="child"></div>
+  </div>
+</div>

third_party/blink/web_tests/external/wpt/css/css-break/break-at-end-container-edge-001.html

+<!DOCTYPE html>
+<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
+<link rel="help" href="https://www.w3.org/TR/css-break-3/#possible-breaks">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<meta name="assert" content="We should break between the last two lines (class B breakpoint), rather than inside the container padding">
+<style>
+  .child { display:inline-block; vertical-align:top; width:100%; height:30px; line-height:30px; }
+</style>
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div style="columns:2; column-fill:auto; column-gap:0; width:100px; height:100px; orphans:1; widows:1; background:red;">
+  <div style="padding-bottom:70px; background:green;">
+    <div class="child"></div>
+    <div class="child"></div>
+    <div class="child"></div>
+  </div>
+</div>

third_party/blink/web_tests/external/wpt/css/css-break/break-at-end-container-edge-002.html

+<!DOCTYPE html>
+<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
+<link rel="help" href="https://www.w3.org/TR/css-break-3/#possible-breaks">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<meta name="assert" content="We should break between the first two lines (class B breakpoint, also honoring widows), rather than inside the container padding">
+<style>
+  .child { display:inline-block; vertical-align:top; width:100%; height:30px; line-height:30px; }
+</style>
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div style="columns:2; column-fill:auto; column-gap:0; width:100px; height:100px; orphans:1; widows:2; background:red;">
+  <div style="padding-bottom:40px; background:green;">
+    <div class="child"></div>
+    <div class="child"></div>
+    <div class="child"></div>
+  </div>
+</div>

third_party/blink/web_tests/external/wpt/css/css-break/break-at-end-container-edge-003.html

+<!DOCTYPE html>
+<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
+<link rel="help" href="https://www.w3.org/TR/css-break-3/#possible-breaks">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<meta name="assert" content="The block with padding should be pushed to the second column, so that we won't have to break between the line and the padding edge">
+<style>
+  .child { display:inline-block; vertical-align:top; width:100%; height:90px; line-height:90px; }
+</style>
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div style="columns:2; column-fill:auto; column-gap:0; width:100px; height:100px; orphans:1; widows:1; background:red;">
+  <div style="float:left; width:100%; height:100px; background:green;"></div>
+  <div style="height:10px;"></div>
+  <div style="padding-bottom:10px; background:green;">
+    <div class="child"></div>
+  </div>
+</div>

third_party/blink/web_tests/external/wpt/css/css-break/break-at-end-container-edge-004.html

+<!DOCTYPE html>
+<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
+<link rel="help" href="https://www.w3.org/TR/css-break-3/#possible-breaks">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<meta name="assert" content="The last line could fit in the third column, but will be pushed to the fourth, so that we won't have to break between the last line and the container padding">
+<style>
+  .child { display:inline-block; vertical-align:top; width:100%; height:40px; line-height:40px; }
+</style>
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div style="columns:4; column-fill:auto; column-gap:0; width:100px; height:100px; orphans:1; widows:1; background:red;">
+  <div style="height:40px; background:green;"></div>
+  <div style="padding-bottom:60px; background:green;">
+    <div class="child"></div>
+    <div class="child"></div>
+    <div class="child"></div>
+    <div class="child"></div>
+    <div class="child"></div>
+  </div>
+</div>