Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement HTMLMediaElement poster attribute #22399

Merged
merged 6 commits into from Jan 15, 2019

Make poster frame load blocker independent from media element's one

  • Loading branch information
ferjm committed Jan 14, 2019
commit fde7d4589f9fe79a2727109e865674dde5250a4f
@@ -2,6 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */

use crate::document_loader::{LoadBlocker, LoadType};
use crate::dom::attr::Attr;
use crate::dom::bindings::cell::DomRefCell;
use crate::dom::bindings::codegen::Bindings::HTMLVideoElementBinding;
@@ -51,6 +52,9 @@ pub struct HTMLVideoElement {
generation_id: Cell<u32>,
/// Poster frame fetch request canceller.
poster_frame_canceller: DomRefCell<FetchCanceller>,
/// Load event blocker. Will block the load event while the poster frame
/// is being fetched.
load_blocker: DomRefCell<Option<LoadBlocker>>,
}

impl HTMLVideoElement {
@@ -65,6 +69,7 @@ impl HTMLVideoElement {
video_height: Cell::new(DEFAULT_HEIGHT),
generation_id: Cell::new(0),
poster_frame_canceller: DomRefCell::new(Default::default()),
load_blocker: Default::default(),
}
}

@@ -99,6 +104,10 @@ impl HTMLVideoElement {
self.video_height.set(height);
}

pub fn allow_load_event(&self) {
LoadBlocker::terminate(&mut *self.load_blocker.borrow_mut());
}

/// https://html.spec.whatwg.org/multipage/#poster-frame
fn fetch_poster_frame(&self, poster_url: &str) {
// Step 1.
@@ -164,7 +173,16 @@ impl HTMLVideoElement {
};

// Step 5.
self.htmlmediaelement.delay_load_event(true);
// This delay must be independent from the ones created by HTMLMediaElement during
// its media load algorithm, otherwise a code like
// <video poster="poster.png"></video>
// (which triggers no media load algorithm unless a explicit call to .load() is done)
// will block the document's load event forever.
let mut blocker = self.load_blocker.borrow_mut();

This comment has been minimized.

@jdm

jdm Jan 14, 2019

Member

Does this still work correctly if we switch the poster attribute value while the poster is loading? I think we probably need to call LoadBlocker::terminate.

This comment has been minimized.

@ferjm

ferjm Jan 14, 2019

Author Member

Good point. Done.

*blocker = Some(LoadBlocker::new(
&document_from_node(self),
LoadType::Image(poster_url.clone()),
));

let window = window_from_node(self);
let context = Arc::new(Mutex::new(PosterFrameFetchContext::new(
@@ -308,6 +326,7 @@ 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));
}
"testharness"
],
"mozilla/video_poster_frame.html": [
"8b321ef5d82e0de6e16625ecd41190abc00bacb6",
"8e85bcd62303b70153f8d451a843cb2bdd96484d",
"reftest"
],
"mozilla/video_poster_frame_ref.html": [
@@ -6,15 +6,10 @@
<link rel="match" href="video_poster_frame_ref.html">
</head>
<body>
<video src="" poster="poster.png"></video>
<video poster="poster.png"></video>
<script>
let video = document.querySelector("video");
video.addEventListener("postershown", function() {
// Apart from removing the `reftest-wait` class we need to get
// the `load` event to signal that it's ok to take the reftest
// screenshot. Video loading delays the document `load` event,
// so we garantee that we get this event by setting an invalid
// src attribute to the video tag.
document.documentElement.classList.remove("reftest-wait");
});
</script>
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.