From 6defd7011be4871e9a72740e1f6abdf36383bf1f Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Wed, 11 Jan 2023 10:10:17 +0100 Subject: [PATCH] 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. Fixes #29204. Fixes #29188. Fixes #29179. Signed-off-by: Martin Robinson --- components/net/image_cache.rs | 2 ++ components/script/dom/htmlmediaelement.rs | 26 ++++++++----------- components/script/dom/htmlvideoelement.rs | 19 +++++++++----- ...object-view-box-fit-contain-video.html.ini | 2 ++ .../object-view-box-fit-cover-video.html.ini | 2 ++ .../object-view-box-fit-none-video.html.ini | 2 ++ ...bject-view-box-writing-mode-video.html.ini | 2 ++ 7 files changed, 34 insertions(+), 21 deletions(-) create mode 100644 tests/wpt/metadata/css/css-images/object-view-box-fit-contain-video.html.ini create mode 100644 tests/wpt/metadata/css/css-images/object-view-box-fit-cover-video.html.ini create mode 100644 tests/wpt/metadata/css/css-images/object-view-box-fit-none-video.html.ini create mode 100644 tests/wpt/metadata/css/css-images/object-view-box-writing-mode-video.html.ini diff --git a/components/net/image_cache.rs b/components/net/image_cache.rs index bdc3ef9e83d..4b88fd5755e 100644 --- a/components/net/image_cache.rs +++ b/components/net/image_cache.rs @@ -253,6 +253,7 @@ impl LoadKeyGenerator { } } +#[derive(Debug)] enum LoadResult { Loaded(Image), PlaceholderLoaded(Arc), @@ -339,6 +340,7 @@ struct ImageCacheStore { impl ImageCacheStore { // Change state of a url from pending -> loaded. fn complete_load(&mut self, key: LoadKey, mut load_result: LoadResult) { + debug!("Completed decoding for {:?}", load_result); let pending_load = match self.pending_loads.remove(&key) { Some(load) => load, None => return, diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs index fcb38aa1427..f5043cac0ea 100644 --- a/components/script/dom/htmlmediaelement.rs +++ b/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) { 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::().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::().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); } } diff --git a/components/script/dom/htmlvideoelement.rs b/components/script/dom/htmlvideoelement.rs index 2100b70330d..eb1b35ee07a 100644 --- a/components/script/dom/htmlvideoelement.rs +++ b/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, Size2D)> { 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) { - self.elem.root().allow_load_event(); self.image_cache .notify_pending_response(self.id, FetchResponseMsg::ProcessResponseEOF(response)); } diff --git a/tests/wpt/metadata/css/css-images/object-view-box-fit-contain-video.html.ini b/tests/wpt/metadata/css/css-images/object-view-box-fit-contain-video.html.ini new file mode 100644 index 00000000000..7431e6e5ae9 --- /dev/null +++ b/tests/wpt/metadata/css/css-images/object-view-box-fit-contain-video.html.ini @@ -0,0 +1,2 @@ +[object-view-box-fit-contain-video.html] + expected: FAIL diff --git a/tests/wpt/metadata/css/css-images/object-view-box-fit-cover-video.html.ini b/tests/wpt/metadata/css/css-images/object-view-box-fit-cover-video.html.ini new file mode 100644 index 00000000000..bc8e4ad026d --- /dev/null +++ b/tests/wpt/metadata/css/css-images/object-view-box-fit-cover-video.html.ini @@ -0,0 +1,2 @@ +[object-view-box-fit-cover-video.html] + expected: FAIL diff --git a/tests/wpt/metadata/css/css-images/object-view-box-fit-none-video.html.ini b/tests/wpt/metadata/css/css-images/object-view-box-fit-none-video.html.ini new file mode 100644 index 00000000000..8dedb9addee --- /dev/null +++ b/tests/wpt/metadata/css/css-images/object-view-box-fit-none-video.html.ini @@ -0,0 +1,2 @@ +[object-view-box-fit-none-video.html] + expected: FAIL diff --git a/tests/wpt/metadata/css/css-images/object-view-box-writing-mode-video.html.ini b/tests/wpt/metadata/css/css-images/object-view-box-writing-mode-video.html.ini new file mode 100644 index 00000000000..dc30f75ad6f --- /dev/null +++ b/tests/wpt/metadata/css/css-images/object-view-box-writing-mode-video.html.ini @@ -0,0 +1,2 @@ +[object-view-box-writing-mode-video.html] + expected: FAIL