Auto merge of #29229 - servo:intermittent-video-poster, r=jdm

Fix intermittency when loading poster images

Wait until a poster image is cached to in order to unblock document load. If not, the document may continue loading before the image is ready to use, leading to intermittency in test output. Now load is unblocked when getting the ImageResponse from the cache, which allows the code to properly unblock the load when the entire load fails or succeeds.

This reveals several false passes in the `object-view-box` test suite which were very flaky.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #29204.
- [x] These changes fix #29188.
- [x] These changes fix #29179.

<!-- Either: -->
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

components/script/dom/htmlmediaelement.rs

@@ -76,7 +76,6 @@ use ipc_channel::ipc;
 use ipc_channel::router::ROUTER;
 use media::{glplayer_channel, GLPlayerMsg, GLPlayerMsgForward, WindowGLContext};
 use net_traits::image::base::Image;
-use net_traits::image_cache::ImageResponse;
 use net_traits::request::Destination;
 use net_traits::{CoreResourceMsg, FetchChannels, FetchMetadata, FetchResponseListener, Metadata};
 use net_traits::{NetworkError, ResourceFetchTiming, ResourceTimingType};
@@ -1306,25 +1305,22 @@ impl HTMLMediaElement {
     }
 
     /// https://html.spec.whatwg.org/multipage/#poster-frame
-    pub fn process_poster_response(&self, image: ImageResponse) {
+    pub fn process_poster_image_loaded(&self, image: Arc<Image>) {
         if !self.show_poster.get() {
             return;
         }
 
         // Step 6.
-        if let ImageResponse::Loaded(image, _) = image {
-            self.video_renderer
-                .lock()
-                .unwrap()
-                .render_poster_frame(image);
-            self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
-            if pref!(media.testing.enabled) {
-                let window = window_from_node(self);
-                let task_source = window.task_manager().media_element_task_source();
-                task_source.queue_simple_event(self.upcast(), atom!("postershown"), &window);
-            } else {
-                return;
-            }
+        self.video_renderer
+            .lock()
+            .unwrap()
+            .render_poster_frame(image);
+        self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
+
+        if pref!(media.testing.enabled) {
+            let window = window_from_node(self);
+            let task_source = window.task_manager().media_element_task_source();
+            task_source.queue_simple_event(self.upcast(), atom!("postershown"), &window);
         }
     }
 

components/script/dom/htmlvideoelement.rs

@@ -109,10 +109,6 @@ impl HTMLVideoElement {
         self.video_height.set(height);
     }
 
-    pub fn allow_load_event(&self) {
-        LoadBlocker::terminate(&mut *self.load_blocker.borrow_mut());
-    }
-
     pub fn get_current_frame_data(&self) -> Option<(Option<ipc::IpcSharedMemory>, Size2D<u32>)> {
         let frame = self.htmlmediaelement.get_current_frame();
         if frame.is_some() {
@@ -293,7 +289,19 @@ impl ImageCacheListener for HTMLVideoElement {
     }
 
     fn process_image_response(&self, response: ImageResponse) {
-        self.htmlmediaelement.process_poster_response(response);
+        match response {
+            ImageResponse::Loaded(image, url) => {
+                debug!("Loaded poster image for video element: {:?}", url);
+                self.htmlmediaelement.process_poster_image_loaded(image);
+                LoadBlocker::terminate(&mut *self.load_blocker.borrow_mut());
+            },
+            ImageResponse::MetadataLoaded(..) => {},
+            ImageResponse::PlaceholderLoaded(..) => unreachable!(),
+            ImageResponse::None => {
+                // A failed load should unblock the document load.
+                LoadBlocker::terminate(&mut *self.load_blocker.borrow_mut());
+            },
+        }
     }
 }
 
@@ -351,7 +359,6 @@ impl FetchResponseListener for PosterFrameFetchContext {
     }
 
     fn process_response_eof(&mut self, response: Result<ResourceFetchTiming, NetworkError>) {
-        self.elem.root().allow_load_event();
         self.image_cache
             .notify_pending_response(self.id, FetchResponseMsg::ProcessResponseEOF(response));
     }