Map the aspect ratio from width/height to CSS

Instead of having special code in image layout, map the aspect ratio that's
computed from the width and height attributes to the aspect-ratio property.

As a side-effect, this patch will also make the aspect-ratio property work
when contain: size is specified in an image (as well as error images when
no alt text is specified).

As specified in whatwg/html#6032.

R=masonfreed@chromium.org, xiaochengh@chromium.org

Bug: 1137004
Change-Id: I1253fa2ecf1e176711d3c4d7bd4159f466656ae8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2495560
Reviewed-by: Yu Han <yuzhehan@chromium.org>
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Auto-Submit: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823124}

third_party/blink/renderer/core/html/html_element.cc

@@ -30,9 +30,12 @@
 #include "third_party/blink/renderer/bindings/core/v8/string_or_trusted_script.h"
 #include "third_party/blink/renderer/bindings/core/v8/string_treat_null_as_empty_string_or_trusted_script.h"
 #include "third_party/blink/renderer/core/css/css_color_value.h"
+#include "third_party/blink/renderer/core/css/css_identifier_value.h"
 #include "third_party/blink/renderer/core/css/css_markup.h"
+#include "third_party/blink/renderer/core/css/css_numeric_literal_value.h"
 #include "third_party/blink/renderer/core/css/css_property_names.h"
 #include "third_party/blink/renderer/core/css/css_property_value_set.h"
+#include "third_party/blink/renderer/core/css/css_value_list.h"
 #include "third_party/blink/renderer/core/css/style_change_reason.h"
 #include "third_party/blink/renderer/core/css_value_keywords.h"
 #include "third_party/blink/renderer/core/dom/document_fragment.h"
@@ -883,6 +886,31 @@ void HTMLElement::setOuterText(const String& text,
     MergeWithNextTextNode(prev_text_node, exception_state);
 }
 
+void HTMLElement::ApplyAspectRatioToStyle(const AtomicString& width,
+                                          const AtomicString& height,
+                                          MutableCSSPropertyValueSet* style) {
+  HTMLDimension width_dim, height_dim;
+  if (!ParseDimensionValue(width, width_dim))
+    return;
+  if (!ParseDimensionValue(height, height_dim))
+    return;
+  if (!width_dim.IsAbsolute() || !height_dim.IsAbsolute())
+    return;
+  CSSValue* width_val = CSSNumericLiteralValue::Create(
+      width_dim.Value(), CSSPrimitiveValue::UnitType::kNumber);
+  CSSValue* height_val = CSSNumericLiteralValue::Create(
+      height_dim.Value(), CSSPrimitiveValue::UnitType::kNumber);
+  CSSValueList* ratio_list = CSSValueList::CreateSlashSeparated();
+  ratio_list->Append(*width_val);
+  ratio_list->Append(*height_val);
+
+  CSSValueList* list = CSSValueList::CreateSpaceSeparated();
+  list->Append(*CSSIdentifierValue::Create(CSSValueID::kAuto));
+  list->Append(*ratio_list);
+
+  style->SetProperty(CSSPropertyID::kAspectRatio, *list);
+}
+
 void HTMLElement::ApplyAlignmentAttributeToStyle(
     const AtomicString& alignment,
     MutableCSSPropertyValueSet* style) {

third_party/blink/renderer/core/html/html_element.h

@@ -165,6 +165,9 @@ class CORE_EXPORT HTMLElement : public Element {
                            CSSPropertyID,
                            const String& color);
 
+  void ApplyAspectRatioToStyle(const AtomicString& width,
+                               const AtomicString& height,
+                               MutableCSSPropertyValueSet*);
   void ApplyAlignmentAttributeToStyle(const AtomicString&,
                                       MutableCSSPropertyValueSet*);
   void ApplyBorderAttributeToStyle(const AtomicString&,

third_party/blink/renderer/core/html/html_image_element.cc

@@ -169,8 +169,16 @@ void HTMLImageElement::CollectStyleForPresentationAttribute(
     MutableCSSPropertyValueSet* style) {
   if (name == html_names::kWidthAttr) {
     AddHTMLLengthToStyle(style, CSSPropertyID::kWidth, value);
+    if (FastHasAttribute(html_names::kHeightAttr)) {
+      const AtomicString& height = FastGetAttribute(html_names::kHeightAttr);
+      ApplyAspectRatioToStyle(value, height, style);
+    }
   } else if (name == html_names::kHeightAttr) {
     AddHTMLLengthToStyle(style, CSSPropertyID::kHeight, value);
+    if (FastHasAttribute(html_names::kWidthAttr)) {
+      const AtomicString& width = FastGetAttribute(html_names::kWidthAttr);
+      ApplyAspectRatioToStyle(width, value, style);
+    }
   } else if (name == html_names::kBorderAttr) {
     ApplyBorderAttributeToStyle(value, style);
   } else if (name == html_names::kVspaceAttr) {

third_party/blink/renderer/core/html/html_image_fallback_helper.cc

@@ -179,10 +179,14 @@ void HTMLImageFallbackHelper::CustomStyleForAltText(Element& element,
   bool image_has_intrinsic_dimensions =
       new_style.Width().IsSpecifiedOrIntrinsic() &&
       new_style.Height().IsSpecifiedOrIntrinsic();
+  bool image_has_dimensions_from_ar =
+      !new_style.AspectRatio().IsAuto() &&
+      (new_style.Width().IsSpecifiedOrIntrinsic() ||
+       new_style.Height().IsSpecifiedOrIntrinsic());
   bool image_has_no_alt_attribute =
       element.getAttribute(html_names::kAltAttr).IsEmpty();
   bool treat_as_replaced =
-      image_has_intrinsic_dimensions &&
+      (image_has_intrinsic_dimensions || image_has_dimensions_from_ar) &&
       (element.GetDocument().InQuirksMode() || image_has_no_alt_attribute);
   if (treat_as_replaced) {
     // https://html.spec.whatwg.org/C/#images-3:
@@ -209,6 +213,8 @@ void HTMLImageFallbackHelper::CustomStyleForAltText(Element& element,
     if (new_style.Display() == EDisplay::kInline) {
       new_style.SetWidth(Length());
       new_style.SetHeight(Length());
+      new_style.SetAspectRatio(
+          ComputedStyleInitialValues::InitialAspectRatio());
     }
     if (ElementRepresentsNothing(element)) {
       // "If the element is an img element that represents nothing and the user

third_party/blink/renderer/core/layout/layout_replaced.cc

@@ -723,24 +723,6 @@ void LayoutReplaced::ComputeIntrinsicSizingInfo(
 
   if (!intrinsic_sizing_info.size.IsEmpty())
     intrinsic_sizing_info.aspect_ratio = intrinsic_sizing_info.size;
-
-  auto* elem = DynamicTo<Element>(GetNode());
-  if (elem && IsA<HTMLImageElement>(elem) &&
-      intrinsic_sizing_info.aspect_ratio.IsEmpty() &&
-      elem->FastHasAttribute(html_names::kWidthAttr) &&
-      elem->FastHasAttribute(html_names::kHeightAttr)) {
-    const AtomicString& width_str =
-        elem->FastGetAttribute(html_names::kWidthAttr);
-    const AtomicString& height_str =
-        elem->FastGetAttribute(html_names::kHeightAttr);
-    HTMLDimension width_dim, height_dim;
-    if (ParseDimensionValue(width_str, width_dim) &&
-        ParseDimensionValue(height_str, height_dim) && width_dim.IsAbsolute() &&
-        height_dim.IsAbsolute()) {
-      intrinsic_sizing_info.aspect_ratio.SetWidth(width_dim.Value());
-      intrinsic_sizing_info.aspect_ratio.SetHeight(height_dim.Value());
-    }
-  }
 }
 
 static inline LayoutUnit ResolveWidthForRatio(LayoutUnit height,

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

@@ -1538,7 +1538,8 @@ LogicalSize NGBlockNode::GetAspectRatio() const {
   // The CSS parser will ensure that this will only be set if the feature
   // is enabled.
   const StyleAspectRatio& ratio = Style().AspectRatio();
-  if (ratio.GetType() == EAspectRatioType::kRatio)
+  if (ratio.GetType() == EAspectRatioType::kRatio ||
+      (ratio.GetType() == EAspectRatioType::kAutoAndRatio && !IsReplaced()))
     return Style().LogicalAspectRatio();
 
   base::Optional<LayoutUnit> computed_inline_size;

third_party/blink/web_tests/external/wpt/css/css-contain/contain-size-replaced-007.html

@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Containment Test: Size containment replaced elements intrinsic size</title>
+<link rel="help" href="https://drafts.csswg.org/css-contain-1/#containment-size">
+<link rel="help" href="https://html.spec.whatwg.org/multipage/rendering.html#attributes-for-embedded-content-and-images">
+<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
+<meta name=assert content="This test checks that an aspect ratio computed from width and height attributes is used even with contain: size.">
+
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<img src="support/60x60-green.png" width="60" height="60" style="width: 100px; height: auto; contain: size;">

third_party/blink/web_tests/external/wpt/css/css-sizing/aspect-ratio/replaced-element-028.html

@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<title>CSS aspect-ratio: img</title>
+<link rel="author" title="Google LLC" href="https://www.google.com/">
+<link rel="help" href="https://drafts.csswg.org/css-sizing-4/#aspect-ratio">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<div id="log"></div>
+
+<img id="test" src="error.png" style="width: 100px; aspect-ratio: 1/5; background: yellow;" alt="Should not be 500px high">
+
+<script>
+function run_test() {
+  test(function() {
+    var img = document.getElementById("test");
+    assert_not_equals(img.offsetHeight, 500);
+  });
+}
+onload = run_test;
+</script>

third_party/blink/web_tests/external/wpt/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/img-aspect-ratio.html

@@ -14,11 +14,13 @@
 <img src="" width=100 height=125>
 <img src="error.png" width=100 height=125>
 <img src="error.png">
+<img src="error.png" alt="Alt text" width=100 height=500>
 <script>
 let t = async_test("Image width and height attributes are used to infer aspect-ratio");
-function assert_ratio(img, expected) {
+function assert_ratio(img, expected, description) {
   let epsilon = 0.001;
-  assert_approx_equals(parseFloat(getComputedStyle(img).width, 10) / parseFloat(getComputedStyle(img).height, 10), expected, epsilon);
+  assert_approx_equals(parseFloat(getComputedStyle(img).width, 10) / parseFloat(getComputedStyle(img).height, 10),
+                       expected, epsilon, description);
 }
 // Create and append a new image and immediately check the ratio.
 // This is not racy because the spec requires the user agent to queue a task:
@@ -49,10 +51,11 @@
 
 onload = t.step_func_done(function() {
   let images = document.querySelectorAll("img");
-  assert_ratio(images[0], 2.0); // Loaded image's aspect ratio, at least by default, overrides width / height ratio.
-  assert_ratio(images[1], 2.0); // 2.0 is the original aspect ratio of green.png
-  assert_equals(getComputedStyle(images[2]).height, "0px"); // aspect-ratio doesn't override intrinsic size of images that don't have any src.
-  assert_equals(getComputedStyle(images[3]).height, getComputedStyle(images[4]).height); // aspect-ratio doesn't override intrinsic size of error images.
-  assert_ratio(images[5], 133/106); // The original aspect ratio of blue.png
+  assert_ratio(images[0], 2.0, "2.0 is the original aspect ratio of green.png");
+  assert_ratio(images[1], 2.0, "Loaded image's aspect ratio, at least by default, overrides width / height ratio.");
+  assert_ratio(images[2], 100/125, "aspect-ratio should override intrinsic size of images that don't have any src.");
+  assert_ratio(images[3], 100/125, "aspect-ratio should affect the size of error images.");
+  assert_not_equals(images[5].offsetHeight, 500, "Images with alt text should be inline and ignore the aspect ratio");
+  assert_ratio(images[6], 133/106, "The original aspect ratio of blue.png");
 });
 </script>