Bug 1461877 - Ensure we don't dispatch 'playing' when we're about to reject pending play promises. r=bryce
authorChris Pearce <cpearce@mozilla.com>
Wed, 16 May 2018 17:27:01 +1200
changeset 419060 25513d2e78e9dbda43c84cbd1f1c1adbf17fc89a
parent 419059 e1eb21be77ba723bba7d5ccd461eb65d074cfc35
child 419061 b7ebe102efa20de8b384aa603c8df1a9cdb61429
push id34025
push userapavel@mozilla.com
push dateMon, 21 May 2018 09:46:09 +0000
treeherdermozilla-central@8850728602d6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbryce
bugs1461877
milestone62.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 1461877 - Ensure we don't dispatch 'playing' when we're about to reject pending play promises. r=bryce Currently we can end up dispatching a 'playing' event right before we reject play() promises, and this confuses YouTube's controls, and it doesn't make sense to dispatch a 'playing' event when we're not playing anyway. This is because the logic to delay resolving the play() promise until after we've reached loadedmetadata doesn't prevent the 'playing' event from being dispatched. We shouldn't dispatch 'playing' until we resolve the play() promise(s). MozReview-Commit-ID: 5H4dcObfu4M
dom/html/HTMLMediaElement.cpp
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -1817,16 +1817,17 @@ void HTMLMediaElement::AbortExistingLoad
   mTags = nullptr;
 
   if (mNetworkState != NETWORK_EMPTY) {
     NS_ASSERTION(!mDecoder && !mSrcStream, "How did someone setup a new stream/decoder already?");
     // ChangeNetworkState() will call UpdateAudioChannelPlayingState()
     // indirectly which depends on mPaused. So we need to update mPaused first.
     if (!mPaused) {
       mPaused = true;
+      DispatchAsyncEvent(NS_LITERAL_STRING("pause"));
       RejectPromises(TakePendingPlayPromises(), NS_ERROR_DOM_MEDIA_ABORT_ERR);
     }
     ChangeNetworkState(NETWORK_EMPTY);
     ChangeReadyState(HAVE_NOTHING);
 
     //TODO: Apply the rules for text track cue rendering Bug 865407
     if (mTextTrackManager) {
       mTextTrackManager->GetTextTracks()->SetCuesInactive();
@@ -4073,17 +4074,16 @@ HTMLMediaElement::PlayInternal(ErrorResu
   UpdateSrcMediaStreamPlaying();
 
   // 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.)
     // This step is uplifted because the "block-media-playback" feature needs
@@ -4109,20 +4109,22 @@ HTMLMediaElement::PlayInternal(ErrorResu
     case HAVE_METADATA:
     case HAVE_CURRENT_DATA:
       FireTimeUpdate(false);
       DispatchAsyncEvent(NS_LITERAL_STRING("waiting"));
       break;
     case HAVE_FUTURE_DATA:
     case HAVE_ENOUGH_DATA:
       FireTimeUpdate(false);
-      NotifyAboutPlaying();
+      if (!mAttemptPlayUponLoadedMetadata) {
+        NotifyAboutPlaying();
+      }
       break;
     }
-  } else if (mReadyState >= HAVE_FUTURE_DATA) {
+  } else if (mReadyState >= HAVE_FUTURE_DATA && !mAttemptPlayUponLoadedMetadata) {
     // 7. Otherwise, if the media element's readyState attribute has the value
     //    HAVE_FUTURE_DATA or HAVE_ENOUGH_DATA, take pending play promises and
     //    queue a task to resolve pending play promises with the result.
     AsyncResolvePendingPlayPromises();
   }
 
   // 8. Set the media element's autoplaying flag to false. (Already done.)
 
@@ -5887,16 +5889,27 @@ HTMLMediaElement::ChangeReadyState(nsMed
   DDLOG(DDLogCategory::Property, "ready_state", gReadyStateToString[aState]);
 
   if (mNetworkState == NETWORK_EMPTY) {
     return;
   }
 
   UpdateAudioChannelPlayingState();
 
+  if (oldState < HAVE_METADATA &&
+      mReadyState >= HAVE_METADATA &&
+      mAttemptPlayUponLoadedMetadata) {
+    mAttemptPlayUponLoadedMetadata = false;
+    if (!mPaused && !IsAllowedToPlay()) {
+      mPaused = true;
+      DispatchAsyncEvent(NS_LITERAL_STRING("pause"));
+      AsyncRejectPendingPlayPromises(NS_ERROR_DOM_MEDIA_NOT_ALLOWED_ERR);
+    }
+  }
+
   // Handle raising of "waiting" event during seek (see 4.8.10.9)
   // or
   // 4.8.12.7 Ready states:
   // "If the previous ready state was HAVE_FUTURE_DATA or more, and the new
   // ready state is HAVE_CURRENT_DATA or less
   // If the media element was potentially playing before its readyState
   // attribute changed to a value lower than HAVE_FUTURE_DATA, and the element
   // has not ended playback, and playback has not stopped due to errors,
@@ -5919,25 +5932,18 @@ HTMLMediaElement::ChangeReadyState(nsMed
     DispatchAsyncEvent(NS_LITERAL_STRING("loadeddata"));
     mLoadedDataFired = true;
   }
 
   if (oldState < HAVE_FUTURE_DATA &&
       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);
-        }
+      if (mDecoder) {
+        mDecoder->Play();
       }
       NotifyAboutPlaying();
     }
   }
 
   CheckAutoplayDataReady();
 
   if (oldState < HAVE_ENOUGH_DATA &&