Bug 1458566 - Make MediaDecoder::Play() return void. r?bryce draft
authorJean-Yves Avenard <jyavenard@mozilla.com>
Wed, 02 May 2018 17:27:27 +0200
changeset 790670 5de059ee73ed1547d9adfdbf9b7004fd3872ff5a
parent 790669 ef38bff0183dc0edc2cd3287de2f6e17f952abc6
push id108552
push userbmo:jyavenard@mozilla.com
push dateWed, 02 May 2018 15:30:24 +0000
reviewersbryce
bugs1458566
milestone61.0a1
Bug 1458566 - Make MediaDecoder::Play() return void. r?bryce MediaDecoder::Play() cannot fail and was always returning NS_OK MozReview-Commit-ID: 7OgwZQw569Y
dom/html/HTMLMediaElement.cpp
dom/media/MediaDecoder.cpp
dom/media/MediaDecoder.h
dom/media/hls/HLSDecoder.cpp
dom/media/hls/HLSDecoder.h
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -4047,30 +4047,17 @@ HTMLMediaElement::PlayInternal(ErrorResu
     }
     if (!mPausedForInactiveDocumentOrChannel) {
       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));
-          promise->MaybeReject(rv);
-          return promise.forget();
-        }
+        mDecoder->Play();
       }
     }
   } else if (mReadyState < HAVE_METADATA) {
     mAttemptPlayUponLoadedMetadata = true;
   }
 
   if (mCurrentPlayRangeStart == -1.0) {
     mCurrentPlayRangeStart = CurrentTime();
@@ -4928,29 +4915,24 @@ HTMLMediaElement::FinishDecoderSetup(Med
   // We may want to suspend the new stream now.
   // This will also do an AddRemoveSelfReference.
   NotifyOwnerDocumentActivityChanged();
 
   if (mPausedForInactiveDocumentOrChannel) {
     mDecoder->Suspend();
   }
 
-  nsresult rv = NS_OK;
   if (!mPaused && !mAttemptPlayUponLoadedMetadata) {
     SetPlayedOrSeeked(true);
     if (!mPausedForInactiveDocumentOrChannel) {
-      rv = mDecoder->Play();
-    }
-  }
-
-  if (NS_FAILED(rv)) {
-    ShutdownDecoder();
-  }
-
-  return rv;
+      mDecoder->Play();
+    }
+  }
+
+  return NS_OK;
 }
 
 class HTMLMediaElement::StreamListener : public MediaStreamListener
 {
 public:
   StreamListener(HTMLMediaElement* aElement, const char* aName) :
     mElement(aElement),
     mHaveCurrentData(false),
--- a/dom/media/MediaDecoder.cpp
+++ b/dom/media/MediaDecoder.cpp
@@ -493,37 +493,36 @@ MediaDecoder::SetStateMachineParameters(
   mOnEncrypted = mReader->OnEncrypted().Connect(
     mAbstractMainThread, GetOwner(), &MediaDecoderOwner::DispatchEncrypted);
   mOnWaitingForKey = mReader->OnWaitingForKey().Connect(
     mAbstractMainThread, GetOwner(), &MediaDecoderOwner::NotifyWaitingForKey);
   mOnDecodeWarning = mReader->OnDecodeWarning().Connect(
     mAbstractMainThread, GetOwner(), &MediaDecoderOwner::DecodeWarning);
 }
 
-nsresult
+void
 MediaDecoder::Play()
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   AbstractThread::AutoEnter context(AbstractMainThread());
   NS_ASSERTION(mDecoderStateMachine != nullptr, "Should have state machine.");
   if (mPlaybackRate == 0) {
-    return NS_OK;
+    return;
   }
 
   if (IsEnded()) {
     Seek(0, SeekTarget::PrevSyncPoint);
-    return NS_OK;
+    return;
   } else if (mPlayState == PLAY_STATE_LOADING) {
     mNextState = PLAY_STATE_PLAYING;
-    return NS_OK;
+    return;
   }
 
   ChangeState(PLAY_STATE_PLAYING);
-  return NS_OK;
 }
 
 void
 MediaDecoder::Seek(double aTime, SeekTarget::Type aSeekType)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_DIAGNOSTIC_ASSERT(!IsShutdown());
 
--- a/dom/media/MediaDecoder.h
+++ b/dom/media/MediaDecoder.h
@@ -136,17 +136,17 @@ public:
   // the seek target.
   void Seek(double aTime, SeekTarget::Type aSeekType);
 
   // Initialize state machine and schedule it.
   nsresult InitializeStateMachine();
 
   // Start playback of a video. 'Load' must have previously been
   // called.
-  virtual nsresult Play();
+  virtual void Play();
 
   // Notify activity of the decoder owner is changed.
   virtual void NotifyOwnerActivityChanged(bool aIsDocumentVisible,
                                           Visibility aElementVisibility,
                                           bool aIsElementInTree);
 
   // Pause video playback.
   virtual void Pause();
--- a/dom/media/hls/HLSDecoder.cpp
+++ b/dom/media/hls/HLSDecoder.cpp
@@ -187,17 +187,17 @@ HLSDecoder::GetCurrentPrincipal()
   nsIScriptSecurityManager* secMan = nsContentUtils::GetSecurityManager();
   if (!secMan || !mChannel) {
     return nullptr;
   }
   secMan->GetChannelResultPrincipal(mChannel, getter_AddRefs(principal));
   return principal.forget();
 }
 
-nsresult
+void
 HLSDecoder::Play()
 {
   MOZ_ASSERT(NS_IsMainThread());
   HLS_DEBUG("HLSDecoder", "MediaElement called Play");
   mHLSResourceWrapper->Play();
   return MediaDecoder::Play();
 }
 
--- a/dom/media/hls/HLSDecoder.h
+++ b/dom/media/hls/HLSDecoder.h
@@ -25,17 +25,17 @@ public:
 
   // Returns true if aContainerType is an HLS type that we think we can render
   // with the a platform decoder backend.
   // If provided, codecs are checked for support.
   static bool IsSupportedType(const MediaContainerType& aContainerType);
 
   nsresult Load(nsIChannel* aChannel);
 
-  nsresult Play() override;
+  void Play() override;
 
   void Pause() override;
 
   void AddSizeOfResources(ResourceSizes* aSizes) override;
   already_AddRefed<nsIPrincipal> GetCurrentPrincipal() override;
   bool IsTransportSeekable() override { return true; }
   void Suspend() override;
   void Resume() override;