[LayoutNG] Properly stop layout at spanner containers.

In certain cases it was possible to lay out stuff before and after a
spanner in the same column, because we didn't detect that something
broke.

This was due to an ancient out-of-space check in
FinalizeForFragmentation(), which sometimes prevented us from finishing
fragmentation correctly (the assumption was that if there's zero space
left, this fragment can't fit here (so just return early), but that's
not necessarily true). Check correctly for out-of-space in the generic
fragmentation machinery instead.

This addresses a TODO about DCHECKing that we broke inside when
encountering a spanner.

Bug: 829028
Change-Id: I86af5dcdf01e44c2b0d05d7c6c04821000b25426
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2443854
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Alison Maher <almaher@microsoft.com>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813353}

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

@@ -1888,9 +1888,7 @@ NGLayoutResult::EStatus NGBlockLayoutAlgorithm::FinishInflow(
   // the spanner to the column layout algorithm, so that it can take care of it.
   if (UNLIKELY(ConstraintSpace().IsInColumnBfc())) {
     if (NGBlockNode spanner_node = layout_result->ColumnSpanner()) {
-      // TODO(mstensho): It would be nice to DCHECK for container_builder_
-      // .HasInflowChildBreakInside() here, but currently quite a few tests
-      // would fail then.
+      DCHECK(container_builder_.HasInflowChildBreakInside());
       container_builder_.SetColumnSpanner(spanner_node);
     }
   }
@@ -2161,22 +2159,8 @@ bool NGBlockLayoutAlgorithm::FinalizeForFragmentation() {
   }
 
   LayoutUnit space_left = kIndefiniteSize;
-  if (ConstraintSpace().HasKnownFragmentainerBlockSize()) {
+  if (ConstraintSpace().HasKnownFragmentainerBlockSize())
     space_left = FragmentainerSpaceAvailable();
-    if (space_left <= LayoutUnit()) {
-      // The amount of space available may be zero, or even negative, if the
-      // border-start edge of this block starts exactly at, or even after the
-      // fragmentainer boundary. We're going to need a break before this block,
-      // because no part of it fits in the current fragmentainer. Due to margin
-      // collapsing with children, this situation is something that we cannot
-      // always detect prior to layout. The fragment produced by this algorithm
-      // is going to be thrown away. The parent layout algorithm will eventually
-      // detect that there's no room for a fragment for this node, and drop the
-      // fragment on the floor. Therefore it doesn't matter how we set up the
-      // container builder, so just return.
-      return true;
-    }
-  }
 
   return FinishFragmentation(Node(), ConstraintSpace(), BreakToken(),
                              BorderPadding().block_end, space_left,

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

@@ -588,6 +588,9 @@ bool MovePastBreakpoint(const NGConstraintSpace& space,
     return true;
   }
 
+  const auto* break_token =
+      DynamicTo<NGBlockBreakToken>(physical_fragment.BreakToken());
+
   LayoutUnit space_left =
       space.FragmentainerBlockSize() - fragmentainer_block_offset;
 
@@ -598,13 +601,24 @@ bool MovePastBreakpoint(const NGConstraintSpace& space,
 
   // If the child starts past the end of the fragmentainer (probably due to a
   // block-start margin), we must break before it.
-  bool must_break_before = space_left < LayoutUnit();
+  bool must_break_before = false;
+  if (space_left < LayoutUnit()) {
+    must_break_before = true;
+  } else if (space_left == LayoutUnit()) {
+    // If the child starts exactly at the end, we'll allow the child here if the
+    // fragment contains the block-end of the child, or if it's a column
+    // spanner. Otherwise we have to break before it. We don't want empty
+    // fragments with nothing useful inside, if it's to be resumed in the next
+    // fragmentainer.
+    must_break_before = !layout_result.ColumnSpanner() && break_token &&
+                        !break_token->IsAtBlockEnd();
+  }
   if (must_break_before) {
     DCHECK(!refuse_break_before);
     return false;
   }
 
-  if (IsA<NGBlockBreakToken>(physical_fragment.BreakToken())) {
+  if (break_token) {
     // The block child broke inside. We now need to decide whether to keep that
     // break, or if it would be better to break before it.
     NGBreakAppeal appeal_inside = CalculateBreakAppealInside(

third_party/blink/web_tests/TestExpectations

@@ -992,6 +992,7 @@ virtual/layout_ng_block_frag/external/wpt/css/css-multicol/baseline-008.html [ P
 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-006.html [ Pass ]
 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-007.html [ Pass ]
 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-008.html [ Pass ]
+virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-014.html [ Pass ]
 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-fieldset-001.html [ Pass ]
 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-margin-bottom-001.xht [ Pass ]
 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-float-001.xht [ Pass ]
@@ -3318,6 +3319,7 @@ crbug.com/915204 external/wpt/css/css-multicol/multicol-span-all-009.html [ Fail
 crbug.com/915204 external/wpt/css/css-multicol/multicol-span-all-008.html [ Failure ]
 crbug.com/926685 external/wpt/css/css-multicol/multicol-span-all-010.html [ Failure ]
 crbug.com/915204 external/wpt/css/css-multicol/multicol-span-all-011.html [ Failure ]
+crbug.com/829028 external/wpt/css/css-multicol/multicol-span-all-014.html [ Failure ]
 crbug.com/892817 external/wpt/css/css-multicol/multicol-span-all-margin-bottom-001.xht [ Failure ]
 crbug.com/636055 external/wpt/css/css-multicol/multicol-span-all-margin-nested-002.xht [ Failure ]
 crbug.com/990240 external/wpt/css/css-multicol/multicol-span-all-rule-001.html [ Failure ]

third_party/blink/web_tests/external/wpt/css/css-multicol/multicol-span-all-014.html

+<!DOCTYPE html>
+<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
+<link rel="help" href="https://www.w3.org/TR/css-multicol-1/#spanning-columns">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div style="columns:2; column-gap:0; width:100px; background:red;">
+  <div style="height:100px;"></div>
+  <div>
+    <div style="column-span:all; height:50px;"></div>
+  </div>
+  <div style="display:flow-root; height:0;">
+    <div style="width:100px; height:100px; margin-top:-100px; background:green;"></div>
+  </div>
+</div>