[LayoutNG] Truncate block-end child margins at fragmentainer boundaries.

This was almost working correctly even without this fix, because we
currently just brutally break inside a block that overflows if there are
no breaks inside, losing any trailing margins. The only problem
currently, is that such a break still produces a break token, so that
the broken block would resume in the next fragmentainer (even if there's
nothing left to lay out), producing no content there, but an empty
fragment would create a break opportunity that shouldn't really be
there. This was a problem both for regular block containers and
fieldsets. This CL fixes that. It's also a problem for column spanners
if they participate in an outer fragmentainer context, but we have some
underlying issues there, so I just added a TODO.

The -001.html test wouldn't even fail without this CL, but as I'm
working on fixing the aforementioned brutal breaking, I felt we need
slightly better test coverage.

The new function AdjustedMarginAfterFinalChildFragment() is extremely
simple, and the amount of code duplication it prevents is negligible (or
even non-existent). Still, it seems reasonable to provide this function,
for the sake of visibility, as this is something that all algorithms
need to implement, if they want to get block fragmentation right.

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

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

@@ -777,16 +777,7 @@ scoped_refptr<const NGLayoutResult> NGBlockLayoutAlgorithm::FinishLayout(
     //  - There was a self-collapsing child affected by clearance.
     //  - We are a new formatting context.
     // Additionally this fragment produces no end margin strut.
-    //
-    // If we are a quirky container, we ignore any quirky margins and
-    // just consider normal margins to extend our size.  Other UAs
-    // perform this calculation differently, e.g. by just ignoring the
-    // *last* quirky margin.
-    // TODO: revisit previous implementation to avoid changing behavior and
-    // https://html.spec.whatwg.org/C/#margin-collapsing-quirks
-    LayoutUnit margin_strut_sum = node_.IsQuirkyContainer()
-                                      ? end_margin_strut.QuirkyContainerSum()
-                                      : end_margin_strut.Sum();
+
     if (!container_builder_.BfcBlockOffset()) {
       // If we have collapsed through the block start and all children (if any),
       // now is the time to determine the BFC block offset, because finally we
@@ -801,6 +792,22 @@ scoped_refptr<const NGLayoutResult> NGBlockLayoutAlgorithm::FinishLayout(
       }
       DCHECK(container_builder_.BfcBlockOffset());
     } else {
+      // If we are a quirky container, we ignore any quirky margins and just
+      // consider normal margins to extend our size.  Other UAs perform this
+      // calculation differently, e.g. by just ignoring the *last* quirky
+      // margin.
+      LayoutUnit margin_strut_sum = node_.IsQuirkyContainer()
+                                        ? end_margin_strut.QuirkyContainerSum()
+                                        : end_margin_strut.Sum();
+
+      if (ConstraintSpace().HasKnownFragmentainerBlockSize()) {
+        LayoutUnit bfc_block_offset =
+            *container_builder_.BfcBlockOffset() +
+            previous_inflow_position->logical_block_offset;
+        margin_strut_sum = AdjustedMarginAfterFinalChildFragment(
+            ConstraintSpace(), bfc_block_offset, margin_strut_sum);
+      }
+
       // The trailing margin strut will be part of our intrinsic block size, but
       // only if there is something that separates the end margin strut from the
       // input margin strut (typically child content, block start

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

@@ -378,6 +378,9 @@ NGBreakStatus NGColumnLayoutAlgorithm::LayoutChildren() {
     // resuming.
     container_builder_.SetHasSeenAllChildren();
 
+    // TODO(mstensho): Truncate the child margin if it overflows the
+    // fragmentainer, by using AdjustedMarginAfterFinalChildFragment().
+
     intrinsic_block_size_ += margin_strut.Sum();
   }
 

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

@@ -277,8 +277,16 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutLegend(
     }
 
     LayoutUnit legend_margin_box_block_size =
-        NGFragment(writing_mode_, physical_fragment).BlockSize() +
-        legend_margins.BlockSum();
+        legend_margins.block_start +
+        NGFragment(writing_mode_, physical_fragment).BlockSize();
+
+    LayoutUnit block_end_margin = legend_margins.block_end;
+    if (ConstraintSpace().HasKnownFragmentainerBlockSize()) {
+      block_end_margin = AdjustedMarginAfterFinalChildFragment(
+          ConstraintSpace(), legend_margin_box_block_size, block_end_margin);
+    }
+    legend_margin_box_block_size += block_end_margin;
+
     LayoutUnit space_left = borders_.block_start - legend_margin_box_block_size;
 
     if (space_left > LayoutUnit()) {

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

@@ -239,6 +239,19 @@ bool AttemptSoftBreak(const NGConstraintSpace&,
                       NGBreakAppeal appeal_before,
                       NGBoxFragmentBuilder*);
 
+// Return the adjusted child margin to be applied at the end of a fragment.
+// Margins should collapse with the fragmentainer boundary. |bfc_block_offset|
+// is the BFC offset where the margin should be applied (i.e. after the
+// block-end border edge of the last child fragment).
+inline LayoutUnit AdjustedMarginAfterFinalChildFragment(
+    const NGConstraintSpace& space,
+    LayoutUnit bfc_block_offset,
+    LayoutUnit block_end_margin) {
+  LayoutUnit space_left =
+      FragmentainerSpaceAtBfcStart(space) - bfc_block_offset;
+  return std::min(block_end_margin, space_left.ClampNegativeToZero());
+}
+
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_CORE_LAYOUT_NG_NG_FRAGMENTATION_UTILS_H_

third_party/blink/web_tests/TestExpectations

@@ -891,6 +891,8 @@ crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-break/floats-
 crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-break/line-after-unbreakable-float-after-padding.html [ Failure ]
 crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-break/overflowed-block-with-no-room-after-001.html [ Pass ]
 crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-break/overflowed-block-with-no-room-after-000.html [ Pass ]
+crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-break/trailing-child-margin-000.html [ Pass ]
+crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-break/trailing-child-margin-002.html [ Pass ]
 crbug.com/1066380 virtual/layout_ng_block_frag/external/wpt/css/css-break/widows-orphans-002.html [ Failure ]
 crbug.com/1066380 virtual/layout_ng_block_frag/external/wpt/css/css-break/widows-orphans-004.html [ Failure ]
 crbug.com/1058792 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/composited-under-clip-under-multicol.html [ Failure Crash ]
@@ -3595,6 +3597,8 @@ crbug.com/829028 external/wpt/css/css-break/break-between-avoid-003.html [ Failu
 crbug.com/829028 external/wpt/css/css-break/break-between-avoid-004.html [ Failure ]
 crbug.com/829028 external/wpt/css/css-break/overflowed-block-with-no-room-after-001.html [ Failure ]
 crbug.com/829028 external/wpt/css/css-break/overflowed-block-with-no-room-after-000.html [ Failure ]
+crbug.com/829028 external/wpt/css/css-break/trailing-child-margin-000.html [ Failure ]
+crbug.com/829028 external/wpt/css/css-break/trailing-child-margin-002.html [ Failure ]
 crbug.com/967329 external/wpt/css/css-multicol/columnfill-auto-max-height-001.html [ Failure ]
 crbug.com/967329 external/wpt/css/css-multicol/columnfill-auto-max-height-002.html [ Failure ]
 crbug.com/990240 external/wpt/css/css-multicol/multicol-breaking-000.html [ Failure ]

third_party/blink/web_tests/external/wpt/css/css-break/trailing-child-margin-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/#break-margins">
+<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; column-fill:auto; height:100px; background:red;">
+  <div style="display:flow-root; background:green;">
+    <div style="height:70px; margin-bottom:50px;"></div>
+  </div>
+  <!-- We're at the very beginning of the second column here, so inserting a
+       forced break will have no effect. -->
+  <div style="break-before:column; height:100px; background:green;"></div>
+</div>

third_party/blink/web_tests/external/wpt/css/css-break/trailing-child-margin-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/#break-margins">
+<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; column-fill:auto; height:100px; background:red;">
+  <div style="display:flow-root; background:green;">
+    <div style="height:70px; margin-bottom:50px;"></div>
+    <div style="height:100px;"></div>
+  </div>
+</div>

third_party/blink/web_tests/external/wpt/css/css-break/trailing-child-margin-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/#break-margins">
+<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; column-fill:auto; height:100px; background:red;">
+  <fieldset style="margin:0; border:none; padding:0; background:green;">
+    <div style="height:70px; margin-bottom:50px;"></div>
+  </fieldset>
+  <div style="break-before:column; height:100px; background:green;"></div>
+</div>