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

2015-04-24 20:35:06 -07:00 · 2015-04-24 20:35:06 -07:00 · cd25ae6fac
--- a/dom/media/MediaDecoder.cpp
+++ b/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());
--- a/dom/media/MediaDecoder.h
+++ b/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.
--- a/dom/media/MediaDecoderStateMachine.cpp
+++ b/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()) {
--- a/dom/media/MediaDecoderStateMachine.h
+++ b/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;