Bug 1309116. Part 3 - replace InitiateSeek() with StateObject::SetState(). draft
authorJW Wang <jwwang@mozilla.com>
Tue, 11 Oct 2016 14:56:52 +0800
changeset 425045 5f32414bc1448b8dbb6586141117b65548118cdf
parent 425044 ea4a14f8e64228e492000611c0e4e70c44792369
child 425046 77c4344c0366d6e57d1a14d53e23e2ee4287bcde
push id32322
push userjwwang@mozilla.com
push dateFri, 14 Oct 2016 02:59:40 +0000
bugs1309116
milestone52.0a1
Bug 1309116. Part 3 - replace InitiateSeek() with StateObject::SetState(). MozReview-Commit-ID: 3iXZ5r402rz
dom/media/MediaDecoderStateMachine.cpp
dom/media/MediaDecoderStateMachine.h
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -219,38 +219,45 @@ public:
 protected:
   using Master = MediaDecoderStateMachine;
   explicit StateObject(Master* aPtr) : mMaster(aPtr) {}
   TaskQueue* OwnerThread() const { return mMaster->mTaskQueue; }
   MediaResource* Resource() const { return mMaster->mResource; }
   MediaDecoderReaderWrapper* Reader() const { return mMaster->mReader; }
   const MediaInfo& Info() const { return mMaster->Info(); }
 
+public:
+  // TODO: this function is public because VisibilityChanged() calls it.
+  // We should handle visibility changes in state objects so this function
+  // can be made protected again.
+  //
   // Note this function will delete the current state object.
   // Don't access members to avoid UAF after this call.
   template <class S, typename... Ts>
   void SetState(Ts&&... aArgs)
   {
     // keep mMaster in a local object because mMaster will become invalid after
     // the current state object is deleted.
     auto master = mMaster;
 
     UniquePtr<StateObject> s = MakeUnique<S>(master, Forward<Ts>(aArgs)...);
-    if (master->mState == s->GetState()) {
+    if (master->mState == s->GetState() &&
+        master->mState != DECODER_STATE_SEEKING) {
       return;
     }
 
     SLOG("change state to: %s", ToStateStr(s->GetState()));
 
     Exit();
     master->mState = s->GetState();
     master->mStateObj = Move(s); // Will delete |this|!
     master->mStateObj->Enter();
   }
 
+protected:
   // Take a raw pointer in order not to change the life cycle of MDSM.
   // It is guaranteed to be valid by MDSM.
   Master* mMaster;
 };
 
 class MediaDecoderStateMachine::DecodeMetadataState
   : public MediaDecoderStateMachine::StateObject
 {
@@ -1005,17 +1012,17 @@ WaitForCDMState::HandleCDMProxyReady()
 void
 MediaDecoderStateMachine::
 DecodingFirstFrameState::Enter()
 {
   // Handle pending seek.
   if (mMaster->mQueuedSeek.Exists() &&
       (mMaster->mSentFirstFrameLoadedEvent ||
        Reader()->ForceZeroStartTime())) {
-    mMaster->InitiateSeek(Move(mMaster->mQueuedSeek));
+    SetState<SeekingState>(Move(mMaster->mQueuedSeek));
     return;
   }
 
   // Transition to DECODING if we've decoded first frames.
   if (mMaster->mSentFirstFrameLoadedEvent) {
     SetState<DecodingState>();
     return;
   }
@@ -1042,17 +1049,17 @@ DecodingFirstFrameState::HandleSeek(Seek
   // Since ForceZeroStartTime() is true, we should've transitioned to SEEKING
   // in Enter() if there is any queued seek.
   MOZ_ASSERT(!mMaster->mQueuedSeek.Exists());
 
   SLOG("Changed state to SEEKING (to %lld)", aTarget.GetTime().ToMicroseconds());
   SeekJob seekJob;
   seekJob.mTarget = aTarget;
   RefPtr<MediaDecoder::SeekPromise> p = seekJob.mPromise.Ensure(__func__);
-  mMaster->InitiateSeek(Move(seekJob));
+  SetState<SeekingState>(Move(seekJob));
   return p.forget();
 }
 
 void
 MediaDecoderStateMachine::
 DecodingFirstFrameState::MaybeFinishDecodeFirstFrame()
 {
   MOZ_ASSERT(!mMaster->mSentFirstFrameLoadedEvent);
@@ -1060,17 +1067,17 @@ DecodingFirstFrameState::MaybeFinishDeco
   if ((mMaster->IsAudioDecoding() && mMaster->AudioQueue().GetSize() == 0) ||
       (mMaster->IsVideoDecoding() && mMaster->VideoQueue().GetSize() == 0)) {
     return;
   }
 
   mMaster->FinishDecodeFirstFrame();
 
   if (mMaster->mQueuedSeek.Exists()) {
-    mMaster->InitiateSeek(Move(mMaster->mQueuedSeek));
+    SetState<SeekingState>(Move(mMaster->mQueuedSeek));
   } else {
     SetState<DecodingState>();
   }
 }
 
 void
 MediaDecoderStateMachine::
 DecodingState::Enter()
@@ -1099,17 +1106,17 @@ RefPtr<MediaDecoder::SeekPromise>
 MediaDecoderStateMachine::
 DecodingState::HandleSeek(SeekTarget aTarget)
 {
   mMaster->mQueuedSeek.RejectIfExists(__func__);
   SLOG("Changed state to SEEKING (to %lld)", aTarget.GetTime().ToMicroseconds());
   SeekJob seekJob;
   seekJob.mTarget = aTarget;
   RefPtr<MediaDecoder::SeekPromise> p = seekJob.mPromise.Ensure(__func__);
-  mMaster->InitiateSeek(Move(seekJob));
+  SetState<SeekingState>(Move(seekJob));
   return p.forget();
 }
 
 bool
 MediaDecoderStateMachine::
 DecodingState::HandleEndOfStream()
 {
   if (mMaster->CheckIfDecodeComplete()) {
@@ -1146,17 +1153,17 @@ RefPtr<MediaDecoder::SeekPromise>
 MediaDecoderStateMachine::
 SeekingState::HandleSeek(SeekTarget aTarget)
 {
   mMaster->mQueuedSeek.RejectIfExists(__func__);
   SLOG("Changed state to SEEKING (to %lld)", aTarget.GetTime().ToMicroseconds());
   SeekJob seekJob;
   seekJob.mTarget = aTarget;
   RefPtr<MediaDecoder::SeekPromise> p = seekJob.mPromise.Ensure(__func__);
-  mMaster->InitiateSeek(Move(seekJob));
+  SetState<SeekingState>(Move(seekJob));
   return p.forget();
 }
 
 void
 MediaDecoderStateMachine::
 SeekingState::SeekCompleted()
 {
   int64_t seekTime = mSeekTask->GetSeekTarget().GetTime().ToMicroseconds();
@@ -1297,30 +1304,30 @@ RefPtr<MediaDecoder::SeekPromise>
 MediaDecoderStateMachine::
 BufferingState::HandleSeek(SeekTarget aTarget)
 {
   mMaster->mQueuedSeek.RejectIfExists(__func__);
   SLOG("Changed state to SEEKING (to %lld)", aTarget.GetTime().ToMicroseconds());
   SeekJob seekJob;
   seekJob.mTarget = aTarget;
   RefPtr<MediaDecoder::SeekPromise> p = seekJob.mPromise.Ensure(__func__);
-  mMaster->InitiateSeek(Move(seekJob));
+  SetState<SeekingState>(Move(seekJob));
   return p.forget();
 }
 
 RefPtr<MediaDecoder::SeekPromise>
 MediaDecoderStateMachine::
 CompletedState::HandleSeek(SeekTarget aTarget)
 {
   mMaster->mQueuedSeek.RejectIfExists(__func__);
   SLOG("Changed state to SEEKING (to %lld)", aTarget.GetTime().ToMicroseconds());
   SeekJob seekJob;
   seekJob.mTarget = aTarget;
   RefPtr<MediaDecoder::SeekPromise> p = seekJob.mPromise.Ensure(__func__);
-  mMaster->InitiateSeek(Move(seekJob));
+  SetState<SeekingState>(Move(seekJob));
   return p.forget();
 }
 
 #define INIT_WATCHABLE(name, val) \
   name(val, "MediaDecoderStateMachine::" #name)
 #define INIT_MIRROR(name, val) \
   name(mTaskQueue, val, "MediaDecoderStateMachine::" #name " (Mirror)")
 #define INIT_CANONICAL(name, val) \
@@ -2330,17 +2337,17 @@ void MediaDecoderStateMachine::Visibilit
                                  type,
                                  MediaDecoderEventVisibility::Suppressed,
                                  true /* aVideoOnly */);
 
     RefPtr<MediaDecoder::SeekPromise> p = seekJob.mPromise.Ensure(__func__);
     p->Then(AbstractThread::MainThread(), __func__,
             [start, info, hw](){ ReportRecoveryTelemetry(start, info, hw); },
             [](){});
-    InitiateSeek(Move(seekJob));
+    mStateObj->SetState<SeekingState>(Move(seekJob));
   }
 }
 
 void MediaDecoderStateMachine::BufferedRangeUpdated()
 {
   MOZ_ASSERT(OnTaskQueue());
 
   // While playing an unseekable stream of unknown duration, mObservedDuration
@@ -2463,29 +2470,16 @@ MediaDecoderStateMachine::DispatchDecode
     DECODER_LOG("Dispatching SetIdle() audioQueue=%lld videoQueue=%lld",
                 GetDecodedAudioDuration(),
                 VideoQueue().Duration());
     mReader->SetIdle();
   }
 }
 
 void
-MediaDecoderStateMachine::InitiateSeek(SeekJob aSeekJob)
-{
-  MOZ_ASSERT(OnTaskQueue());
-
-  // Note we can't call SetState(DECODER_STATE_SEEKING) which does nothing
-  // if we are already in the SEEKING state.
-  mStateObj->Exit();
-  mState = DECODER_STATE_SEEKING;
-  mStateObj = MakeUnique<SeekingState>(this, Move(aSeekJob));
-  mStateObj->Enter();
-}
-
-void
 MediaDecoderStateMachine::DispatchAudioDecodeTaskIfNeeded()
 {
   MOZ_ASSERT(OnTaskQueue());
   if (!IsShutdown() && NeedToDecodeAudio()) {
     EnsureAudioDecodeTaskQueued();
   }
 }
 
--- a/dom/media/MediaDecoderStateMachine.h
+++ b/dom/media/MediaDecoderStateMachine.h
@@ -470,19 +470,16 @@ protected:
 
   // Dispatches a LoadedMetadataEvent.
   // This is threadsafe and can be called on any thread.
   // The decoder monitor must be held.
   void EnqueueLoadedMetadataEvent();
 
   void EnqueueFirstFrameLoadedEvent();
 
-  // Clears any previous seeking state and initiates a new seek on the decoder.
-  void InitiateSeek(SeekJob aSeekJob);
-
   void DispatchAudioDecodeTaskIfNeeded();
   void DispatchVideoDecodeTaskIfNeeded();
 
   // Dispatch a task to decode audio if there is not.
   void EnsureAudioDecodeTaskQueued();
 
   // Dispatch a task to decode video if there is not.
   void EnsureVideoDecodeTaskQueued();