Bug 1312321. Part 1 - make StateObject::SetState pass-by-value. r=JamesCheng
authorJW Wang <jwwang@mozilla.com>
Mon, 24 Oct 2016 11:43:57 +0800
changeset 362122 b7fb1a72dd5cfe9342a6f5284da74502de1bea36
parent 362121 2be061c5d070c962862c62bed8171b5d41654533
child 362123 bf7df2a398a4a1ccb4a3ed0335973c0bd012483f
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-beta@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersJamesCheng
bugs1312321
milestone52.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 1312321. Part 1 - make StateObject::SetState pass-by-value. r=JamesCheng MozReview-Commit-ID: 34Y4bNAGmbk
dom/media/MediaDecoderStateMachine.cpp
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/dom/media/MediaDecoderStateMachine.cpp
@@ -232,39 +232,35 @@ protected:
 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>
-  auto SetState(Ts&&... aArgs)
-    -> decltype(DeclVal<S>().Enter(Forward<Ts>(aArgs)...))
+  auto SetState(Ts... aArgs)
+    -> decltype(DeclVal<S>().Enter(Move(aArgs)...))
   {
     // keep mMaster in a local object because mMaster will become invalid after
     // the current state object is deleted.
     auto master = mMaster;
 
     auto s = new S(master);
 
     MOZ_ASSERT(master->mState != s->GetState() ||
                master->mState == DECODER_STATE_SEEKING);
 
     SLOG("change state to: %s", ToStateStr(s->GetState()));
 
     Exit();
 
-    // Note |aArgs| might reference data members of |this|. We need to keep
-    // |this| alive until |s->Enter()| returns.
-    UniquePtr<StateObject> deathGrip(master->mStateObj.release());
-
     master->mState = s->GetState();
     master->mStateObj.reset(s);
-    return s->Enter(Forward<Ts>(aArgs)...);
+    return s->Enter(Move(aArgs)...);
   }
 
 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;
 };
 
@@ -1289,31 +1285,29 @@ WaitForCDMState::HandleDormant(bool aDor
 }
 
 bool
 MediaDecoderStateMachine::
 DormantState::HandleDormant(bool aDormant)
 {
   if (!aDormant) {
     MOZ_ASSERT(!Info().IsEncrypted() || mMaster->mCDMProxy);
-    SeekJob seekJob = Move(mPendingSeek);
-    SetState<DecodingFirstFrameState>(Move(seekJob));
+    SetState<DecodingFirstFrameState>(Move(mPendingSeek));
   }
   return true;
 }
 
 bool
 MediaDecoderStateMachine::
 WaitForCDMState::HandleCDMProxyReady()
 {
-  SeekJob seekJob = Move(mPendingSeek);
   if (mPendingDormant) {
-    SetState<DormantState>(Move(seekJob));
+    SetState<DormantState>(Move(mPendingSeek));
   } else {
-    SetState<DecodingFirstFrameState>(Move(seekJob));
+    SetState<DecodingFirstFrameState>(Move(mPendingSeek));
   }
   return true;
 }
 
 void
 MediaDecoderStateMachine::
 DecodingFirstFrameState::Enter(SeekJob aPendingSeek)
 {
@@ -1364,18 +1358,17 @@ DecodingFirstFrameState::HandleSeek(Seek
   return SetState<SeekingState>(Move(seekJob));
 }
 
 bool
 MediaDecoderStateMachine::
 DecodingFirstFrameState::HandleDormant(bool aDormant)
 {
   if (aDormant) {
-    SeekJob seekJob = Move(mPendingSeek);
-    SetState<DormantState>(Move(seekJob));
+    SetState<DormantState>(Move(mPendingSeek));
   }
   return true;
 }
 
 void
 MediaDecoderStateMachine::
 DecodingFirstFrameState::MaybeFinishDecodeFirstFrame()
 {
@@ -1384,18 +1377,17 @@ DecodingFirstFrameState::MaybeFinishDeco
   if ((mMaster->IsAudioDecoding() && mMaster->AudioQueue().GetSize() == 0) ||
       (mMaster->IsVideoDecoding() && mMaster->VideoQueue().GetSize() == 0)) {
     return;
   }
 
   mMaster->FinishDecodeFirstFrame();
 
   if (mPendingSeek.Exists()) {
-    SeekJob seekJob = Move(mPendingSeek);
-    SetState<SeekingState>(Move(seekJob));
+    SetState<SeekingState>(Move(mPendingSeek));
   } else {
     SetState<DecodingState>();
   }
 }
 
 void
 MediaDecoderStateMachine::
 DecodingState::Enter()
@@ -1495,18 +1487,17 @@ SeekingState::HandleDormant(bool aDorman
   // 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 (mSeekJob.mTarget.IsVideoOnly()) {
     mSeekJob.mTarget.SetType(SeekTarget::Accurate);
     mSeekJob.mTarget.SetVideoOnly(false);
   }
-  SeekJob seekJob = Move(mSeekJob);
-  SetState<DormantState>(Move(seekJob));
+  SetState<DormantState>(Move(mSeekJob));
   return true;
 }
 
 RefPtr<MediaDecoder::SeekPromise>
 MediaDecoderStateMachine::
 SeekingState::HandleSeek(SeekTarget aTarget)
 {
   SLOG("Changed state to SEEKING (to %lld)", aTarget.GetTime().ToMicroseconds());