Bug 1453843 - Ensure we fire "pause" event when rejecting play() promise. r=bryce
authorChris Pearce <cpearce@mozilla.com>
Fri, 20 Apr 2018 17:53:37 +1200
changeset 468534 37ac866c0cdc64f65cb5fb41967cecf826b84bcb
parent 468521 3709d682bb55a88747275fff818a033f8de558c1
child 468535 6714516bb697bab6b0cc4514a0b12a705b184b28
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbryce
bugs1453843, 1435133
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1453843 - Ensure we fire "pause" event when rejecting play() promise. r=bryce Bug 1435133 introduced a new path where we block autoplay and reject the play() promise, but we didn't fire a "pause" event. This confuses YouTube's controls. Additionally, even if we're not in a user generated event handler, we unilaterally consider the media element blessed if execution reaches here: https://searchfox.org/mozilla-central/rev/11a2ae294f50049e12515b5821f5a396d951aacb/dom/html/HTMLMediaElement.cpp#4110 We previously rejected before reaching here when not in a user generated event handler, but now if play() is called before we've reached loadedmetadata, we reject the promise if we're not in a non-event handler and bail out early, and so we'll bless even if not in a user generated event handler. Meaning when we do reach loadedmetadata, we think we were in a user generated event handler when play() was originally called, and so we won't reject the play promise. So this patch ensures we dispatch a "pause" event when we reject the play() promise here. The WHATWG spec says we should do this when pausing anyway. Note: calling our interal Pause() function when rejecting the play() promise here breaks YouTube, as if we do that we fire a "timeupdate" event. So I opted to manually code to fire the event here instead of just calling Pause() everywhere we want to ensure we're paused. MozReview-Commit-ID: 1snkiTnPGih
dom/html/HTMLMediaElement.cpp
dom/media/test/file_autoplay_policy_play_before_loadedmetadata.html
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -4101,18 +4101,22 @@ HTMLMediaElement::PlayInternal(ErrorResu
   mAutoplaying = false;
 
   // We changed mPaused and mAutoplaying which can affect AddRemoveSelfReference
   // and our preload status.
   AddRemoveSelfReference();
   UpdatePreloadAction();
   UpdateSrcMediaStreamPlaying();
 
-  // once media start playing, we would always allow it to autoplay
-  mIsBlessed = true;
+  // Once play() has been called in a user generated event handler,
+  // it is allowed to autoplay. Note: we can reach here when not in
+  // a user generated event handler if our readyState has not yet
+  // reached HAVE_METADATA.
+  mIsBlessed |= EventStateManager::IsHandlingUserInput();
+
 
   // TODO: If the playback has ended, then the user agent must set
   // seek to the effective start.
 
   // 4.8.12.8 - Step 6:
   // If the media element's paused attribute is true, run the following steps:
   if (oldPaused) {
     // 6.1. Change the value of paused to false. (Already done.)
@@ -5960,18 +5964,19 @@ HTMLMediaElement::ChangeReadyState(nsMed
       mReadyState >= HAVE_FUTURE_DATA) {
     DispatchAsyncEvent(NS_LITERAL_STRING("canplay"));
     if (!mPaused) {
       if (mAttemptPlayUponLoadedMetadata && mDecoder) {
         mAttemptPlayUponLoadedMetadata = false;
         if (IsAllowedToPlay()) {
           mDecoder->Play();
         } else {
+          mPaused = true;
+          DispatchAsyncEvent(NS_LITERAL_STRING("pause"));
           AsyncRejectPendingPlayPromises(NS_ERROR_DOM_MEDIA_NOT_ALLOWED_ERR);
-          mPaused = true;
         }
       }
       NotifyAboutPlaying();
     }
   }
 
   CheckAutoplayDataReady();
 
--- a/dom/media/test/file_autoplay_policy_play_before_loadedmetadata.html
+++ b/dom/media/test/file_autoplay_policy_play_before_loadedmetadata.html
@@ -24,21 +24,26 @@
 
         async function testPlayBeforeLoadedMetata(testCase, parent_window) {
           info("testPlayBeforeLoadedMetata: " + testCase.resource);
 
           let element = document.createElement("video");
           element.preload = "auto";
           element.src = testCase.resource;
           document.body.appendChild(element);
-
+          is(element.paused, true, testCase.resource + " - should start out paused.");
+          let playEventFired = false;
+          once(element, "play").then(() => { playEventFired = true; });
+          let pauseEventFired = false;
+          once(element, "pause").then(() => { pauseEventFired = true; });
           let played = await element.play().then(() => true, () => false);
           let msg = testCase.resource + " should " + (!testCase.shouldPlay ? "not " : "") + "play";
           is(played, testCase.shouldPlay, msg);
-
+          is(playEventFired, true, testCase.resource + " - we should always get a play event");
+          is(pauseEventFired, !testCase.shouldPlay, testCase.resource + " - if we shouldn't play, we should get a pause event");
           removeNodeAndSource(element);
         }
 
         nextWindowMessage().then(
           async (event) => {
             await testPlayBeforeLoadedMetata(event.data, event.source);
             event.source.postMessage("done", "*");
           });