Avoid multicols shorter than tallest piece of unbreakable content.

Unless block-size constraints prevent it, attempt to make a multicol
container at least as tall as its tallest piece of unbreakable content.
This almost worked correctly, but not if the multicol container was
nested inside another fragmentation context. It would then be
constrained by the space left in the outer fragmentainer (in the case of
the tests now fixed that was 0 space left). Don't attempt to make it
shorter than the tallest piece of unbrekable content here, as that would
actually make it (the multicol container fragment itself) fit in the
current outer fragmentainer (while its contents would overflow), rather
than breaking before the inner multicol container and retrying in the
next outer fragmentainer.

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

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

@@ -922,7 +922,7 @@ LayoutUnit NGColumnLayoutAlgorithm::CalculateBalancedColumnBlockSize(
   // First split into content runs at explicit (forced) breaks.
   ContentRuns content_runs;
   scoped_refptr<const NGBlockBreakToken> break_token = child_break_token;
-  LayoutUnit tallest_unbreakable_block_size;
+  tallest_unbreakable_block_size_ = LayoutUnit();
   do {
     NGBlockLayoutAlgorithm balancing_algorithm(
         {Node(), fragment_geometry, space, break_token.get()});
@@ -938,8 +938,8 @@ LayoutUnit NGColumnLayoutAlgorithm::CalculateBalancedColumnBlockSize(
         CalculateColumnContentBlockSize(fragment, space.GetWritingDirection());
     content_runs.emplace_back(column_block_size);
 
-    tallest_unbreakable_block_size = std::max(
-        tallest_unbreakable_block_size, result->TallestUnbreakableBlockSize());
+    tallest_unbreakable_block_size_ = std::max(
+        tallest_unbreakable_block_size_, result->TallestUnbreakableBlockSize());
 
     // Stop when we reach a spanner. That's where this row of columns will end.
     if (result->ColumnSpanner())
@@ -964,7 +964,7 @@ LayoutUnit NGColumnLayoutAlgorithm::CalculateBalancedColumnBlockSize(
     // initial balancing pass, so it also wants to know the largest unbreakable
     // block-size.
     container_builder_.PropagateTallestUnbreakableBlockSize(
-        tallest_unbreakable_block_size);
+        tallest_unbreakable_block_size_);
   }
 
   // We now have an estimated minimal block-size for the columns. Roughly
@@ -973,12 +973,10 @@ LayoutUnit NGColumnLayoutAlgorithm::CalculateBalancedColumnBlockSize(
   // though, since there will typically be unbreakable pieces of content, such
   // as replaced content, lines of text, and other things. We need to actually
   // lay out into columns to figure out if they are tall enough or not (and
-  // stretch and retry if not). Also honor {,min-,max-}{height,width} properties
-  // before returning.
-  LayoutUnit block_size = std::max(content_runs.TallestColumnBlockSize(),
-                                   tallest_unbreakable_block_size);
-
-  return ConstrainColumnBlockSize(block_size);
+  // stretch and retry if not). Also honor {,min-,max-}block-size properties
+  // before returning, and also try to not become shorter than the tallest piece
+  // of unbreakable content.
+  return ConstrainColumnBlockSize(content_runs.TallestColumnBlockSize());
 }
 
 LayoutUnit NGColumnLayoutAlgorithm::StretchColumnBlockSize(
@@ -993,14 +991,6 @@ LayoutUnit NGColumnLayoutAlgorithm::StretchColumnBlockSize(
 // container.
 LayoutUnit NGColumnLayoutAlgorithm::ConstrainColumnBlockSize(
     LayoutUnit size) const {
-  // The {,max-}{height,width} properties are specified on the multicol
-  // container, but here we're calculating the column block sizes inside the
-  // multicol container, which isn't exactly the same. We may shrink the column
-  // block size here, but we'll never stretch it, because the value passed is
-  // the perfect balanced block size. Making it taller would only disrupt the
-  // balanced output, for no reason. The only thing we need to worry about here
-  // is to not overflow the multicol container.
-
   if (is_constrained_by_outer_fragmentation_context_) {
     // Don't become too tall to fit in the outer fragmentation context.
     LayoutUnit available_outer_space =
@@ -1009,6 +999,17 @@ LayoutUnit NGColumnLayoutAlgorithm::ConstrainColumnBlockSize(
     size = std::min(size, available_outer_space);
   }
 
+  // But avoid becoming shorter than the tallest piece of unbreakable content.
+  size = std::max(size, tallest_unbreakable_block_size_);
+
+  // The {,min-,max-}block-size properties are specified on the multicol
+  // container, but here we're calculating the column block sizes inside the
+  // multicol container, which isn't exactly the same. We may shrink the column
+  // block size here, but we'll never stretch them, because the value passed is
+  // the perfect balanced block size. Making it taller would only disrupt the
+  // balanced output, for no reason. The only thing we need to worry about here
+  // is to not overflow the multicol container.
+  //
   // First of all we need to convert the size to a value that can be compared
   // against the resolved properties on the multicol container. That means that
   // we have to convert the value from content-box to border-box.

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

@@ -100,6 +100,7 @@ class CORE_EXPORT NGColumnLayoutAlgorithm
   LayoutUnit column_inline_progression_;
   LayoutUnit column_block_size_;
   LayoutUnit intrinsic_block_size_;
+  LayoutUnit tallest_unbreakable_block_size_;
   bool is_constrained_by_outer_fragmentation_context_ = false;
 
   // This will be set during (outer) block fragmentation once we've processed

third_party/blink/web_tests/TestExpectations

@@ -1013,11 +1013,13 @@ virtual/layout_ng_block_frag/fast/multicol/vertical-lr/float-break.html [ Pass ]
 virtual/layout_ng_block_frag/fast/multicol/vertical-lr/float-content-break.html [ Pass ]
 virtual/layout_ng_block_frag/fast/multicol/vertical-lr/float-edge.html [ Pass ]
 virtual/layout_ng_block_frag/fast/multicol/vertical-lr/float-paginate.html [ Pass ]
+virtual/layout_ng_block_frag/fast/multicol/vertical-lr/nested-columns.html [ Pass ]
 virtual/layout_ng_block_frag/fast/multicol/vertical-lr/unsplittable-inline-block.html [ Pass ]
 virtual/layout_ng_block_frag/fast/multicol/vertical-rl/column-count-with-rules.html [ Pass ]
 virtual/layout_ng_block_frag/fast/multicol/vertical-rl/float-big-line.html [ Pass ]
 virtual/layout_ng_block_frag/fast/multicol/vertical-rl/float-content-break.html [ Pass ]
 virtual/layout_ng_block_frag/fast/multicol/vertical-rl/float-edge.html [ Pass ]
+virtual/layout_ng_block_frag/fast/multicol/vertical-rl/nested-columns.html [ Pass ]
 
 ### Tests failing with LayoutNGBlockFragmentation enabled:
 crbug.com/1079031 virtual/layout_ng_block_frag/external/wpt/css/css-break/block-end-aligned-abspos-nested.html [ Failure ]
@@ -1080,7 +1082,6 @@ crbug.com/1066626 virtual/layout_ng_block_frag/fast/multicol/mixed-opacity-fixed
 crbug.com/1079031 virtual/layout_ng_block_frag/fast/multicol/mixed-opacity-test.html [ Crash Failure ]
 crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/nested-auto-height-short-first-row.html [ Failure ]
 crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/nested-balanced-inner-column-count-1-with-forced-break.html [ Failure ]
-crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/nested-columns.html [ Failure ]
 crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/nested-short-first-row-extra-tall-line.html [ Failure ]
 crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/nested-short-first-row-unsplittable-block.html [ Failure ]
 crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/nested-uneven-inner-column-height.html [ Failure ]
@@ -1111,9 +1112,7 @@ crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/tall-float1.html [ F
 crbug.com/1079031 virtual/layout_ng_block_frag/fast/multicol/tall-line-in-short-block.html [ Failure ]
 crbug.com/1079031 virtual/layout_ng_block_frag/fast/multicol/transform-with-fixedpos.html [ Failure ]
 crbug.com/1058792 virtual/layout_ng_block_frag/fast/multicol/vertical-lr/composited-relpos-overlapping-will-change.html [ Failure ]
-crbug.com/829028 [ Mac ] virtual/layout_ng_block_frag/fast/multicol/vertical-lr/nested-columns.html [ Failure ]
 crbug.com/1058792 virtual/layout_ng_block_frag/fast/multicol/vertical-rl/composited-relpos-overlapping-will-change.html [ Failure ]
-crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/vertical-rl/nested-columns.html [ Crash Failure ]
 crbug.com/829028 virtual/layout_ng_block_frag/fast/multicol/widows.html [ Failure ]
 crbug.com/829028 virtual/layout_ng_block_frag/fragmentation/auto-scrollbar-shrink-to-fit.html [ Failure ]
 crbug.com/1079031 virtual/layout_ng_block_frag/fragmentation/content-preceding-first-fragmentainer.html [ Crash Failure ]