[LayoutNG] Only propagate break-before of the first child.

The break-before value should only be propagated if we're not at a valid
class A breakpoint. This is only the case at the first in-flow child.
Propagating it in other situations could cause spurious forced breaks in
the ancestry, as the new test would demonstrate.

The actual fix here is to simply check that we're at the first child,
but I rearranged the code a little while I was at it, to keep the block
fragmentation code more closely together. We'll also no longer update
the has_processed_first_child_ member unless we're block-fragmenting.

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

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

@@ -527,14 +527,6 @@ inline scoped_refptr<const NGLayoutResult> NGBlockLayoutAlgorithm::Layout(
       }
       break;
     } else {
-      // We need to propagate the initial break-before value up our container
-      // chain, until we reach a container that's not a first child. If we get
-      // all the way to the root of the fragmentation context without finding
-      // any such container, we have no valid class A break point, and if a
-      // forced break was requested, none will be inserted.
-      if (!child.IsInline() && ConstraintSpace().HasBlockFragmentation())
-        container_builder_.SetInitialBreakBefore(child.Style().BreakBefore());
-
       bool abort;
       if (child.CreatesNewFormattingContext()) {
         abort = !HandleNewFormattingContext(child, child_break_token,
@@ -558,11 +550,21 @@ inline scoped_refptr<const NGLayoutResult> NGBlockLayoutAlgorithm::Layout(
         return container_builder_.Abort(
             NGLayoutResult::kBfcBlockOffsetResolved);
       }
-      if (container_builder_.DidBreak() &&
-          IsFragmentainerOutOfSpace(
-              previous_inflow_position.logical_block_offset))
-        break;
-      has_processed_first_child_ = true;
+      if (ConstraintSpace().HasBlockFragmentation()) {
+        if (container_builder_.DidBreak() &&
+            IsFragmentainerOutOfSpace(
+                previous_inflow_position.logical_block_offset))
+          break;
+
+        // We need to propagate the initial break-before value up our container
+        // chain, until we reach a container that's not a first child. If we get
+        // all the way to the root of the fragmentation context without finding
+        // any such container, we have no valid class A break point, and if a
+        // forced break was requested, none will be inserted.
+        if (!has_processed_first_child_ && !child.IsInline())
+          container_builder_.SetInitialBreakBefore(child.Style().BreakBefore());
+        has_processed_first_child_ = true;
+      }
     }
   }
 

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

@@ -357,6 +357,9 @@ class CORE_EXPORT NGBlockLayoutAlgorithm
   // set, and abort layout if it is.
   bool abort_when_bfc_block_offset_updated_ = false;
 
+  // This will be set during block fragmentation once we've processed the first
+  // in-flow child of a container. It is used to check if we're at a valid class
+  // A or B breakpoint (between block-level siblings or line box siblings).
   bool has_processed_first_child_ = false;
 
   NGExclusionSpace exclusion_space_;

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

@@ -2023,6 +2023,43 @@ TEST_F(NGColumnLayoutAlgorithmTest, ForcedBreaks) {
   EXPECT_EQ(expectation, dump);
 }
 
+TEST_F(NGColumnLayoutAlgorithmTest, ForcedBreakInSecondChild) {
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      #parent {
+        columns: 3;
+        column-fill: auto;
+        column-gap: 10px;
+        width: 320px;
+        height: 100px;
+      }
+    </style>
+    <div id="container">
+      <div id="parent">
+        <div style="width:33px; height:20px;"></div>
+        <div style="width:34px;">
+          <div style="width:35px; height:20px;"></div>
+          <div style="break-before:column; width:36px; height:20px;"></div>
+        </div>
+      </div>
+    </div>
+  )HTML");
+
+  String dump = DumpFragmentTree(GetElementById("container"));
+  String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
+  offset:unplaced size:1000x100
+    offset:0,0 size:320x100
+      offset:0,0 size:100x100
+        offset:0,0 size:33x20
+        offset:0,20 size:34x80
+          offset:0,0 size:35x20
+      offset:110,0 size:100x20
+        offset:0,0 size:34x20
+          offset:0,0 size:36x20
+)DUMP";
+  EXPECT_EQ(expectation, dump);
+}
+
 TEST_F(NGColumnLayoutAlgorithmTest, ForcedAndUnforcedBreaksAtSameBoundary) {
   // We have two parallel flows, one with a forced break inside and one with an
   // unforced break. Check that we handle the block-start margins correctly