Bug 1157803 - Mirror mPlayState and mNextState to the state machine task queue and eliminate cross-thread access. r=jww

dom/media/MediaDecoder.cpp

@@ -596,8 +596,6 @@ MediaDecoder::MediaDecoder() :
   mMediaSeekable(true),
   mSameOriginMedia(false),
   mReentrantMonitor("media.decoder"),
-  mPlayState(PLAY_STATE_LOADING, "MediaDecoder::mPlayState"),
-  mNextState(PLAY_STATE_PAUSED),
   mIgnoreProgressData(false),
   mInfiniteStream(false),
   mOwner(nullptr),
@@ -626,12 +624,18 @@ MediaDecoder::MediaDecoder() :
   EnsureStateWatchingLog();
 #endif
 
+  // Initialize canonicals.
+  mPlayState.Init(AbstractThread::MainThread(), PLAY_STATE_LOADING, "MediaDecoder::mPlayState (Canonical)");
+  mNextState.Init(AbstractThread::MainThread(), PLAY_STATE_PAUSED, "MediaDecoder::mNextState (Canonical)");
+
+  // Initialize mirrors.
   mNextFrameStatus.Init(AbstractThread::MainThread(), MediaDecoderOwner::NEXT_FRAME_UNINITIALIZED,
                         "MediaDecoder::mNextFrameStatus (Mirror)");
 
 
   mAudioChannel = AudioChannelService::GetDefaultAudioChannel();
 
+  // Initialize watchers.
   mReadyStateWatchTarget->Watch(mPlayState);
   mReadyStateWatchTarget->Watch(mNextFrameStatus);
 }
@@ -828,13 +832,6 @@ nsresult MediaDecoder::Seek(double aTime, SeekTarget::Type aSeekType)
   return ScheduleStateMachineThread();
 }
 
-bool MediaDecoder::IsLogicallyPlaying()
-{
-  GetReentrantMonitor().AssertCurrentThreadIn();
-  return mPlayState == PLAY_STATE_PLAYING ||
-         mNextState == PLAY_STATE_PLAYING;
-}
-
 double MediaDecoder::GetCurrentTime()
 {
   MOZ_ASSERT(NS_IsMainThread());

dom/media/MediaDecoder.h

@@ -703,12 +703,6 @@ public:
   }
   layers::ImageContainer* GetImageContainer() override;
 
-  // Return the current state. Can be called on any thread. If called from
-  // a non-main thread, the decoder monitor must be held.
-  PlayState GetState() {
-    return mPlayState;
-  }
-
   // Fire timeupdate events if needed according to the time constraints
   // outlined in the specification.
   void FireTimeUpdate();
@@ -847,13 +841,6 @@ public:
 
   MediaDecoderOwner* GetOwner() override;
 
-  // Returns true if we're logically playing, that is, if the Play() has
-  // been called and Pause() has not or we have not yet reached the end
-  // of media. This is irrespective of the seeking state; if the owner
-  // calls Play() and then Seek(), we still count as logically playing.
-  // The decoder monitor must be held.
-  bool IsLogicallyPlaying();
-
 #ifdef MOZ_EME
   // This takes the decoder monitor.
   virtual nsresult SetCDMProxy(CDMProxy* aProxy) override;
@@ -1149,15 +1136,18 @@ protected:
   // OR on the main thread.
   // Any change to the state on the main thread must call NotifyAll on the
   // monitor so the decode thread can wake up.
-  Watchable<PlayState> mPlayState;
+  Canonical<PlayState>::Holder mPlayState;
 
-  // The state to change to after a seek or load operation.
   // This can only be changed on the main thread while holding the decoder
   // monitor. Thus, it can be safely read while holding the decoder monitor
   // OR on the main thread.
   // Any change to the state must call NotifyAll on the monitor.
   // This can only be PLAY_STATE_PAUSED or PLAY_STATE_PLAYING.
-  PlayState mNextState;
+  Canonical<PlayState>::Holder mNextState;
+public:
+  AbstractCanonical<PlayState>* CanonicalPlayState() { return &mPlayState; }
+  AbstractCanonical<PlayState>* CanonicalNextPlayState() { return &mNextState; }
+protected:
 
   // Position to seek to when the seek notification is received by the
   // decode thread.

dom/media/MediaDecoderStateMachine.cpp

@@ -255,9 +255,16 @@ MediaDecoderStateMachine::MediaDecoderStateMachine(MediaDecoder* aDecoder,
   MOZ_DIAGNOSTIC_ASSERT(pool);
   mTaskQueue = new MediaTaskQueue(pool.forget(), /* aAssertTailDispatch = */ true);
 
+  // Initialize canonicals.
   mNextFrameStatus.Init(mTaskQueue, MediaDecoderOwner::NEXT_FRAME_UNINITIALIZED,
                         "MediaDecoderStateMachine::mNextFrameStatus (Canonical)");
 
+  // Initialize mirrors.
+  mPlayState.Init(mTaskQueue, MediaDecoder::PLAY_STATE_LOADING, "MediaDecoderStateMachine::mPlayState (Mirror)",
+                  aDecoder->CanonicalPlayState());
+  mNextPlayState.Init(mTaskQueue, MediaDecoder::PLAY_STATE_PAUSED, "MediaDecoderStateMachine::mNextPlayState (Mirror)",
+                  aDecoder->CanonicalNextPlayState());
+
   // Skip the initial notification we get when we Watch the value, since we're
   // not on the right thread yet.
   mNextFrameStatusUpdater->Watch(mState, /* aSkipInitialNotify = */ true);
@@ -1222,7 +1229,7 @@ void MediaDecoderStateMachine::MaybeStartPlayback()
     return;
   }
 
-  bool playStatePermits = mDecoder->GetState() == MediaDecoder::PLAY_STATE_PLAYING;
+  bool playStatePermits = mPlayState == MediaDecoder::PLAY_STATE_PLAYING;
   bool decodeStatePermits = mState == DECODER_STATE_DECODING || mState == DECODER_STATE_COMPLETED;
   if (!playStatePermits || !decodeStatePermits || mIsAudioPrerolling || mIsVideoPrerolling) {
     DECODER_LOG("Not starting playback [playStatePermits: %d, decodeStatePermits: %d, "
@@ -1797,7 +1804,7 @@ MediaDecoderStateMachine::DispatchDecodeTasksIfNeeded()
   MOZ_ASSERT(mState != DECODER_STATE_COMPLETED ||
              (!needToDecodeAudio && !needToDecodeVideo));
 
-  bool needIdle = !mDecoder->IsLogicallyPlaying() &&
+  bool needIdle = !IsLogicallyPlaying() &&
                   mState != DECODER_STATE_SEEKING &&
                   !needToDecodeAudio &&
                   !needToDecodeVideo &&
@@ -2513,7 +2520,9 @@ MediaDecoderStateMachine::FinishShutdown()
   // mPendingWakeDecoder being needed again. Revoke it.
   mPendingWakeDecoder = nullptr;
 
-  // Disconnect mirrors before shutting down our task queue.
+  // Disconnect canonicals and mirrors before shutting down our task queue.
+  mPlayState.DisconnectIfConnected();
+  mNextPlayState.DisconnectIfConnected();
   mNextFrameStatus.DisconnectAll();
 
   MOZ_ASSERT(mState == DECODER_STATE_SHUTDOWN,
@@ -2616,8 +2625,7 @@ nsresult MediaDecoderStateMachine::RunStateMachine()
     }
 
     case DECODER_STATE_DECODING: {
-      if (mDecoder->GetState() != MediaDecoder::PLAY_STATE_PLAYING &&
-          IsPlaying())
+      if (mPlayState != MediaDecoder::PLAY_STATE_PLAYING && IsPlaying())
       {
         // We're playing, but the element/decoder is in paused state. Stop
         // playing!
@@ -2628,7 +2636,7 @@ nsresult MediaDecoderStateMachine::RunStateMachine()
       MaybeStartPlayback();
 
       AdvanceFrame();
-      NS_ASSERTION(mDecoder->GetState() != MediaDecoder::PLAY_STATE_PLAYING ||
+      NS_ASSERTION(mPlayState != MediaDecoder::PLAY_STATE_PLAYING ||
                    IsStateMachineScheduled() ||
                    mPlaybackRate == 0.0, "Must have timer scheduled");
       return NS_OK;
@@ -2696,9 +2704,8 @@ nsresult MediaDecoderStateMachine::RunStateMachine()
           (mAudioCaptured && !mDecoder->GetDecodedStream()->IsFinished()))
       {
         AdvanceFrame();
-        NS_ASSERTION(mDecoder->GetState() != MediaDecoder::PLAY_STATE_PLAYING ||
-                     mPlaybackRate == 0 ||
-                     IsStateMachineScheduled(),
+        NS_ASSERTION(mPlayState != MediaDecoder::PLAY_STATE_PLAYING ||
+                     mPlaybackRate == 0 || IsStateMachineScheduled(),
                      "Must have timer scheduled");
         return NS_OK;
       }
@@ -2716,7 +2723,7 @@ nsresult MediaDecoderStateMachine::RunStateMachine()
 
       StopAudioThread();
 
-      if (mDecoder->GetState() == MediaDecoder::PLAY_STATE_PLAYING &&
+      if (mPlayState == MediaDecoder::PLAY_STATE_PLAYING &&
           !mSentPlaybackEndedEvent)
       {
         int64_t clockTime = std::max(mAudioEndTime, mVideoFrameEndTime);
@@ -2896,7 +2903,7 @@ void MediaDecoderStateMachine::AdvanceFrame()
   NS_ASSERTION(!HasAudio() || mAudioStartTime != -1,
                "Should know audio start time if we have audio.");
 
-  if (mDecoder->GetState() != MediaDecoder::PLAY_STATE_PLAYING) {
+  if (mPlayState != MediaDecoder::PLAY_STATE_PLAYING) {
     return;
   }
 
@@ -2953,7 +2960,7 @@ void MediaDecoderStateMachine::AdvanceFrame()
   // Check to see if we don't have enough data to play up to the next frame.
   // If we don't, switch to buffering mode.
   if (mState == DECODER_STATE_DECODING &&
-      mDecoder->GetState() == MediaDecoder::PLAY_STATE_PLAYING &&
+      mPlayState == MediaDecoder::PLAY_STATE_PLAYING &&
       mDecoder->IsExpectingMoreData()) {
     bool shouldBuffer;
     if (mReader->UseBufferingHeuristics()) {

dom/media/MediaDecoderStateMachine.h

@@ -885,6 +885,22 @@ public:
   // as mStartTime and mEndTime could have been set separately.
   bool mDurationSet;
 
+  // The current play state and next play state, mirrored from the main thread.
+  Mirror<MediaDecoder::PlayState>::Holder mPlayState;
+  Mirror<MediaDecoder::PlayState>::Holder mNextPlayState;
+
+  // Returns true if we're logically playing, that is, if the Play() has
+  // been called and Pause() has not or we have not yet reached the end
+  // of media. This is irrespective of the seeking state; if the owner
+  // calls Play() and then Seek(), we still count as logically playing.
+  // The decoder monitor must be held.
+  bool IsLogicallyPlaying()
+  {
+    MOZ_ASSERT(OnTaskQueue());
+    return mPlayState == MediaDecoder::PLAY_STATE_PLAYING ||
+           mNextPlayState == MediaDecoder::PLAY_STATE_PLAYING;
+  }
+
   // The status of our next frame. Mirrored on the main thread and used to
   // compute ready state.
   WatcherHolder mNextFrameStatusUpdater;