[LayoutNG] AddColumnResult() in sync with with column fragments actually added.

We'd fail to add column layout results to LayoutBox if a fragmentainer
got interrupted by a spanner, which would eventually mess up fragment
index calculation in AddColumnResult(), which in turn could make us keep
old fragments associated with deleted layout objects.

This fixes 4 out of 5 tests that failed because of bug 1105758.

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

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

@@ -544,7 +544,19 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow(
   // (colum balancing). Keep them in this list, and add them to the fragment
   // builder when we have the final column fragments. Or clear the list and
   // retry otherwise.
-  NGContainerFragmentBuilder::ChildrenVector new_columns;
+  struct ResultWithOffset {
+    scoped_refptr<const NGLayoutResult> result;
+    LogicalOffset offset;
+
+    ResultWithOffset(scoped_refptr<const NGLayoutResult> result,
+                     LogicalOffset offset)
+        : result(result), offset(offset) {}
+
+    const NGPhysicalBoxFragment& Fragment() const {
+      return To<NGPhysicalBoxFragment>(result->PhysicalFragment());
+    }
+  };
+  Vector<ResultWithOffset, 16> new_columns;
 
   scoped_refptr<const NGLayoutResult> result;
 
@@ -584,7 +596,7 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow(
       // Add the new column fragment to the list, but don't commit anything to
       // the fragment builder until we know whether these are the final columns.
       LogicalOffset logical_offset(column_inline_offset, column_block_offset);
-      new_columns.emplace_back(logical_offset, &result->PhysicalFragment());
+      new_columns.emplace_back(result, logical_offset);
 
       LayoutUnit space_shortage = result->MinimalSpaceShortage();
       if (space_shortage > LayoutUnit()) {
@@ -600,8 +612,6 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow(
       if (result->ColumnSpanner())
         break;
 
-      Node().AddColumnResult(result, column_break_token.get());
-
       column_break_token = To<NGBlockBreakToken>(column.BreakToken());
 
       // If we're participating in an outer fragmentation context, we'll only
@@ -698,8 +708,7 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow(
   // If there was no content inside to process, we don't want the resulting
   // empty column fragment.
   if (new_columns.size() == 1u) {
-    const NGPhysicalBoxFragment& column =
-        *To<NGPhysicalBoxFragment>(new_columns[0].fragment.get());
+    const NGPhysicalBoxFragment& column = new_columns[0].Fragment();
 
     if (column.Children().size() == 0) {
       // No content. Keep the trailing margin from any previous column spanner.
@@ -725,9 +734,12 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow(
   }
 
   // Commit all column fragments to the fragment builder.
-  for (auto column : new_columns) {
-    container_builder_.AddChild(To<NGPhysicalBoxFragment>(*column.fragment),
-                                column.offset);
+  const NGBlockBreakToken* incoming_column_token = next_column_token;
+  for (auto result_with_offset : new_columns) {
+    const NGPhysicalBoxFragment& fragment = result_with_offset.Fragment();
+    container_builder_.AddChild(fragment, result_with_offset.offset);
+    Node().AddColumnResult(result_with_offset.result, incoming_column_token);
+    incoming_column_token = To<NGBlockBreakToken>(fragment.BreakToken());
   }
 
   return result;

third_party/blink/web_tests/TestExpectations

@@ -1099,10 +1099,6 @@ crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/dynamic/change-secon
 crbug.com/1105758 virtual/layout_ng_block_frag/fast/multicol/dynamic/insert-spanner-before-content.html [ Crash ]
 crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/dynamic/insert-spanner-into-stf-constrained-width.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/dynamic/insert-spanner-into-stf-unconstrained-width.html [ Failure ]
-crbug.com/1105758 virtual/layout_ng_block_frag/fast/multicol/dynamic/insert-spanner-pseudo-after-then-content.html [ Crash ]
-crbug.com/1105758 virtual/layout_ng_block_frag/fast/multicol/dynamic/insert-spanner-pseudo-before-after-in-content.html [ Crash ]
-crbug.com/1105758 virtual/layout_ng_block_frag/fast/multicol/dynamic/insert-spanner-pseudo-before-following-content.html [ Crash ]
-crbug.com/1105758 virtual/layout_ng_block_frag/fast/multicol/dynamic/insert-spanner-pseudo-before.html [ Crash ]
 crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/dynamic/relpos-becomes-static-has-abspos.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/dynamic/remove-column-content-next-to-abspos-between-spanners.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/dynamic/static-becomes-relpos-has-abspos.html [ Crash Failure ]