[LayoutNG] Don't truncate margins after spanners.

A column spanner forcefully breaks a column, so any leading margins on
content right after the spanner shouldn't be discarded.

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

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

@@ -42,6 +42,7 @@ NGBlockBreakToken::NGBlockBreakToken(PassKey key,
       num_children_(builder.child_break_tokens_.size()) {
   break_appeal_ = builder.break_appeal_;
   has_seen_all_children_ = builder.has_seen_all_children_;
+  is_caused_by_column_spanner_ = builder.FoundColumnSpanner();
   is_at_block_end_ = builder.is_at_block_end_;
   for (wtf_size_t i = 0; i < builder.child_break_tokens_.size(); ++i) {
     child_break_tokens_[i] = builder.child_break_tokens_[i].get();

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

@@ -72,6 +72,8 @@ class CORE_EXPORT NGBlockBreakToken final : public NGBreakToken {
 
   bool IsForcedBreak() const { return is_forced_break_; }
 
+  bool IsCausedByColumnSpanner() const { return is_caused_by_column_spanner_; }
+
   // Return true if all children have been "seen". When we have reached this
   // point, and resume layout in a fragmentainer, we should only process child
   // break tokens, if any, and not attempt to start laying out nodes that don't

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

@@ -2489,7 +2489,7 @@ NGConstraintSpace NGBlockLayoutAlgorithm::CreateConstraintSpaceForChild(
           container_builder_.AdjoiningObjectTypes());
     }
     builder.SetLinesUntilClamp(lines_until_clamp_);
-  } else if (child_data.is_resuming_after_break) {
+  } else if (child_data.allow_discard_start_margin) {
     // If the child is being resumed after a break, margins inside the child may
     // be adjoining with the fragmentainer boundary, regardless of whether the
     // child establishes a new formatting context or not.

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

@@ -45,7 +45,7 @@ struct NGInflowChildData {
   NGMarginStrut margin_strut;
   NGBoxStrut margins;
   bool margins_fully_resolved;
-  bool is_resuming_after_break;
+  bool allow_discard_start_margin;
 };
 
 // A class for general block layout (e.g. a <div> with no special style).

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

@@ -78,6 +78,7 @@ class CORE_EXPORT NGBreakToken : public RefCounted<NGBreakToken> {
         flags_(0),
         is_break_before_(false),
         is_forced_break_(false),
+        is_caused_by_column_spanner_(false),
         is_at_block_end_(false),
         break_appeal_(kBreakAppealPerfect),
         has_seen_all_children_(false) {
@@ -105,6 +106,8 @@ class CORE_EXPORT NGBreakToken : public RefCounted<NGBreakToken> {
 
   unsigned is_forced_break_ : 1;
 
+  unsigned is_caused_by_column_spanner_ : 1;
+
   // Set when layout is past the block-end border edge. If we break when we're
   // in this state, it means that something is overflowing, and thus establishes
   // a parallel flow.

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

@@ -566,9 +566,8 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow(
     scoped_refptr<const NGBlockBreakToken> column_break_token =
         next_column_token;
 
-    // This is the first column in this fragmentation context if there are no
-    // preceding columns in this row and there are also no preceding rows.
-    bool is_first_fragmentainer = !column_break_token && !BreakToken();
+    bool allow_discard_start_margin =
+        column_break_token && !column_break_token->IsCausedByColumnSpanner();
 
     LayoutUnit column_inline_offset(BorderScrollbarPadding().inline_start);
     int actual_column_count = 0;
@@ -584,7 +583,7 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow(
       // Lay out one column. Each column will become a fragment.
       NGConstraintSpace child_space = CreateConstraintSpaceForColumns(
           ConstraintSpace(), column_size, ColumnPercentageResolutionSize(),
-          is_first_fragmentainer, balance_columns);
+          allow_discard_start_margin, balance_columns);
 
       NGFragmentGeometry fragment_geometry =
           CalculateInitialFragmentGeometry(child_space, Node());
@@ -637,7 +636,7 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow(
         break;
       }
 
-      is_first_fragmentainer = false;
+      allow_discard_start_margin = true;
     } while (column_break_token);
 
     // TODO(mstensho): Nested column balancing.

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

@@ -735,7 +735,7 @@ NGConstraintSpace CreateConstraintSpaceForColumns(
     const NGConstraintSpace& parent_space,
     LogicalSize column_size,
     LogicalSize percentage_resolution_size,
-    bool is_first_fragmentainer,
+    bool allow_discard_start_margin,
     bool balance_columns) {
   NGConstraintSpaceBuilder space_builder(
       parent_space, parent_space.GetWritingMode(), /* is_new_fc */ true);
@@ -754,11 +754,12 @@ NGConstraintSpace CreateConstraintSpaceForColumns(
   space_builder.SetIsInColumnBfc();
   if (balance_columns)
     space_builder.SetIsInsideBalancedColumns();
-  if (!is_first_fragmentainer) {
-    // Margins at fragmentainer boundaries should be eaten and truncated to
-    // zero. Note that this doesn't apply to margins at forced breaks, but we'll
-    // deal with those when we get to them. Set up a margin strut that eats all
-    // leading adjacent margins.
+  if (allow_discard_start_margin) {
+    // Unless it's the first column in the multicol container, or the first
+    // column after a spanner, margins at fragmentainer boundaries should be
+    // eaten and truncated to zero. Note that this doesn't apply to margins at
+    // forced breaks, but we'll deal with those when we get to them. Set up a
+    // margin strut that eats all leading adjacent margins.
     space_builder.SetDiscardingMarginStrut();
   }
 

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

@@ -264,7 +264,7 @@ NGConstraintSpace CreateConstraintSpaceForColumns(
     const NGConstraintSpace& parent_space,
     LogicalSize column_size,
     LogicalSize percentage_resolution_size,
-    bool is_first_fragmentainer,
+    bool allow_discard_start_margin,
     bool balance_columns);
 
 // Return the adjusted child margin to be applied at the end of a fragment.

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

@@ -1172,7 +1172,16 @@ const NGConstraintSpace& NGOutOfFlowLayoutPart::GetFragmentainerConstraintSpace(
 
   wtf_size_t num_children = container_builder_->Children().size();
   bool is_new_fragment = index >= num_children;
-  bool is_first_fragmentainer = index == 0;
+  // Allow margins to be discarded if this is not the first column in the
+  // multicol container, and we're not right after a spanner.
+  //
+  // TODO(layout-dev): This check is incorrect in nested multicol. If the
+  // previous outer fragmentainer ended with regular column content (i.e. not a
+  // spanner), and this is the first column in the next outer fragmentainer, we
+  // should still discard margins, since there is no explicit break involved.
+  bool allow_discard_start_margin =
+      is_new_fragment || (index > 0 && container_builder_->Children()[index - 1]
+                                           .fragment->IsFragmentainerBox());
 
   // If we are a new fragment, find a non-spanner fragmentainer to base our
   // constraint space off of.
@@ -1213,7 +1222,7 @@ const NGConstraintSpace& NGOutOfFlowLayoutPart::GetFragmentainerConstraintSpace(
   NGConstraintSpace fragmentainer_constraint_space =
       CreateConstraintSpaceForColumns(*container_builder_->ConstraintSpace(),
                                       column_size, percentage_resolution_size,
-                                      is_first_fragmentainer,
+                                      allow_discard_start_margin,
                                       /* balance_columns */ false);
 
   return fragmentainer_constraint_space_map_

third_party/blink/web_tests/TestExpectations

@@ -979,6 +979,7 @@ virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-006
 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-015.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 ]
@@ -3270,6 +3271,7 @@ crbug.com/915204 external/wpt/css/css-multicol/multicol-span-all-008.html [ Fail
 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/829028 external/wpt/css/css-multicol/multicol-span-all-015.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-015.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="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; background:red;">
+  <div style="height:30px;"></div>
+  <div style="height:30px; background:green;"></div>
+  <div>
+    <div style="column-span:all; height:40px; background:green;">
+      <div style="width:50px; height:40px; background:red;"></div>
+    </div>
+  </div>
+  <div style="margin-top:-70px; height:130px; background:green;"></div>
+</div>