[LayoutNG] Fix replaced abspos with no intrinsic size

One more abspos edge case:
Replaced element, with no intrinsic size, but with
intrinsic aspect ratio.

The example problem here is an SVG with aspect ratio,
but no intinsic size. According to the spec, its
inline size should fill container, and block size
should be derived from inline size and aspect ratio.

Existing code did not handle this case. If element
had no size, its size would be computed and get clamped
by minmax.

A replaced element with no size gets a default minmax of
300x150, so elements would effectively get clamped to minmax.

My fix is a bit of a hack. It uses absence of minmax as a signal
that size should not be clamped.

The intrinsic size handling is surprisingly complex for replaced
elements. I wonder if there  are any other parts of NG that
need a close look in how replaced with aspect ratio/no size
are handled.

FF handles this correctly. Legacy does not.

Bug: 1003545
Change-Id: I1510611ff84a99a26bcade74cace2855e022f997
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1830326
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702150}

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

@@ -70,9 +70,26 @@ inline LayoutUnit StaticPositionEndInset(bool is_static_position_start,
          (is_static_position_start ? size : LayoutUnit());
 }
 
+// https://www.w3.org/TR/css-position-3/#abs-replaced-width
+// Handle the special case of an element with aspect ratio, but no intrinsic
+// width or height.
+LayoutUnit ComputeShrinkToFitSize(
+    const base::Optional<MinMaxSize>& child_minmax,
+    LayoutUnit computed_available_size,
+    LayoutUnit margin_start,
+    LayoutUnit margin_end) {
+  LayoutUnit size = (computed_available_size - margin_start - margin_end)
+                        .ClampNegativeToZero();
+  if (child_minmax)
+    return child_minmax->ShrinkToFit(size);
+  return size;
+}
+
 // Implement the absolute size resolution algorithm.
 // https://www.w3.org/TR/css-position-3/#abs-non-replaced-width
 // https://www.w3.org/TR/css-position-3/#abs-non-replaced-height
+// |child_minmax| can have no value if an element is replaced, and has no
+// intrinsic width or height, but has an aspect ratio.
 void ComputeAbsoluteSize(const LayoutUnit border_padding_size,
                          const base::Optional<MinMaxSize>& child_minmax,
                          const LayoutUnit margin_percentage_resolution_size,
@@ -123,14 +140,12 @@ void ComputeAbsoluteSize(const LayoutUnit border_padding_size,
       margin_start = LayoutUnit();
     if (!margin_end)
       margin_end = LayoutUnit();
-    DCHECK(child_minmax.has_value());
 
     LayoutUnit computed_available_size =
         is_static_position_start ? available_size - static_position_offset
                                  : static_position_offset;
-    size = child_minmax->ShrinkToFit(
-        (computed_available_size - *margin_start - *margin_end)
-            .ClampNegativeToZero());
+    size = ComputeShrinkToFitSize(child_minmax, computed_available_size,
+                                  *margin_start, *margin_end);
     LayoutUnit margin_size = *size + *margin_start + *margin_end;
     if (is_start_dominant) {
       inset_start = StaticPositionStartInset(
@@ -188,11 +203,9 @@ void ComputeAbsoluteSize(const LayoutUnit border_padding_size,
   if (!inset_start && !size) {
     // Rule 1: left/width are unknown.
     DCHECK(inset_end.has_value());
-    DCHECK(child_minmax.has_value());
     LayoutUnit computed_available_size = available_size - *inset_end;
-    size = child_minmax->ShrinkToFit(
-        (computed_available_size - *margin_start - *margin_end)
-            .ClampNegativeToZero());
+    size = ComputeShrinkToFitSize(child_minmax, computed_available_size,
+                                  *margin_start, *margin_end);
   } else if (!inset_start && !inset_end) {
     // Rule 2.
     DCHECK(size.has_value());
@@ -207,11 +220,9 @@ void ComputeAbsoluteSize(const LayoutUnit border_padding_size,
     }
   } else if (!size && !inset_end) {
     // Rule 3.
-    DCHECK(child_minmax.has_value());
     LayoutUnit computed_available_size = available_size - *inset_start;
-    size = child_minmax->ShrinkToFit(
-        (computed_available_size - *margin_start - *margin_end)
-            .ClampNegativeToZero());
+    size = ComputeShrinkToFitSize(child_minmax, computed_available_size,
+                                  *margin_start, *margin_end);
   }
 
   // Rules 4 through 6: 1 out of 3 are unknown.

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

@@ -549,11 +549,15 @@ LayoutUnit ComputeBlockSizeForFragment(
 }
 
 // Computes size for a replaced element.
-LogicalSize ComputeReplacedSize(
-    const NGLayoutInputNode& node,
-    const NGConstraintSpace& space,
-    const base::Optional<MinMaxSize>& child_minmax) {
+void ComputeReplacedSize(const NGLayoutInputNode& node,
+                         const NGConstraintSpace& space,
+                         const base::Optional<MinMaxSize>& child_minmax,
+                         base::Optional<LogicalSize>* out_replaced_size,
+                         base::Optional<LogicalSize>* out_aspect_ratio) {
   DCHECK(node.IsReplaced());
+  DCHECK(!out_replaced_size->has_value());
+  DCHECK(!out_aspect_ratio->has_value());
+
   const ComputedStyle& style = node.Style();
 
   NGBoxStrut border_padding =
@@ -587,8 +591,10 @@ LogicalSize ComputeReplacedSize(
         space.AvailableSize().block_size, LengthResolvePhase::kLayout);
     replaced_block = ConstrainByMinMax(*replaced_block, block_min, block_max);
   }
-  if (replaced_inline && replaced_block)
-    return LogicalSize(*replaced_inline, *replaced_block);
+  if (replaced_inline && replaced_block) {
+    out_replaced_size->emplace(*replaced_inline, *replaced_block);
+    return;
+  }
 
   base::Optional<LayoutUnit> intrinsic_inline;
   base::Optional<LayoutUnit> intrinsic_block;
@@ -613,10 +619,8 @@ LogicalSize ComputeReplacedSize(
                           aspect_ratio.inline_size / aspect_ratio.block_size) +
                          border_padding.InlineSum();
     } else {
-      NGBoxStrut margins = ComputeMarginsForSelf(space, node.Style());
-      intrinsic_inline =
-          (space.AvailableSize().inline_size - margins.InlineSum())
-              .ClampNegativeToZero();
+      *out_aspect_ratio = aspect_ratio;
+      return;
     }
   }
   if (!intrinsic_block) {
@@ -712,7 +716,8 @@ LogicalSize ComputeReplacedSize(
       }
     }
   }
-  return LogicalSize(*replaced_inline, *replaced_block);
+  out_replaced_size->emplace(*replaced_inline, *replaced_block);
+  return;
 }
 
 int ResolveUsedColumnCount(int computed_count,

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

@@ -255,10 +255,21 @@ ComputeBlockSizeForFragment(const NGConstraintSpace&,
                             const NGBoxStrut& border_padding,
                             LayoutUnit content_size);
 
-// Computes intrinsic size for replaced elements.
-CORE_EXPORT LogicalSize ComputeReplacedSize(const NGLayoutInputNode&,
-                                            const NGConstraintSpace&,
-                                            const base::Optional<MinMaxSize>&);
+// Intrinsic size for replaced elements is computed as:
+// - |out_replaced_size| intrinsic size of the element. It might have no value.
+// - |out_aspect_ratio| only set if out_replaced_size is empty.
+//   If out_replaced_size is not empty, that is the aspect ratio.
+// This routine will return one of the following:
+// - out_replaced_size, and no out_aspect_ratio
+// - out_aspect_ratio, and no out_replaced_size
+// - neither out_aspect_ratio, nor out_replaced_size
+// SVG elements can return any of the three options above.
+CORE_EXPORT void ComputeReplacedSize(
+    const NGLayoutInputNode&,
+    const NGConstraintSpace&,
+    const base::Optional<MinMaxSize>&,
+    base::Optional<LogicalSize>* out_replaced_size,
+    base::Optional<LogicalSize>* out_aspect_ratio);
 
 // Based on available inline size, CSS computed column-width, CSS computed
 // column-count and CSS used column-gap, return CSS used column-count.

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

@@ -570,9 +570,21 @@ scoped_refptr<const NGLayoutResult> NGOutOfFlowLayoutPart::Layout(
   }
 
   base::Optional<LogicalSize> replaced_size;
+  base::Optional<LogicalSize> replaced_aspect_ratio;
+  bool is_replaced_with_only_aspect_ratio = false;
   if (is_replaced) {
-    replaced_size =
-        ComputeReplacedSize(node, candidate_constraint_space, min_max_size);
+    ComputeReplacedSize(node, candidate_constraint_space, min_max_size,
+                        &replaced_size, &replaced_aspect_ratio);
+    is_replaced_with_only_aspect_ratio = !replaced_size &&
+                                         replaced_aspect_ratio &&
+                                         !replaced_aspect_ratio->IsEmpty();
+    // If we only have aspect ratio, and no replaced size, intrinsic size
+    // defaults to 300x150. min_max_size gets computed from the intrinsic size.
+    // We reset the min_max_size because spec says that OOF-positioned size
+    // should not be constrained by intrinsic size in this case.
+    // https://www.w3.org/TR/CSS22/visudet.html#inline-replaced-width
+    if (is_replaced_with_only_aspect_ratio)
+      min_max_size.reset();
   } else if (should_be_considered_as_replaced) {
     replaced_size =
         LogicalSize{min_max_size->ShrinkToFit(
@@ -590,6 +602,17 @@ scoped_refptr<const NGLayoutResult> NGOutOfFlowLayoutPart::Layout(
   if (!is_replaced && should_be_considered_as_replaced)
     replaced_size.reset();
 
+  // Replaced elements with only aspect ratio compute their block size from
+  // inline size and aspect ratio.
+  // https://www.w3.org/TR/css-sizing-3/#intrinsic-sizes
+  if (is_replaced_with_only_aspect_ratio) {
+    replaced_size = LogicalSize(
+        node_position.size.inline_size,
+        (replaced_aspect_ratio->block_size *
+         ((node_position.size.inline_size - border_padding.InlineSum()) /
+          replaced_aspect_ratio->inline_size)) +
+            border_padding.BlockSum());
+  }
   if (AbsoluteNeedsChildBlockSize(candidate_style)) {
     layout_result =
         GenerateFragment(node, container_content_size_in_candidate_writing_mode,

third_party/blink/web_tests/external/wpt/css/css-position/position-absolute-replaced-minmax.html

@@ -251,13 +251,20 @@
   data-expected-width="338" data-expected-height="182" data-offset-y="11" data-offset-x="59"
   >
 </div>
-<!-- Just viewbox. Has aspect_ration, but no intrinsic size
+<!-- Just viewbox. Has aspect_ratio, but no intrinsic size
  inline_width defaults to container width -->
 <div class="container">
   <img class="target" src="data:image/svg+xml,%3Csvg viewBox='0 0 100 10' xmlns='http://www.w3.org/2000/svg' %3E%3Crect width='100%' height='100%' style='fill:rgb(0,255,0);'/%3E%3C/svg%3E"
   data-expected-width="388" data-expected-height="67" data-offset-y="126" data-offset-x="9"
   >
 </div>
+<!-- Just viewbox. Has aspect_ratio, but no intrinsic size
+ inline_width is constrained by left/right, height computed proportionally -->
+<div class="container">
+  <img class="target" style="left:100px;right:100px" src="data:image/svg+xml,%3Csvg viewBox='0 0 100 10' xmlns='http://www.w3.org/2000/svg' %3E%3Crect width='100%' height='100%' style='fill:rgb(0,255,0);'/%3E%3C/svg%3E"
+  data-expected-width="188" data-expected-height="47" data-offset-y="146" data-offset-x="109"
+  >
+</div>
 <!-- Viewbox + svg width. Has aspect_ratio and width. Height is scaled -->
 <div class="container">
   <img class="target" src="data:image/svg+xml,%3Csvg viewBox='0 0 100 10' width='100px' xmlns='http://www.w3.org/2000/svg' %3E%3Crect width='100%' height='100%' style='fill:rgb(0,255,0);'/%3E%3C/svg%3E"

third_party/blink/web_tests/flag-specific/disable-blink-features=LayoutNG/external/wpt/css/css-position/position-absolute-replaced-minmax-expected.txt

@@ -34,7 +34,8 @@ PASS minmax replaced IMG svg 32
 PASS minmax replaced IMG svg 33
 PASS minmax replaced IMG 34
 FAIL minmax replaced IMG 35 assert_equals: incorrect offsetWidth expected "388" but got "426"
-PASS minmax replaced IMG 36
+FAIL minmax replaced IMG 36 assert_equals: incorrect offsetWidth expected "188" but got "426"
 PASS minmax replaced IMG 37
+PASS minmax replaced IMG 38
 Harness: the test ran to completion.