[css-flex] Fix flex base size for some aspect ratio items

Old behavior: Use max content contribution for flex base size, which is
roughly: use computed width if it exists, otherwise use intrinsic size
after applying the aspect ratio and min/main/max height. In either
case, apply min/max width.

New behavior is similar to the 'max-content' size: use the intrinsic
width after applying the aspect ratio and min/main/max height. Ignore
computed min/main/max width.

Also, we weren't returning available size as the max-content size of an
SVG item that only has an aspect ratio and no intrinsic sizes, which is
what Firefox does and what the spec dictates.

Both of these are fixed with a new aspect-ratio sizing method that
currently lives in flex but will be moved to NGReplacedLayoutAlgorithm
when it exists.

Change-Id: I4afb382a7604a4fcd0626f0702edf72f21cd0bcc
Bug: 987000
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2399082Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809430}

third_party/blink/renderer/core/layout/ng/flex/ng_flex_layout_algorithm.cc

@@ -10,6 +10,7 @@
 #include "third_party/blink/renderer/core/layout/layout_button.h"
 #include "third_party/blink/renderer/core/layout/layout_flexible_box.h"
 #include "third_party/blink/renderer/core/layout/ng/flex/ng_flex_child_iterator.h"
+#include "third_party/blink/renderer/core/layout/ng/geometry/ng_box_strut.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_block_break_token.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_box_fragment.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_constraint_space.h"
@@ -21,6 +22,7 @@
 #include "third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_space_utils.h"
 #include "third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h"
+#include "third_party/blink/renderer/core/style/computed_style_base_constants.h"
 #include "third_party/blink/renderer/core/style/computed_style_constants.h"
 #include "third_party/blink/renderer/platform/wtf/vector.h"
 
@@ -407,6 +409,61 @@ NGConstraintSpace NGFlexLayoutAlgorithm::BuildSpaceForFlexBasis(
   return space_builder.ToConstraintSpace();
 }
 
+namespace {
+
+// This function will be superseded by
+// NGReplacedLayoutAlgorithm.ComputeMinMaxSizes, when such method exists.
+LayoutUnit ComputeIntrinsicInlineSizeForAspectRatioElement(
+    const NGBlockNode& node,
+    const NGConstraintSpace& space,
+    const MinMaxSizes& used_min_max_block_sizes) {
+  DCHECK(node.HasAspectRatio());
+  LogicalSize aspect_ratio = node.GetAspectRatio();
+  const ComputedStyle& style = node.Style();
+  NGBoxStrut border_padding =
+      ComputeBorders(space, node) + ComputePadding(space, style);
+
+  DCHECK_NE(style.LogicalHeight().GetType(), Length::Type::kFixed)
+      << "Flex will not use this function if the block size of the replaced "
+         "element is definite.";
+
+  base::Optional<LayoutUnit> intrinsic_inline;
+  base::Optional<LayoutUnit> intrinsic_block;
+  node.IntrinsicSize(&intrinsic_inline, &intrinsic_block);
+
+  // intrinsic_inline and intrinsic_block can be empty independent of each
+  // other.
+  if (intrinsic_inline) {
+    MinMaxSizes inline_min_max = ComputeTransferredMinMaxInlineSizes(
+        aspect_ratio, used_min_max_block_sizes, border_padding,
+        EBoxSizing::kContentBox);
+    LayoutUnit intrinsic_inline_border_box =
+        *intrinsic_inline + border_padding.InlineSum();
+    return inline_min_max.ClampSizeToMinAndMax(intrinsic_inline_border_box);
+  }
+
+  if (intrinsic_block) {
+    intrinsic_block = *intrinsic_block + border_padding.BlockSum();
+    intrinsic_block =
+        used_min_max_block_sizes.ClampSizeToMinAndMax(*intrinsic_block);
+    return InlineSizeFromAspectRatio(border_padding, aspect_ratio,
+                                     EBoxSizing::kContentBox, *intrinsic_block);
+  }
+
+  // If control flow reaches here, the item has aspect ratio only, no natural
+  // sizes. Spec says:
+  // * If the available space is definite in the inline axis, use the stretch
+  // fit into that size for the inline size and calculate the block size using
+  // the aspect ratio.
+  // https://drafts.csswg.org/css-sizing-3/#intrinsic-sizes
+  DCHECK_NE(space.AvailableSize().inline_size, kIndefiniteSize);
+  NGBoxStrut margins = ComputeMarginsForSelf(space, style);
+  return (space.AvailableSize().inline_size - margins.InlineSum())
+      .ClampNegativeToZero();
+}
+
+}  // namespace
+
 void NGFlexLayoutAlgorithm::ConstructAndAppendFlexItems() {
   NGFlexChildIterator iterator(Node());
   for (NGBlockNode child = iterator.NextChild(); child;
@@ -564,16 +621,28 @@ void NGFlexLayoutAlgorithm::ConstructAndAppendFlexItems() {
         // containers AND children) row flex containers. I _think_ the C and D
         // cases are correctly handled by this code, which was originally
         // written for case E.
-        if (child.HasAspectRatio()) {
-          // Legacy uses child.PreferredLogicalWidths() for this case, which
-          // is not exactly correct.
-          // TODO(dgrogan): Replace with a variant of ComputeReplacedSize that
-          // ignores min-width, width, max-width.
-          MinMaxSizesInput input(child_percentage_size_.block_size,
-                                 MinMaxSizesType::kContent);
-          flex_base_border_box =
-              ComputeMinAndMaxContentContribution(Style(), child, input)
-                  .sizes.max_size;
+
+        // Non-replaced AspectRatio items work fine using
+        // MinMaxSizesFunc(MinMaxSizesType::kContent).sizes.max_size below, so
+        // they don't need to use
+        // ComputeIntrinsicInlineSizeForAspectRatioElement.
+        // Also, ComputeIntrinsicInlineSizeForAspectRatioElement DCHECKs for
+        // non-replaced items.
+        if (child.HasAspectRatio() && child.IsReplaced()) {
+          if (RuntimeEnabledFeatures::FlexAspectRatioEnabled()) {
+            // Legacy uses child.PreferredLogicalWidths() for this case, which
+            // is not exactly correct.
+            flex_base_border_box =
+                ComputeIntrinsicInlineSizeForAspectRatioElement(
+                    child, flex_basis_space,
+                    min_max_sizes_in_cross_axis_direction);
+          } else {
+            MinMaxSizesInput input(child_percentage_size_.block_size,
+                                   MinMaxSizesType::kContent);
+            flex_base_border_box =
+                ComputeMinAndMaxContentContribution(Style(), child, input)
+                    .sizes.max_size;
+          }
         } else {
           flex_base_border_box =
               MinMaxSizesFunc(MinMaxSizesType::kContent).sizes.max_size;

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

@@ -495,6 +495,26 @@ MinMaxSizes ComputeMinMaxBlockSize(
   return result;
 }
 
+MinMaxSizes ComputeTransferredMinMaxInlineSizes(
+    const LogicalSize& ratio,
+    const MinMaxSizes& block_min_max,
+    const NGBoxStrut& border_padding,
+    const EBoxSizing sizing) {
+  MinMaxSizes transferred_min_max = {LayoutUnit(), LayoutUnit::Max()};
+  if (block_min_max.min_size > LayoutUnit()) {
+    transferred_min_max.min_size = InlineSizeFromAspectRatio(
+        border_padding, ratio, sizing, block_min_max.min_size);
+  }
+  if (block_min_max.max_size != LayoutUnit::Max()) {
+    transferred_min_max.max_size = InlineSizeFromAspectRatio(
+        border_padding, ratio, sizing, block_min_max.max_size);
+  }
+  // Minimum size wins over maximum size.
+  transferred_min_max.max_size =
+      std::max(transferred_min_max.max_size, transferred_min_max.min_size);
+  return transferred_min_max;
+}
+
 MinMaxSizes ComputeMinMaxInlineSizesFromAspectRatio(
     const NGConstraintSpace& constraint_space,
     const ComputedStyle& style,
@@ -514,19 +534,8 @@ MinMaxSizes ComputeMinMaxInlineSizesFromAspectRatio(
   MinMaxSizes block_min_max =
       ComputeMinMaxBlockSize(constraint_space, style, border_padding,
                              /* content_size */ kIndefiniteSize);
-  MinMaxSizes transferred_min_max = {LayoutUnit(), LayoutUnit::Max()};
-  if (block_min_max.min_size > LayoutUnit()) {
-    transferred_min_max.min_size = InlineSizeFromAspectRatio(
-        border_padding, ratio, style.BoxSizing(), block_min_max.min_size);
-  }
-  if (block_min_max.max_size != LayoutUnit::Max()) {
-    transferred_min_max.max_size = InlineSizeFromAspectRatio(
-        border_padding, ratio, style.BoxSizing(), block_min_max.max_size);
-  }
-  // Minimum size wins over maximum size.
-  transferred_min_max.max_size =
-      std::max(transferred_min_max.max_size, transferred_min_max.min_size);
-  return transferred_min_max;
+  return ComputeTransferredMinMaxInlineSizes(ratio, block_min_max,
+                                             border_padding, style.BoxSizing());
 }
 
 namespace {

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

@@ -318,8 +318,17 @@ MinMaxSizes ComputeMinMaxBlockSize(
     const LayoutUnit* opt_percentage_resolution_block_size_for_min_max =
         nullptr);
 
+MinMaxSizes ComputeTransferredMinMaxInlineSizes(
+    const LogicalSize& ratio,
+    const MinMaxSizes& block_min_max,
+    const NGBoxStrut& border_padding,
+    const EBoxSizing sizing);
+
 // Computes the transferred min/max inline sizes from the min/max block
 // sizes and the aspect ratio.
+// This will compute the min/max block sizes for you, but it only works with
+// styles that have a LogicalAspectRatio. It doesn't work if the aspect ratio is
+// coming from a replaced element.
 MinMaxSizes ComputeMinMaxInlineSizesFromAspectRatio(
     const NGConstraintSpace&,
     const ComputedStyle&,

third_party/blink/web_tests/FlagExpectations/disable-layout-ng

@@ -483,6 +483,9 @@ crbug.com/721123 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftest
 crbug.com/721123 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-intrinsic-ratio-007v.html [ Failure ]
 crbug.com/704294 external/wpt/css/css-flexbox/flex-aspect-ratio-img-row-006.html [ Failure ]
 crbug.com/704294 external/wpt/css/css-flexbox/canvas-dynamic-change-001.html [ Failure ]
+crbug.com/987000 external/wpt/css/css-flexbox/flex-aspect-ratio-img-row-007.html [ Failure ]
+crbug.com/987000 external/wpt/css/css-flexbox/flex-aspect-ratio-img-row-010.html [ Failure ]
+crbug.com/987000 external/wpt/css/css-flexbox/svg-root-as-flex-item-002.html [ Failure ]
 
 # These would need a rebaseline back from LayoutNGBlockFlow to LayoutBlockFlow
 crbug.com/864567 paint/float/float-under-inline-self-painting-change.html [ Failure ]

third_party/blink/web_tests/external/wpt/css/css-flexbox/flex-aspect-ratio-img-row-007.html

+<!doctype html>
+<title>Aspect-ratio flex item</title>
+<link rel="author" title="David Grogan" href="mailto:dgrogan@chromium.org">
+<link rel="help" href="https://drafts.csswg.org/css-flexbox/#algo-main-item" title="Part E">
+<link rel="match" href="../reference/ref-filled-green-100px-square-only.html">
+<meta name="assert" content="flex base size is not influenced by specified min-width for aspect ratio items">
+
+<p>Test passes if there is a filled green square.</p>
+
+<!-- Chrome 86 makes the green rectangle be 149.5 x 100. -->
+
+<div style="display: flex; width:200px;">
+  <img style="min-width: 100px; flex: 1 0 auto;" src="support/1x1-green.png">
+  <div style="flex: 1 0 1px;"></div>
+</div>

third_party/blink/web_tests/external/wpt/css/css-flexbox/flex-aspect-ratio-img-row-008.html

+<!DOCTYPE html>
+<title>SVG img as flex item</title>
+<link rel="author" title="David Grogan" href="mailto:dgrogan@chromium.org">
+<link rel="help" href="https://drafts.csswg.org/css-flexbox/#algo-main-item" title="Part E">
+<link rel="match" href="../reference/ref-filled-green-100px-square-only.html">
+<meta name="assert" content="SVG's specified intrinsic width is honored when used as a flex base size and no intrinsic height is specified." />
+
+<p>Test passes if there is a filled green square.</p>
+
+<div style="display: flex; width: 100px;">
+  <img src="data:image/svg+xml,%3Csvg viewBox='0 0 200 400' width='50px' xmlns='http://www.w3.org/2000/svg' %3E%3Crect width='100%' fill='green' /%3E%3C/svg%3E" style="border-left: 50px solid green; min-width: 0px;">
+</div>

third_party/blink/web_tests/external/wpt/css/css-flexbox/flex-aspect-ratio-img-row-009.html

+<!DOCTYPE html>
+<title>SVG img as flex item</title>
+<link rel="author" title="David Grogan" href="mailto:dgrogan@chromium.org">
+<link rel="help" href="https://drafts.csswg.org/css-flexbox/#algo-main-item" title="Part E">
+<link rel="help" href="https://drafts.csswg.org/css-flexbox/#min-size-auto">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<meta name="assert" content="min-width: auto honor's an SVG's intrinsic width when it has an aspect ratio and no specified intrinsic height." />
+
+<style>
+#reference-overlapped-red {
+  position: absolute;
+  background-color: red;
+  width: 100px;
+  height: 100px;
+  z-index: -1;
+}
+</style>
+
+<!-- Firefox 81b8 fails this test, probably because of https://bugzilla.mozilla.org/show_bug.cgi?id=1136312 -->
+
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<div id=reference-overlapped-red></div>
+
+<div style="display: flex; width: 50px;">
+  <img src="data:image/svg+xml,%3Csvg viewBox='0 0 200 400' width='50px' xmlns='http://www.w3.org/2000/svg' %3E%3Crect width='100%' fill='green' /%3E%3C/svg%3E" style="border-left: 50px solid green;">
+</div>

third_party/blink/web_tests/external/wpt/css/css-flexbox/flex-aspect-ratio-img-row-010.html

+<!DOCTYPE html>
+<title>SVG img as flex item</title>
+<link rel="author" title="David Grogan" href="mailto:dgrogan@chromium.org">
+<link rel="help" href="https://drafts.csswg.org/css-flexbox/#algo-main-item" title="Part E">
+<link rel="help" href="https://www.w3.org/TR/CSS22/visudet.html#min-max-widths" title="h > max-height line of the table">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<meta name="assert" content="Flex base size of image item with aspect ratio + intrinsic width + no intrinsic height honors transferred max-height." />
+
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+
+<div style="display: flex;">
+  <img src="data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 100 100' width='200'%3E%3Crect width='100%25' height='100%25' fill='green'/%3E%3C/svg%3E" style="max-height: 100px; flex: 0 0 auto; background: red;">
+</div>

third_party/blink/web_tests/external/wpt/css/css-flexbox/flex-aspect-ratio-img-row-011.html

+<!DOCTYPE html>
+<title>SVG img as flex item</title>
+<link rel="author" title="David Grogan" href="mailto:dgrogan@chromium.org">
+<link rel="help" href="https://drafts.csswg.org/css-flexbox/#algo-main-item" title="Part E">
+<link rel="help" href="https://www.w3.org/TR/CSS22/visudet.html#min-max-widths" title="h > max-height line of the table">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<meta name="assert" content="Flex base size of image item with aspect ratio + intrinsic height + no intrinsic width honors transferred max-height." />
+
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+
+<div style="display: flex;">
+  <img src="data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 100 100' height='200'%3E%3Crect width='100%25' height='100%25' fill='green'/%3E%3C/svg%3E" style="max-height: 100px; flex: 0 0 auto; background: red;">
+</div>

third_party/blink/web_tests/external/wpt/css/css-flexbox/svg-root-as-flex-item-002.html

+<!DOCTYPE html>
+<title>SVG root as flex item</title>
+<link rel="author" title="David Grogan" href="mailto:dgrogan@chromium.org">
+<link rel="help" href="https://drafts.csswg.org/css-flexbox/#algo-main-item" title="Part E">
+<link rel="help" href="https://www.w3.org/TR/css-sizing-3/#intrinsic-sizes" title="For boxes with an intrinsic aspect ratio, but no intrinsic size">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<meta name="assert" content="SVG's intrinsic width when used as flex base size stretches into the available size when it has no specified intrinsic sizes." />
+
+<style>
+#reference-overlapped-red {
+  position: absolute;
+  background-color: red;
+  width: 100px;
+  height: 100px;
+  z-index: -1;
+}
+</style>
+
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+
+<div id="reference-overlapped-red"></div>
+
+<div style="display: flex; width: 100px;">
+  <svg viewBox="0 0 200 200">
+    <rect width="100%" height="100%" fill="green" />
+  </svg>
+</div>

third_party/blink/web_tests/external/wpt/css/css-flexbox/svg-root-as-flex-item-003.html

+<!DOCTYPE html>
+<title>SVG root as flex item</title>
+<link rel="author" title="David Grogan" href="mailto:dgrogan@chromium.org">
+<link rel="help" href="https://drafts.csswg.org/css-flexbox/#algo-main-item" title="Part E">
+<link rel="help" href="https://www.w3.org/TR/css-sizing-3/#intrinsic-sizes" title="For boxes with an intrinsic aspect ratio, but no intrinsic size">
+<link rel="match" href="../reference/ref-filled-green-100px-square-only.html">
+<meta name="assert" content="SVG's intrinsic width when used as flex base size stretches into the available size (minus inline margins) when it has no specified intrinsic sizes." />
+
+<p>Test passes if there is a filled green square.</p>
+
+<div style="display: flex; width: 150px;">
+  <svg viewBox="0 0 200 200" style="margin-right: 50px; flex: 1 0 auto;">
+    <rect width="100%" height="100%" fill="green" />
+  </svg>
+</div>