Use Aspect Ratio Computed From Presentational Hints Before Video Loads.

https://bugs.webkit.org/show_bug.cgi?id=245001
rdar://99755477

Reviewed by Brent Fulgham.

When the width and height attributes are provided to video elements,
those values should contribute to the aspect-ratio property with the
form: auto width x height. This is the aspect-ratio that should be used
when sizing the video before its metadata have been received. Once the
metadata has been received, we should instead use the aspect-ratio
provided by the intrinsic sizes. Another subtle trait about the video
element is that its default object size (300px x 150px) should not
contribute to the aspect-ratio at any point in time.

To help match this behavior, the RenderBox::boxAspectRatio virtual
method was created to help determine the appropriate aspect ratio for
different boxes. The default behavior is to just return
style().logicalAspectRatio(), so that the behavior can remain the same
in RenderBox::computeMinMaxLogicalHeightFromAspectRatio and
RenderBox::computeMinMaxLogicalWidthFromAspectRatio. In scenarios where
this may not be correct, such as with RenderVideo, the subclass can
override the behavior. For RenderVideo's case, we need to determine
whether or not we have received the video metadata. If we have, we
should return the aspect-ratio provided by the intrinsic size of the
video. Otherwise, we can use the aspect ratio provided by either the
CSS property or the presentational hints.

* LayoutTests/imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/video-aspect-ratio-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/video-aspect-ratio.html:
Imported most recent WPT test

* LayoutTests/media/video-intrinsic-width-height.html:
The final video element in this test should have a final dimension of
300px x 300px. This is because:

    1.) Since there is not actually any video content, this box will not
        have any intrinsic width/height.
    2.) There is an inline style that overrides the width/height
        attributes, so it will have width and height of auto.
    3.) Even though the width/height style is overridden, the attributes
        still contribute to the aspect ratio, giving it an aspect-ratio
        value of "auto 100/100"
    4.) When computing the automatic width we fall back to the default
        object size value, which is 300px.
    5.) To compute the height we take the aspect-ratio we computed and
        arrive at a height of 300px.

* Source/WebCore/rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::computeAspectRatioInformationForRenderBox const):
The new code path provides the width and height based on the aspect
ratio for video elements. Even though it seems like this logic should
work for any ReplacedElement, it seems to fail various tests. For now
we may want to just keep this logic separated for RenderVideo objects
specifically.

(WebCore::RenderReplaced::computeIntrinsicRatioInformation const):
We need to make sure we do not override the computed aspect ratio with
the values of the default object size for video elements. This is
because the default object size is not an intrinsic size and should not
contribute to the aspect ratio. Basically, we need to check out if we
have an intrinsic size set and bail out early if we do.
RenderVideo::calculateIntrinsicSize contains logic that decides if the
size needs to be set to the default size, so we can base most off of the
logic off of that.

(WebCore::isVideoWithDefaultObjectSize):
* Source/WebCore/rendering/RenderReplaced.h:
* Source/WebCore/rendering/RenderVideo.cpp:
(WebCore::RenderVideo::calculateIntrinsicSize):
(WebCore::RenderVideo::computeReplacedLogicalWidth const):
(WebCore::RenderVideo::hasVideoMetadata const):
(WebCore::RenderVideo::hasPosterFrameSize const):
(WebCore::RenderVideo::hasDefaultObjectSize const):
* Source/WebCore/rendering/RenderVideo.h:

Canonical link: https://commits.webkit.org/255743@main

LayoutTests/imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/video-aspect-ratio-expected.txt

@@ -1,6 +1,6 @@
 
 
-PASS Video width and height attributes are not used to infer aspect-ratio
+PASS Video width and height attributes are used to infer aspect-ratio
 PASS Computed style test: video with {"width":"10","height":"20"}
 PASS Computed style test: video with {"width":"0.5","height":"1.5"}
 PASS Computed style test: video with {"width":"0","height":"1"}

LayoutTests/imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/video-aspect-ratio.html

@@ -1,4 +1,4 @@
-<!doctype html><!-- webkit-test-runner [ AspectRatioOfImgFromWidthAndHeightEnabled=true ] -->
+<!doctype html>
 <title>Video width and height attributes are used to infer aspect-ratio</title>
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
@@ -14,7 +14,6 @@
 <body>
 <video width="250" height="100" id="contained" style="contain: size;"></video>
 <script>
-let t = async_test("Video width and height attributes are not used to infer aspect-ratio");
 function assert_ratio(img, expected) {
   let epsilon = 0.001;
   assert_approx_equals(parseInt(getComputedStyle(img).width, 10) / parseInt(getComputedStyle(img).height, 10), expected, epsilon);
@@ -24,41 +23,50 @@
   test_computed_style_aspect_ratio("video", {width: width, height: height}, expected);
 }
 
-t.step(function() {
-  var video = document.getElementById("contained");
-  video.src = getVideoURI('/media/2x2-green');
-  assert_ratio(video, 2.5);
-}, "contain:size aspect ratio");
-
-// Create and append a new video and immediately check the ratio.
-// This is not racy because the spec requires the user agent to queue a task:
-// https://html.spec.whatwg.org/multipage/media.html#concept-media-load-algorithm
-t.step(function() {
-  video = document.createElement("video");
-  video.setAttribute("width", "250");
-  video.setAttribute("height", "100");
-  video.src = getVideoURI('/media/2x2-green');
-  document.body.appendChild(video);
-  // Videos default to a size of 300x150px and calculate their aspect ratio
-  // based on that before the video is loaded. So this should be 2, ignoring
-  // the 2.5 that it would be based on the attributes.
-  assert_ratio(video, 2);
+promise_test(async function() {
+  {
+    let video = document.getElementById("contained");
+    video.src = getVideoURI('/media/2x2-green');
+    assert_ratio(video, 2.5, "contain:size aspect ratio");
+  }
 
-  video.onloadeddata = t.step_func_done(function() {
+  // Create and append a new video and immediately check the ratio.
+  // This is not racy because the spec requires the user agent to queue a task:
+  // https://html.spec.whatwg.org/multipage/media.html#concept-media-load-algorithm
+  {
+    let video = document.createElement("video");
+    video.setAttribute("width", "250");
+    video.setAttribute("height", "100");
+    video.src = getVideoURI('/media/2x2-green');
+    document.body.appendChild(video);
+    assert_ratio(video, 2.5, "aspect ratio for regular video before load");
+    await new Promise(r => video.addEventListener("loadeddata", r, { once: true }));
     // When loaded this video is square.
-    assert_ratio(video, 1);
-  });
-}, "aspect ratio for regular video");
+    assert_ratio(video, 1, "aspect ratio for regular video after load");
+  }
 
-test_computed_style("10", "20", "auto 10 / 20");
-test_computed_style("0.5", "1.5", "auto 0.5 / 1.5");
-test_computed_style("0", "1", "auto 0 / 1");
-test_computed_style("1", "0", "auto 1 / 0");
-test_computed_style("0", "0", "auto 0 / 0");
-test_computed_style(null, null, "auto");
-test_computed_style("10", null, "auto");
-test_computed_style(null, "20", "auto");
-test_computed_style("xx", "20", "auto");
-test_computed_style("10%", "20", "auto");
+  // Same but with auto width.
+  {
+    let video = document.createElement("video");
+    video.setAttribute("width", "250");
+    video.setAttribute("height", "100");
+    video.style.width = "auto";
+    video.src = getVideoURI('/media/2x2-green');
+    document.body.appendChild(video);
+    assert_ratio(video, 2.5, "aspect ratio for regular video with width: auto before load");
+    await new Promise(r => video.addEventListener("loadeddata", r, { once: true }));
+    assert_ratio(video, 1, "aspect ratio for regular video with width: auto after load");
+  }
 
-</script>
+  test_computed_style("10", "20", "auto 10 / 20");
+  test_computed_style("0.5", "1.5", "auto 0.5 / 1.5");
+  test_computed_style("0", "1", "auto 0 / 1");
+  test_computed_style("1", "0", "auto 1 / 0");
+  test_computed_style("0", "0", "auto 0 / 0");
+  test_computed_style(null, null, "auto");
+  test_computed_style("10", null, "auto");
+  test_computed_style(null, "20", "auto");
+  test_computed_style("xx", "20", "auto");
+  test_computed_style("10%", "20", "auto");
+});
+</script>

LayoutTests/media/video-intrinsic-width-height.html → LayoutTests/imported/w3c/web-platform-tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/video-intrinsic-width-height.html

@@ -2,8 +2,12 @@
 <html>
     <head>
         <title>video element intrinsic width/height</title>
-        <script src="../resources/testharness.js"></script>
-        <script src="../resources/testharnessreport.js"></script>
+        <link rel="author" title="Sammy Gill" href="sammy.gill@apple.com" />
+        <link rel="help" href="https://github.com/w3c/csswg-drafts/issues/7524" />
+        <link rel="help" href="https://html.spec.whatwg.org/multipage/rendering.html#replaced-elements" />
+        <link rel="help" href="https://w3c.github.io/csswg-drafts/css-sizing-4/#aspect-ratio" />
+        <script src="/resources/testharness.js"></script>
+        <script src="/resources/testharnessreport.js"></script>
     </head>
     <body>
         <div id="log"></div>
@@ -21,7 +25,7 @@
         <!-- A width:height ratio other than 2:1 and overriding the specified style must be used to
              verify that width/height does not influence intrinsic ratio -->
         <video title="both width/height attributes and style"
-               data-expected-width="300" data-expected-height="150"
+               data-expected-width="300" data-expected-height="300"
                width="100" height="100" style="width: auto; height: auto"></video>
         <script>
             Array.prototype.forEach.call(document.querySelectorAll('video'), function(video)

Source/WebCore/rendering/RenderReplaced.cpp

@@ -45,13 +45,14 @@
 #include "RenderImage.h"
 #include "RenderLayer.h"
 #include "RenderTheme.h"
+#include "RenderVideo.h"
 #include "RenderView.h"
 #include "RenderedDocumentMarker.h"
 #include "Settings.h"
 #include "VisiblePosition.h"
 #include <wtf/IsoMallocInlines.h>
 #include <wtf/StackStats.h>
-
+#include <wtf/TypeCasts.h>
 namespace WebCore {
 
 WTF_MAKE_ISO_ALLOCATED_IMPL(RenderReplaced);
@@ -382,7 +383,14 @@ bool RenderReplaced::setNeedsLayoutIfNeededAfterIntrinsicSizeChange()
 
     return false;
 }
-
+
+static bool isVideoWithDefaultObjectSize(const RenderReplaced* maybeVideo)
+{
+    if (auto* video = dynamicDowncast<RenderVideo>(maybeVideo))
+        return video->hasDefaultObjectSize();
+    return false;
+} 
+
 void RenderReplaced::computeAspectRatioInformationForRenderBox(RenderBox* contentRenderer, FloatSize& constrainedSize, double& intrinsicRatio) const
 {
     FloatSize intrinsicSize;
@@ -425,10 +433,17 @@ void RenderReplaced::computeAspectRatioInformationForRenderBox(RenderBox* conten
     // function was added, since all it has done is make the code more unclear.
     constrainedSize = intrinsicSize;
     if (intrinsicRatio && !intrinsicSize.isEmpty() && style().logicalWidth().isAuto() && style().logicalHeight().isAuto()) {
-        // We can't multiply or divide by 'intrinsicRatio' here, it breaks tests, like fast/images/zoomed-img-size.html, which
-        // can only be fixed once subpixel precision is available for things like intrinsicWidth/Height - which include zoom!
-        constrainedSize.setWidth(RenderBox::computeReplacedLogicalHeight() * intrinsicSize.width() / intrinsicSize.height());
-        constrainedSize.setHeight(RenderBox::computeReplacedLogicalWidth() * intrinsicSize.height() / intrinsicSize.width());
+        if (isVideoWithDefaultObjectSize(this)) {
+            LayoutUnit width = RenderBox::computeReplacedLogicalWidth();
+            constrainedSize.setWidth(width);
+            LayoutUnit height = blockSizeFromAspectRatio(horizontalBorderAndPaddingExtent(), verticalBorderAndPaddingExtent(), intrinsicRatio, BoxSizing::ContentBox, adjustBorderBoxLogicalWidthForBoxSizing(width, LengthType::Fixed));
+            constrainedSize.setHeight(height.round() - verticalBorderAndPaddingExtent());
+        } else {
+            // We can't multiply or divide by 'intrinsicRatio' here, it breaks tests, like fast/images/zoomed-img-size.html, which
+            // can only be fixed once subpixel precision is available for things like intrinsicWidth/Height - which include zoom!
+            constrainedSize.setWidth(RenderBox::computeReplacedLogicalHeight() * intrinsicSize.width() / intrinsicSize.height());
+            constrainedSize.setHeight(RenderBox::computeReplacedLogicalWidth() * intrinsicSize.height() / intrinsicSize.width());
+        }
     }
 }
 
@@ -489,7 +504,7 @@ void RenderReplaced::computeIntrinsicRatioInformation(FloatSize& intrinsicSize,
 
     if (style().hasAspectRatio()) {
         intrinsicRatio = style().logicalAspectRatio();
-        if (style().aspectRatioType() == AspectRatioType::Ratio)
+        if (style().aspectRatioType() == AspectRatioType::Ratio || isVideoWithDefaultObjectSize(this))
             return;
     }
     // Figure out if we need to compute an intrinsic ratio.

Source/WebCore/rendering/RenderReplaced.h

@@ -52,14 +52,15 @@ class RenderReplaced : public RenderBox {
 
     double computeIntrinsicAspectRatio() const;
 
+    void computeIntrinsicRatioInformation(FloatSize& intrinsicSize, double& intrinsicRatio) const override;
+
 protected:
     RenderReplaced(Element&, RenderStyle&&);
     RenderReplaced(Element&, RenderStyle&&, const LayoutSize& intrinsicSize);
     RenderReplaced(Document&, RenderStyle&&, const LayoutSize& intrinsicSize);
 
     void layout() override;
 
-    void computeIntrinsicRatioInformation(FloatSize& intrinsicSize, double& intrinsicRatio) const override;
 
     void computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, LayoutUnit& maxLogicalWidth) const final;
 

Source/WebCore/rendering/RenderVideo.cpp

@@ -35,6 +35,7 @@
 #include "HTMLNames.h"
 #include "HTMLVideoElement.h"
 #include "MediaPlayer.h"
+#include "MediaPlayerEnums.h"
 #include "Page.h"
 #include "PaintInfo.h"
 #include "RenderView.h"
@@ -132,7 +133,7 @@ LayoutSize RenderVideo::calculateIntrinsicSize()
             return size;
     }
 
-    if (videoElement().shouldDisplayPosterImage() && !m_cachedImageSize.isEmpty() && !imageResource().errorOccurred())
+    if (hasPosterFrameSize())
         return m_cachedImageSize;
 
     // <video> in standalone media documents should not use the default 300x150
@@ -285,7 +286,7 @@ void RenderVideo::updatePlayer()
 
 LayoutUnit RenderVideo::computeReplacedLogicalWidth(ShouldComputePreferred shouldComputePreferred) const
 {
-    return RenderReplaced::computeReplacedLogicalWidth(shouldComputePreferred);
+    return computeReplacedLogicalWidthRespectingMinMaxWidth(RenderReplaced::computeReplacedLogicalWidth(shouldComputePreferred), shouldComputePreferred);
 }
 
 LayoutUnit RenderVideo::minimumReplacedHeight() const 
@@ -324,6 +325,21 @@ bool RenderVideo::foregroundIsKnownToBeOpaqueInRect(const LayoutRect& localRect,
     return false;
 }
 
+bool RenderVideo::hasVideoMetadata() const
+{
+    return videoElement().player() && videoElement().player()->readyState() >= MediaPlayerEnums::ReadyState::HaveMetadata;
+}
+
+bool RenderVideo::hasPosterFrameSize() const
+{
+    return videoElement().shouldDisplayPosterImage() && !m_cachedImageSize.isEmpty() && !imageResource().errorOccurred();
+}
+
+bool RenderVideo::hasDefaultObjectSize() const
+{
+    return !hasVideoMetadata() && !hasPosterFrameSize() && !shouldApplySizeContainment();
+}
+
 } // namespace WebCore
 
 #endif

Source/WebCore/rendering/RenderVideo.h

@@ -53,6 +53,9 @@ class RenderVideo final : public RenderMedia {
     bool failedToLoadPosterImage() const;
 
     void updateFromElement() final;
+    bool hasVideoMetadata() const;
+    bool hasPosterFrameSize() const;
+    bool hasDefaultObjectSize() const;
 
 private:
     void willBeDestroyed() override;