Bug 1294615. Part 2 - Refactor MDSM::SetDormant() to remove unnecessary checks for mQueuedSeek.Exists() and mCurrentSeek.Exists(). draft
authorJW Wang <jwwang@mozilla.com>
Thu, 11 Aug 2016 17:33:07 +0800
changeset 400936 67a9d039e6b2f1be273804255b39ca9074eab858
parent 400935 c627db18d83d2ba4d65b200cac760b7f0590f794
child 400947 bdf543985a37d75b6d021756bd3ec5ff0dc3726e
push id26305
push userjwwang@mozilla.com
push dateTue, 16 Aug 2016 01:04:52 +0000
bugs1294615
milestone51.0a1
Bug 1294615. Part 2 - Refactor MDSM::SetDormant() to remove unnecessary checks for mQueuedSeek.Exists() and mCurrentSeek.Exists(). We've proven that when seek is in action, mQueuedSeek.Exists() is always false and mCurrentSeek.Exists() is always true. MozReview-Commit-ID: CxNU45NXG88
dom/media/MediaDecoderStateMachine.cpp
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -1134,77 +1134,77 @@ void
 MediaDecoderStateMachine::SetDormant(bool aDormant)
 {
   MOZ_ASSERT(OnTaskQueue());
 
   if (IsShutdown()) {
     return;
   }
 
+  bool wasDormant = mState == DECODER_STATE_DORMANT;
+  if (wasDormant == aDormant) {
+    return;
+  }
+
   if (mMetadataRequest.Exists()) {
     mPendingDormant = aDormant;
     return;
   }
 
   DECODER_LOG("SetDormant=%d", aDormant);
 
+  // Enter dormant state.
   if (aDormant) {
     if (mState == DECODER_STATE_SEEKING) {
-      if (mQueuedSeek.Exists()) {
-        // Keep latest seek target
-      } else if (mCurrentSeek.Exists()) {
-        // Because both audio and video decoders are going to be reset in this
-        // method later, we treat a VideoOnly seek task as a normal Accurate
-        // seek task so that while it is resumed, both audio and video playback
-        // are handled.
-        if (mCurrentSeek.mTarget.IsVideoOnly()) {
-          mCurrentSeek.mTarget.SetType(SeekTarget::Accurate);
-          mCurrentSeek.mTarget.SetVideoOnly(false);
-        }
-        mQueuedSeek = Move(mCurrentSeek);
-        mSeekTaskRequest.DisconnectIfExists();
-      } else {
-        mQueuedSeek.mTarget = SeekTarget(mCurrentPosition,
-                                         SeekTarget::Accurate,
-                                         MediaDecoderEventVisibility::Suppressed);
-        // XXXbholley - Nobody is listening to this promise. Do we need to pass it
-        // back to MediaDecoder when we come out of dormant?
-        RefPtr<MediaDecoder::SeekPromise> unused = mQueuedSeek.mPromise.Ensure(__func__);
+      MOZ_ASSERT(!mQueuedSeek.Exists());
+      MOZ_ASSERT(mCurrentSeek.Exists());
+      // Because both audio and video decoders are going to be reset in this
+      // method later, we treat a VideoOnly seek task as a normal Accurate
+      // seek task so that while it is resumed, both audio and video playback
+      // are handled.
+      if (mCurrentSeek.mTarget.IsVideoOnly()) {
+        mCurrentSeek.mTarget.SetType(SeekTarget::Accurate);
+        mCurrentSeek.mTarget.SetVideoOnly(false);
       }
+      mQueuedSeek = Move(mCurrentSeek);
     } else {
       mQueuedSeek.mTarget = SeekTarget(mCurrentPosition,
                                        SeekTarget::Accurate,
                                        MediaDecoderEventVisibility::Suppressed);
-      // XXXbholley - Nobody is listening to this promise. Do we need to pass it
-      // back to MediaDecoder when we come out of dormant?
+      // SeekJob asserts |mTarget.IsValid() == !mPromise.IsEmpty()| so we
+      // need to create the promise even it is not used at all.
       RefPtr<MediaDecoder::SeekPromise> unused = mQueuedSeek.mPromise.Ensure(__func__);
     }
 
+    SetState(DECODER_STATE_DORMANT);
+
     // Discard the current seek task.
     DiscardSeekTaskIfExist();
 
-    SetState(DECODER_STATE_DORMANT);
     if (IsPlaying()) {
       StopPlayback();
     }
 
     Reset();
 
     // Note that we do not wait for the decode task queue to go idle before
     // queuing the ReleaseMediaResources task - instead, we disconnect promises,
     // reset state, and put a ResetDecode in the decode task queue. Any tasks
-    // that run after ResetDecode are supposed to run with a clean slate. We rely
-    // on that in other places (i.e. seeking), so it seems reasonable to rely on
-    // it here as well.
+    // that run after ResetDecode are supposed to run with a clean slate. We
+    // rely on that in other places (i.e. seeking), so it seems reasonable to
+    // rely on it here as well.
     mReader->ReleaseMediaResources();
-  } else if (mState == DECODER_STATE_DORMANT) {
-    mDecodingFirstFrame = true;
-    SetState(DECODER_STATE_DECODING_METADATA);
-    ReadMetadata();
+
+    return;
   }
+
+  // Exit dormant state.
+  SetState(DECODER_STATE_DECODING_METADATA);
+  mDecodingFirstFrame = true;
+  ReadMetadata();
 }
 
 RefPtr<ShutdownPromise>
 MediaDecoderStateMachine::Shutdown()
 {
   MOZ_ASSERT(OnTaskQueue());
 
   // Once we've entered the shutdown state here there's no going back.