Bug 1435134 - Don't play until we've reached metadata. r=bryce
authorChris Pearce <cpearce@mozilla.com>
Tue, 13 Mar 2018 16:40:18 +1300
changeset 468624 72144da2637d2bd2f0695010628c6db651482e23
parent 468623 2a39d6a9a949dac9c9cba0a9dfc1bd93de88c79e
child 468625 41039837009cec7d13229d5e3a3d0b1a2dc77f8a
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbryce
bugs1435134
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 1435134 - Don't play until we've reached metadata. r=bryce We want to block playback of media which have an audio track, so if play() is called before the load of the resource has loaded metadata, we need to delay starting playback and resolving the play promise until we've figured out whether the media has audio. So if play() is called before we've reached readyState >= HAVE_METADATA, set a flag, and check that flag when we do reach HAVE_METADATA and start the play and resolve the promise then. MozReview-Commit-ID: 1K06NK2kfpw
dom/html/HTMLMediaElement.cpp
dom/html/HTMLMediaElement.h
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -1803,16 +1803,17 @@ void HTMLMediaElement::AbortExistingLoad
   mHaveQueuedSelectResource = false;
   mSuspendedForPreloadNone = false;
   mDownloadSuspendedByCache = false;
   mMediaInfo = MediaInfo();
   mIsEncrypted = false;
   mPendingEncryptedInitData.Reset();
   mWaitingForKey = NOT_WAITING_FOR_KEY;
   mSourcePointer = nullptr;
+  mAttemptPlayUponLoadedMetadata = false;
 
   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) {
@@ -3951,17 +3952,17 @@ void
 HTMLMediaElement::NotifyXPCOMShutdown()
 {
   ShutdownDecoder();
 }
 
 already_AddRefed<Promise>
 HTMLMediaElement::Play(ErrorResult& aRv)
 {
-  LOG(LogLevel::Debug, ("%p Play() called by JS", this));
+  LOG(LogLevel::Debug, ("%p Play() called by JS readyState=%d", this, mReadyState));
 
   if (mAudioChannelWrapper && mAudioChannelWrapper->IsPlaybackBlocked()) {
     MaybeDoLoad();
 
     // A blocked media element will be resumed later, so we return a pending
     // promise which might be resolved/rejected depends on the result of
     // resuming the blocked media element.
     RefPtr<Promise> promise = CreateDOMPromise(aRv);
@@ -3990,17 +3991,22 @@ HTMLMediaElement::PlayInternal(ErrorResu
 
   // 4.8.12.8
   // When the play() method on a media element is invoked, the user agent must
   // run the following steps.
 
   // 4.8.12.8 - Step 1:
   // If the media element is not allowed to play, return a promise rejected
   // with a "NotAllowedError" DOMException and abort these steps.
-  if (!IsAllowedToPlay()) {
+  // Note: IsAllowedToPlay() needs to know whether there is an audio track
+  // in the resource, and for that we need to be at readyState HAVE_METADATA
+  // or above. So only reject here if we're at readyState HAVE_METADATA. If
+  // we're below that, we'll we delay fulfilling the play promise until we've
+  // reached readyState >= HAVE_METADATA below.
+  if (mReadyState >= HAVE_METADATA && !IsAllowedToPlay()) {
     // NOTE: for promise-based-play, will return a rejected promise here.
     LOG(LogLevel::Debug,
         ("%p Play() promise rejected because not allowed to play.", this));
     aRv.Throw(NS_ERROR_DOM_MEDIA_NOT_ALLOWED_ERR);
     return nullptr;
   }
 
   // 4.8.12.8 - Step 2:
@@ -4048,31 +4054,40 @@ HTMLMediaElement::PlayInternal(ErrorResu
 
   // Even if we just did Load() or ResumeLoad(), we could already have a decoder
   // here if we managed to clone an existing decoder.
   if (mDecoder) {
     if (mDecoder->IsEnded()) {
       SetCurrentTime(0);
     }
     if (!mPausedForInactiveDocumentOrChannel) {
-      nsresult rv = mDecoder->Play();
-      if (NS_FAILED(rv)) {
-        // We don't need to remove the _promise_ from _mPendingPlayPromises_ here.
-        // If something wrong between |mPendingPlayPromises.AppendElement(promise);|
-        // and here, the _promise_ should already have been rejected. Otherwise,
-        // the _promise_ won't be returned to JS at all, so just leave it in the
-        // _mPendingPlayPromises_ and let it be resolved/rejected with the
-        // following actions and the promise-resolution won't be observed at all.
-        LOG(LogLevel::Debug,
-            ("%p Play() promise rejected because failed to play MediaDecoder.",
-             this));
-        aRv.Throw(rv);
-        return nullptr;
+      if (mReadyState < HAVE_METADATA) {
+        // We don't know whether the autoplay policy will allow us to play yet,
+        // as we don't yet know whether the media has audio tracks. So delay
+        // starting playback until we've loaded metadata.
+        mAttemptPlayUponLoadedMetadata = true;
+      } else {
+        nsresult rv = mDecoder->Play();
+        if (NS_FAILED(rv)) {
+          // We don't need to remove the _promise_ from _mPendingPlayPromises_ here.
+          // If something wrong between |mPendingPlayPromises.AppendElement(promise);|
+          // and here, the _promise_ should already have been rejected. Otherwise,
+          // the _promise_ won't be returned to JS at all, so just leave it in the
+          // _mPendingPlayPromises_ and let it be resolved/rejected with the
+          // following actions and the promise-resolution won't be observed at all.
+          LOG(LogLevel::Debug,
+              ("%p Play() promise rejected because failed to play MediaDecoder.",
+              this));
+          aRv.Throw(rv);
+          return nullptr;
+        }
       }
     }
+  } else if (mReadyState < HAVE_METADATA) {
+    mAttemptPlayUponLoadedMetadata = true;
   }
 
   if (mCurrentPlayRangeStart == -1.0) {
     mCurrentPlayRangeStart = CurrentTime();
   }
 
   const bool oldPaused = mPaused;
   mPaused = false;
@@ -4921,17 +4936,17 @@ HTMLMediaElement::FinishDecoderSetup(Med
   // This will also do an AddRemoveSelfReference.
   NotifyOwnerDocumentActivityChanged();
 
   if (mPausedForInactiveDocumentOrChannel) {
     mDecoder->Suspend();
   }
 
   nsresult rv = NS_OK;
-  if (!mPaused) {
+  if (!mPaused && !mAttemptPlayUponLoadedMetadata) {
     SetPlayedOrSeeked(true);
     if (!mPausedForInactiveDocumentOrChannel) {
       rv = mDecoder->Play();
     }
   }
 
   if (NS_FAILED(rv)) {
     ShutdownDecoder();
@@ -5925,16 +5940,25 @@ 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 {
+          AsyncRejectPendingPlayPromises(NS_ERROR_DOM_MEDIA_NOT_ALLOWED_ERR);
+          mPaused = true;
+        }
+      }
       NotifyAboutPlaying();
     }
   }
 
   CheckAutoplayDataReady();
 
   if (oldState < HAVE_ENOUGH_DATA &&
       mReadyState >= HAVE_ENOUGH_DATA) {
@@ -6045,16 +6069,17 @@ void HTMLMediaElement::CheckAutoplayData
   UpdateSrcMediaStreamPlaying();
   UpdateAudioChannelPlayingState();
 
   if (mDecoder) {
     SetPlayedOrSeeked(true);
     if (mCurrentPlayRangeStart == -1.0) {
       mCurrentPlayRangeStart = CurrentTime();
     }
+    MOZ_ASSERT(!mAttemptPlayUponLoadedMetadata);
     mDecoder->Play();
   } else if (mSrcStream) {
     SetPlayedOrSeeked(true);
   }
 
   // For blocked media, the event would be pending until it is resumed.
   DispatchAsyncEvent(NS_LITERAL_STRING("play"));
 
@@ -6382,16 +6407,17 @@ void HTMLMediaElement::SuspendOrResumeEl
         mDecoder->Pause();
         mDecoder->Suspend();
       }
       mEventDeliveryPaused = aSuspendEvents;
     } else {
       if (mDecoder) {
         mDecoder->Resume();
         if (!mPaused && !mDecoder->IsEnded()) {
+          MOZ_ASSERT(!mAttemptPlayUponLoadedMetadata);
           mDecoder->Play();
         }
       }
       if (mEventDeliveryPaused) {
         mEventDeliveryPaused = false;
         DispatchPendingMediaEvents();
       }
     }
--- a/dom/html/HTMLMediaElement.h
+++ b/dom/html/HTMLMediaElement.h
@@ -1814,16 +1814,23 @@ private:
   // True if media element has been marked as 'tainted' and can't
   // participate in video decoder suspending.
   bool mHasSuspendTaint = false;
 
   // True if media element has been forced into being considered 'hidden'.
   // For use by mochitests. Enabling pref "media.test.video-suspend"
   bool mForcedHidden = false;
 
+  // True if we attempted to play before the media element had loaded
+  // metadata, and we need to attempt the play once we reach loaded metadata.
+  // If autoplay is disabled, we can't decide whether to allow a play()
+  // until we've loaded metadata, as we need to know whether the resource
+  // has an audio track.
+  bool mAttemptPlayUponLoadedMetadata = false;
+
   // True if audio tracks and video tracks are constructed and added into the
   // track list, false if all tracks are removed from the track list.
   bool mMediaTracksConstructed = false;
 
   Visibility mVisibilityState = Visibility::UNTRACKED;
 
   UniquePtr<ErrorSink> mErrorSink;