Prevent resizing of video poster image while waiting for initial frame

The previous logic for what intrinsic size to give to a video element
was based on outdated spec text, this CL updates it to prevent the
poster image from resizing between starting playback and the first video
frame being rendered.

Bug: 1154296
Change-Id: I41a91f6672d8ccf4e3069afe581fac39f1a58820
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2617870
Commit-Queue: Will Cassella <cassew@google.com>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Aleks Totic <atotic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845477}

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

@@ -88,32 +88,30 @@ LayoutSize LayoutVideo::CalculateIntrinsicSize(float scale) {
     }
   }
 
-  // Spec text from 4.8.6
-  //
-  // The intrinsic width of a video element's playback area is the intrinsic
-  // width of the video resource, if that is available; otherwise it is the
-  // intrinsic width of the poster frame, if that is available; otherwise it is
-  // 300 CSS pixels.
-  //
-  // The intrinsic height of a video element's playback area is the intrinsic
-  // height of the video resource, if that is available; otherwise it is the
-  // intrinsic height of the poster frame, if that is available; otherwise it is
-  // 150 CSS pixels.
-  WebMediaPlayer* web_media_player = MediaElement()->GetWebMediaPlayer();
-  if (web_media_player &&
-      video->getReadyState() >= HTMLVideoElement::kHaveMetadata) {
-    IntSize size(web_media_player->NaturalSize());
-    if (!size.IsEmpty()) {
-      LayoutSize layoutSize = LayoutSize(size);
-      layoutSize.Scale(scale);
-      return layoutSize;
-    }
+  switch (GetDisplayMode()) {
+    // This implements the intrinsic width/height calculation from:
+    // https://html.spec.whatwg.org/#the-video-element:dimension-attributes:~:text=The%20intrinsic%20width%20of%20a%20video%20element's%20playback%20area
+    // If the video playback area is currently represented by the poster image,
+    // the intrinsic width and height are that of the poster image.
+    case kPoster:
+      if (!cached_image_size_.IsEmpty() && !ImageResource()->ErrorOccurred()) {
+        return cached_image_size_;
+      }
+      break;
+
+    // Otherwise, the intrinsic width is that of the video.
+    case kVideo:
+      if (const auto* player = MediaElement()->GetWebMediaPlayer()) {
+        IntSize size(player->NaturalSize());
+        if (!size.IsEmpty()) {
+          LayoutSize layout_size = LayoutSize(size);
+          layout_size.Scale(scale);
+          return layout_size;
+        }
+      }
+      break;
   }
 
-  if (video->IsShowPosterFlagSet() && !cached_image_size_.IsEmpty() &&
-      !ImageResource()->ErrorOccurred())
-    return cached_image_size_;
-
   LayoutSize size = DefaultSize();
   size.Scale(scale);
   return size;
@@ -139,12 +137,18 @@ void LayoutVideo::ImageChanged(WrappedImagePtr new_image,
 
 LayoutVideo::DisplayMode LayoutVideo::GetDisplayMode() const {
   NOT_DESTROYED();
-  if (!VideoElement()->IsShowPosterFlagSet() ||
-      VideoElement()->PosterImageURL().IsEmpty()) {
-    return kVideo;
-  } else {
+
+  const auto* video = VideoElement();
+  // If the show-poster-flag is set (or there is no video frame to display) AND
+  // there is a poster image, display that.
+  if ((video->IsShowPosterFlagSet() || !video->HasAvailableVideoFrame()) &&
+      !video->PosterImageURL().IsEmpty()) {
     return kPoster;
   }
+  // Otherwise, try displaying a video frame.
+  else {
+    return kVideo;
+  }
 }
 
 void LayoutVideo::PaintReplaced(const PaintInfo& paint_info,

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

@@ -4,6 +4,7 @@
 
 #include "third_party/blink/renderer/core/layout/layout_video.h"
 
+#include "third_party/blink/renderer/core/html/media/html_video_element.h"
 #include "third_party/blink/renderer/core/layout/layout_image.h"
 #include "third_party/blink/renderer/core/loader/resource/image_resource_content.h"
 #include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
@@ -26,7 +27,9 @@ class LayoutVideoTest : public RenderingTest {
         UnacceleratedStaticBitmapImage::Create(image).get());
 
     // Set image to video
-    auto* layout_image = (LayoutImage*)GetLayoutObjectByElementId(id);
+    auto* video = To<HTMLVideoElement>(GetElementById(id));
+    auto* layout_image = To<LayoutImage>(video->GetLayoutObject());
+    video->setAttribute(html_names::kPosterAttr, "http://example.com/foo.jpg");
     layout_image->ImageResource()->SetImageResource(image_content);
   }
 };
@@ -42,7 +45,7 @@ TEST_F(LayoutVideoTest, PosterSizeWithNormal) {
   CreateAndSetImage("video", 10, 10);
   UpdateAllLifecyclePhasesForTest();
 
-  int width = ((LayoutBox*)GetLayoutObjectByElementId("video"))
+  int width = To<LayoutBox>(GetLayoutObjectByElementId("video"))
                   ->AbsoluteBoundingBoxRect()
                   .Width();
   EXPECT_EQ(width, 10);
@@ -59,10 +62,32 @@ TEST_F(LayoutVideoTest, PosterSizeWithZoom) {
   CreateAndSetImage("video", 10, 10);
   UpdateAllLifecyclePhasesForTest();
 
-  int width = ((LayoutBox*)GetLayoutObjectByElementId("video"))
+  int width = To<LayoutBox>(GetLayoutObjectByElementId("video"))
                   ->AbsoluteBoundingBoxRect()
                   .Width();
   EXPECT_EQ(width, 15);
 }
 
+TEST_F(LayoutVideoTest, PosterSizeAfterPlay) {
+  SetBodyInnerHTML(R"HTML(
+    <video id='video' src='http://example.com/foo.mp4' />
+  )HTML");
+
+  CreateAndSetImage("video", 10, 10);
+  UpdateAllLifecyclePhasesForTest();
+  auto* video = To<HTMLVideoElement>(GetElementById("video"));
+
+  // Try playing the video (should stall without a real source)
+  video->Play();
+  EXPECT_FALSE(video->IsShowPosterFlagSet());
+  EXPECT_FALSE(video->HasAvailableVideoFrame());
+
+  // Width should still be that of the poster image, NOT the default video
+  // element width
+  int width = To<LayoutBox>(GetLayoutObjectByElementId("video"))
+                  ->AbsoluteBoundingBoxRect()
+                  .Width();
+  EXPECT_EQ(width, 10);
+}
+
 }  // namespace blink

third_party/blink/web_tests/flag-specific/composite-after-paint/compositing/video/video-poster-expected.txt

@@ -19,14 +19,9 @@ Case: Displaying poster
   "layers": [
     {
       "name": "Scrolling background of LayoutView #document",
-      "bounds": [785, 605],
+      "bounds": [800, 600],
       "contentsOpaque": true,
       "backgroundColor": "#FFFFFF"
-    },
-    {
-      "name": "VerticalScrollbar",
-      "position": [785, 0],
-      "bounds": [15, 600]
     }
   ]
 }
@@ -37,7 +32,7 @@ Case: Displaying movie
   "layers": [
     {
       "name": "Scrolling background of LayoutView #document",
-      "bounds": [785, 893],
+      "bounds": [785, 813],
       "contentsOpaque": true,
       "backgroundColor": "#FFFFFF"
     },

third_party/blink/web_tests/flag-specific/disable-layout-ng/compositing/video/video-poster-expected.txt

@@ -19,15 +19,9 @@ Case: Displaying poster
   "layers": [
     {
       "name": "Scrolling Contents Layer",
-      "bounds": [785, 605],
+      "bounds": [800, 600],
       "contentsOpaque": true,
       "backgroundColor": "#FFFFFF"
-    },
-    {
-      "name": "ContentsLayer for Vertical Scrollbar Layer",
-      "position": [785, 0],
-      "bounds": [15, 600],
-      "contentsOpaque": true
     }
   ]
 }
@@ -38,7 +32,7 @@ Case: Displaying movie
   "layers": [
     {
       "name": "Scrolling Contents Layer",
-      "bounds": [785, 909],
+      "bounds": [785, 813],
       "contentsOpaque": true,
       "backgroundColor": "#FFFFFF"
     },

third_party/blink/web_tests/media/video-poster-delayed.html

 <!DOCTYPE HTML5>
-<title>Delayed load of poster should not overwrite intrinsic size of video.</title>
+<title>Delayed load of poster should overwrite intrinsic size of video.</title>
 <script src="../resources/testharness.js"></script>
 <script src="../resources/testharnessreport.js"></script>
 <video></video>
@@ -8,20 +8,30 @@ async_test(function(t) {
     var video = document.querySelector("video");
 
     video.onloadeddata = t.step_func(function() {
-        testVideoSize();
+        testVideoSize({
+            clientWidth: 320,
+            clientHeight: 240,
+            videoWidth: 320,
+            videoHeight: 240,
+        });
         video.poster = "content/abe.png";
         var image = document.createElement("img");
         image.src = "content/abe.png";
-        image.onload = t.step_func_done(testVideoSize);
+        image.onload = t.step_func_done(() => testVideoSize({
+            clientWidth: 76,
+            clientHeight: 103,
+            videoWidth: 320,
+            videoHeight: 240,
+        }));
     });
 
-    function testVideoSize() {
-        assert_equals(video.clientWidth, 320);
-        assert_equals(video.clientHeight, 240);
-        assert_equals(video.videoWidth, 320);
-        assert_equals(video.videoHeight, 240);
+    function testVideoSize(size) {
+        assert_equals(video.clientWidth, size.clientWidth);
+        assert_equals(video.clientHeight, size.clientHeight);
+        assert_equals(video.videoWidth, size.videoWidth);
+        assert_equals(video.videoHeight, size.videoHeight);
     }
 
     video.src = "content/test.ogv";
 });
-</script>
\ No newline at end of file
+</script>

third_party/blink/web_tests/media/video-size.html

@@ -36,7 +36,18 @@ var movieInfo = [
     {
         src: "content/test.ogv",
         poster: "content/abe.png",
-        description: "'poster' and 'src', should be 'video' size",
+        play: false,
+        description: "'poster' and 'src', should be 'poster' size before play",
+        width: 76,
+        height: 103,
+        videoWidth: 320,
+        videoHeight: 240
+    },
+    {
+        src: "content/test.ogv",
+        poster: "content/abe.png",
+        play: true,
+        description: "'poster' and 'src', should be 'video' size after play",
         width: 320,
         height: 240,
         videoWidth: 320,
@@ -78,7 +89,12 @@ movieInfo.forEach(function(movie) {
             if (movie.poster)
                 video.poster = movie.poster;
 
-            video.onloadedmetadata = t.step_func_done(testMovieSize);
+            if (movie.play) {
+                video.onplaying = t.step_func_done(testMovieSize);
+                video.play();
+            } else {
+                video.onloadedmetadata = t.step_func_done(testMovieSize);
+            }
 
             if (!movie.src || movie.src.indexOf("bogus") >= 0) {
                 setTimeout(t.step_func_done(testMovieSize), 0);
@@ -93,4 +109,4 @@ movieInfo.forEach(function(movie) {
         }
     }, movie.description);
 });
-</script>
\ No newline at end of file
+</script>

third_party/blink/web_tests/platform/win/compositing/video/video-poster-expected.txt

@@ -19,15 +19,9 @@ Case: Displaying poster
   "layers": [
     {
       "name": "Scrolling Contents Layer",
-      "bounds": [785, 605],
+      "bounds": [800, 600],
       "contentsOpaque": true,
       "backgroundColor": "#FFFFFF"
-    },
-    {
-      "name": "ContentsLayer for Vertical Scrollbar Layer",
-      "position": [785, 0],
-      "bounds": [15, 600],
-      "contentsOpaque": true
     }
   ]
 }
@@ -38,7 +32,7 @@ Case: Displaying movie
   "layers": [
     {
       "name": "Scrolling Contents Layer",
-      "bounds": [785, 909],
+      "bounds": [785, 813],
       "contentsOpaque": true,
       "backgroundColor": "#FFFFFF"
     },