Reland "[layoutng] Use the correct layout algorithm to compute min/max sizes"

This reverts commit 43df04fe.

Looks like there is no need for a flag-specific expectation anymore for
virtual/layout_ng_experimental/fast/multicol/flexbox-with-overflow-auto-child-crash.html

Original change's description:
> [layoutng] Use the correct layout algorithm to compute min/max sizes
>
> This also required adding a check in NGBlockNode::ComputeMinMaxSize
> for whether we are inside of PerformLayout -- on Mac, we can
> compute preferred sizes outside of layout; however, we can't do
> actual layout in that case, so we have to fallback to legacy
> min/max size in this case (c.f. RenderViewImpl::CheckPreferredSize())
>
> Bug: 635619
> Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
> Change-Id: I0caf0c6c79264074ccaf9f45b83a420950dab715
> Reviewed-on: https://chromium-review.googlesource.com/1131562
> Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
> Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575545}

Bug: 635619
Change-Id: I28ca2367e21327d9f911c75b9f9950d171d48e1b
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Reviewed-on: https://chromium-review.googlesource.com/1140206Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575872}

third_party/WebKit/LayoutTests/TestExpectations

@@ -766,6 +766,7 @@ crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/first-line-in-floa
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/fixedpos-child-becomes-static.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/flexbox-starts-at-column-boundary-with-block.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/flexbox-starts-at-column-boundary.html [ Failure ]
+crbug.com/864156 virtual/layout_ng_experimental/fast/multicol/flexbox-with-overflow-auto-child-crash.html [ Crash ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/flexbox.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/flipped-blocks-border-after.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/flipped-blocks-hit-test.html [ Failure ]
@@ -777,7 +778,6 @@ crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-content-brea
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-edge.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-margin-at-row-boundary-fixed-multicol-height.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-margin-at-row-boundary.html [ Failure ]
-crbug.com/591099 [ Mac ] virtual/layout_ng_experimental/fast/multicol/flexbox-with-overflow-auto-child-crash.html [ Timeout ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-moved-by-child-line-and-unbreakable.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-paginate-empty-lines.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/float-paginate.html [ Failure ]
@@ -870,10 +870,8 @@ crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/overflow-content.h
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/overflow-unsplittable.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/pageLogicalOffset-vertical.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/paginate-block-replaced.html [ Failure ]
-
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/positioned-outside-of-columns.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/positioned-split.html [ Failure ]
-crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/positioned-with-constrained-height.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/positive-leading.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/pushed-line-affected-by-float.html [ Failure ]
 crbug.com/591099 virtual/layout_ng_experimental/fast/multicol/relayout-and-push-float.html [ Failure ]

third_party/WebKit/LayoutTests/fast/multicol/intrinsic-width-expected.html

+<!DOCTYPE html>
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div style="width:100px; height:100px; background:green;"></div>

third_party/WebKit/LayoutTests/fast/multicol/intrinsic-width.html

+<!DOCTYPE html>
+<title>Intrinsic width calculation</title>
+
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div style="float:left; background: green;">
+  <div style="column-count: 2; column-gap: 0;">
+    <div style="width: 50px; height: 100px;"></div>
+    <div style="width: 50px; height: 100px;"></div>
+  </div>
+</div>

third_party/WebKit/LayoutTests/virtual/layout_ng_experimental/fast/multicol/flexbox-with-overflow-auto-child-crash-expected.txt

-PASS if no crash or assertion failure.
-
-

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

@@ -42,29 +42,34 @@ inline LayoutMultiColumnFlowThread* GetFlowThread(const LayoutBox& box) {
   return ToLayoutBlockFlow(box).MultiColumnFlowThread();
 }
 
-scoped_refptr<NGLayoutResult> LayoutWithAlgorithm(
-    const ComputedStyle& style,
-    NGBlockNode node,
-    LayoutBox* box,
-    const NGConstraintSpace& space,
-    NGBreakToken* break_token) {
-  auto* token = ToNGBlockBreakToken(break_token);
-  // TODO(dgrogan): Check display value instead of Layout node type?
-  if (box->IsLayoutNGFlexibleBox())
-    return NGFlexLayoutAlgorithm(node, space, token).Layout();
-  // If there's a legacy layout box, we can only do block fragmentation if we
-  // would have done block fragmentation with the legacy engine. Otherwise
-  // writing data back into the legacy tree will fail. Look for the flow
-  // thread.
-  if (!box || GetFlowThread(*box)) {
-    if (style.IsOverflowPaged())
-      return NGPageLayoutAlgorithm(node, space, token).Layout();
-    if (style.SpecifiesColumns())
-      return NGColumnLayoutAlgorithm(node, space, token).Layout();
-    NOTREACHED();
+#define WITH_ALGORITHM(ret, func, argdecl, args)                             \
+  ret func##WithAlgorithm(NGBlockNode node, const NGConstraintSpace& space,  \
+                          NGBreakToken* break_token, argdecl) {              \
+    auto* token = ToNGBlockBreakToken(break_token);                          \
+    const ComputedStyle& style = node.Style();                               \
+    if (node.GetLayoutBox()->IsLayoutNGFlexibleBox())                        \
+      return NGFlexLayoutAlgorithm(node, space, token).func args;            \
+    /* If there's a legacy layout box, we can only do block fragmentation if \
+     * we would have done block fragmentation with the legacy engine.        \
+     * Otherwise writing data back into the legacy tree will fail. Look for  \
+     * the flow thread. */                                                   \
+    if (GetFlowThread(*node.GetLayoutBox())) {                               \
+      if (style.IsOverflowPaged())                                           \
+        return NGPageLayoutAlgorithm(node, space, token).func args;          \
+      if (style.SpecifiesColumns())                                          \
+        return NGColumnLayoutAlgorithm(node, space, token).func args;        \
+      NOTREACHED();                                                          \
+    }                                                                        \
+    return NGBlockLayoutAlgorithm(node, space, token).func args;             \
   }
-  return NGBlockLayoutAlgorithm(node, space, token).Layout();
-}
+
+WITH_ALGORITHM(scoped_refptr<NGLayoutResult>, Layout, void*, ())
+WITH_ALGORITHM(base::Optional<MinMaxSize>,
+               ComputeMinMaxSize,
+               MinMaxSizeInput input,
+               (input))
+
+#undef WITH_ALGORITHM
 
 bool IsFloatFragment(const NGPhysicalFragment& fragment) {
   const LayoutObject* layout_object = fragment.GetLayoutObject();
@@ -236,8 +241,8 @@ scoped_refptr<NGLayoutResult> NGBlockNode::Layout(
     box_->ComputePreferredLogicalWidths();
   }
 
-  layout_result =
-      LayoutWithAlgorithm(Style(), *this, box_, constraint_space, break_token);
+  layout_result = LayoutWithAlgorithm(*this, constraint_space, break_token,
+                                      /* ignored */ nullptr);
   if (block_flow) {
     block_flow->SetCachedLayoutResult(constraint_space, break_token,
                                       layout_result);
@@ -280,16 +285,7 @@ MinMaxSize NGBlockNode::ComputeMinMaxSize(
   // if we're outside of layout, we can't do that. This can happen on Mac.
   if (!CanUseNewLayout() ||
       (is_orthogonal_flow_root && !box_->GetFrameView()->IsInPerformLayout())) {
-    // TODO(layout-ng): This could be somewhat optimized by directly calling
-    // computeIntrinsicLogicalWidths, but that function is currently private.
-    // Consider doing that if this becomes a performance issue.
-    sizes.min_size = box_->ComputeLogicalWidthUsing(
-        kMainOrPreferredSize, Length(kMinContent), LayoutUnit(),
-        box_->ContainingBlock());
-    sizes.max_size = box_->ComputeLogicalWidthUsing(
-        kMainOrPreferredSize, Length(kMaxContent), LayoutUnit(),
-        box_->ContainingBlock());
-    return sizes;
+    return ComputeMinMaxSizeFromLegacy();
   }
 
   scoped_refptr<NGConstraintSpace> zero_constraint_space =
@@ -314,13 +310,18 @@ MinMaxSize NGBlockNode::ComputeMinMaxSize(
     return sizes;
   }
 
-  // TODO(layout-ng): We need to make sure to use the right algorithm
-  NGBlockLayoutAlgorithm minmax_algorithm(*this, *constraint_space);
   base::Optional<MinMaxSize> maybe_sizes =
-      minmax_algorithm.ComputeMinMaxSize(input);
+      ComputeMinMaxSizeWithAlgorithm(*this, *constraint_space,
+                                     /* break token */ nullptr, input);
   if (maybe_sizes.has_value())
     return *maybe_sizes;
 
+  if (!box_->GetFrameView()->IsInPerformLayout()) {
+    // We can't synthesize these using Layout() if we're not in PerformLayout.
+    // This situation can happen on mac. Fall back to legacy instead.
+    return ComputeMinMaxSizeFromLegacy();
+  }
+
   // Have to synthesize this value.
   scoped_refptr<NGLayoutResult> layout_result = Layout(*zero_constraint_space);
   NGBoxFragment min_fragment(
@@ -343,6 +344,20 @@ MinMaxSize NGBlockNode::ComputeMinMaxSize(
   return sizes;
 }
 
+MinMaxSize NGBlockNode::ComputeMinMaxSizeFromLegacy() const {
+  // TODO(layout-ng): This could be somewhat optimized by directly calling
+  // computeIntrinsicLogicalWidths, but that function is currently private.
+  // Consider doing that if this becomes a performance issue.
+  MinMaxSize sizes;
+  sizes.min_size =
+      box_->ComputeLogicalWidthUsing(kMainOrPreferredSize, Length(kMinContent),
+                                     LayoutUnit(), box_->ContainingBlock());
+  sizes.max_size =
+      box_->ComputeLogicalWidthUsing(kMainOrPreferredSize, Length(kMaxContent),
+                                     LayoutUnit(), box_->ContainingBlock());
+  return sizes;
+}
+
 NGBoxStrut NGBlockNode::GetScrollbarSizes() const {
   NGPhysicalBoxStrut sizes;
   const ComputedStyle& style = box_->StyleRef();

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

@@ -53,6 +53,8 @@ class CORE_EXPORT NGBlockNode final : public NGLayoutInputNode {
                                const MinMaxSizeInput&,
                                const NGConstraintSpace* = nullptr);
 
+  MinMaxSize ComputeMinMaxSizeFromLegacy() const;
+
   NGBoxStrut GetScrollbarSizes() const;
 
   NGLayoutInputNode FirstChild() const;