Backed out changesets b2eb913e58c9 and c678e1317fa0 (bug 1196112) for suspicion of causing bug 1197977.

dom/media/DecodedStream.cpp

@@ -105,7 +105,7 @@ UpdateStreamBlocking(MediaStream* aStream, bool aBlocking)
  */
 class DecodedStreamData {
 public:
-  DecodedStreamData(SourceMediaStream* aStream,
+  DecodedStreamData(SourceMediaStream* aStream, bool aPlaying,
                     MozPromiseHolder<GenericPromise>&& aPromise);
   ~DecodedStreamData();
   bool IsFinished() const;
@@ -142,7 +142,7 @@ public:
   bool mEOSVideoCompensation;
 };
 
-DecodedStreamData::DecodedStreamData(SourceMediaStream* aStream,
+DecodedStreamData::DecodedStreamData(SourceMediaStream* aStream, bool aPlaying,
                                      MozPromiseHolder<GenericPromise>&& aPromise)
   : mAudioFramesWritten(0)
   , mNextVideoTime(-1)
@@ -152,15 +152,17 @@ DecodedStreamData::DecodedStreamData(SourceMediaStream* aStream,
   , mHaveSentFinishAudio(false)
   , mHaveSentFinishVideo(false)
   , mStream(aStream)
-  , mPlaying(true)
+  , mPlaying(aPlaying)
   , mEOSVideoCompensation(false)
 {
   // DecodedStreamGraphListener will resolve this promise.
   mListener = new DecodedStreamGraphListener(mStream, Move(aPromise));
   mStream->AddListener(mListener);
 
-  // mPlaying is initially true because MDSM won't start playback until playing
-  // becomes true. This is consistent with the settings of AudioSink.
+  // Block the stream if we are not playing.
+  if (!aPlaying) {
+    UpdateStreamBlocking(mStream, true);
+  }
 }
 
 DecodedStreamData::~DecodedStreamData()
@@ -356,7 +358,7 @@ DecodedStream::DecodedStream(AbstractThread* aOwnerThread,
                              MediaQueue<MediaData>& aAudioQueue,
                              MediaQueue<MediaData>& aVideoQueue)
   : mOwnerThread(aOwnerThread)
-  , mShuttingDown(false)
+  , mMonitor("DecodedStream::mMonitor")
   , mPlaying(false)
   , mVolume(1.0)
   , mAudioQueue(aAudioQueue)
@@ -369,17 +371,11 @@ DecodedStream::~DecodedStream()
   MOZ_ASSERT(mStartTime.isNothing(), "playback should've ended.");
 }
 
-void
-DecodedStream::Shutdown()
-{
-  MOZ_ASSERT(NS_IsMainThread());
-  mShuttingDown = true;
-}
-
 nsRefPtr<GenericPromise>
 DecodedStream::StartPlayback(int64_t aStartTime, const MediaInfo& aInfo)
 {
   AssertOwnerThread();
+  ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
   MOZ_ASSERT(mStartTime.isNothing(), "playback already started.");
 
   mStartTime.emplace(aStartTime);
@@ -417,7 +413,7 @@ DecodedStream::StartPlayback(int64_t aStartTime, const MediaInfo& aInfo)
 void DecodedStream::StopPlayback()
 {
   AssertOwnerThread();
-
+  ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
   // Playback didn't even start at all.
   if (mStartTime.isNothing()) {
     return;
@@ -428,19 +424,12 @@ void DecodedStream::StopPlayback()
 
   // Clear mData immediately when this playback session ends so we won't
   // send data to the wrong stream in SendData() in next playback session.
-  DestroyData(Move(mData));
-}
-
-void
-DecodedStream::DestroyData(UniquePtr<DecodedStreamData> aData)
-{
-  AssertOwnerThread();
-
-  if (!aData) {
+  DecodedStreamData* data = mData.release();
+  // mData is not yet created on the main thread.
+  if (!data) {
     return;
   }
 
-  DecodedStreamData* data = aData.release();
   nsRefPtr<DecodedStream> self = this;
   nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction([=] () {
     self->mOutputStreamManager.Disconnect();
@@ -453,41 +442,35 @@ void
 DecodedStream::CreateData(MozPromiseHolder<GenericPromise>&& aPromise)
 {
   MOZ_ASSERT(NS_IsMainThread());
+  ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
+  MOZ_ASSERT(!mData, "Already created.");
 
   // No need to create a source stream when there are no output streams. This
   // happens when RemoveOutput() is called immediately after StartPlayback().
-  // Also we don't create a source stream when MDSM has begun shutdown.
-  if (!mOutputStreamManager.Graph() || mShuttingDown) {
+  // We also bail out when the playback session has ended. This happens when
+  // StopPlayback() is called immediately after StartPlayback().
+  if (!mOutputStreamManager.Graph() || mStartTime.isNothing()) {
     // Resolve the promise to indicate the end of playback.
     aPromise.Resolve(true, __func__);
     return;
   }
 
   auto source = mOutputStreamManager.Graph()->CreateSourceStream(nullptr);
-  auto data = new DecodedStreamData(source, Move(aPromise));
-  mOutputStreamManager.Connect(data->mStream);
+  mData.reset(new DecodedStreamData(source, mPlaying, Move(aPromise)));
+  mOutputStreamManager.Connect(mData->mStream);
 
-  class R : public nsRunnable {
-    typedef void(DecodedStream::*Method)(UniquePtr<DecodedStreamData>);
-  public:
-    R(DecodedStream* aThis, Method aMethod, DecodedStreamData* aData)
-      : mThis(aThis), mMethod(aMethod), mData(aData) {}
-    NS_IMETHOD Run() override
-    {
-      (mThis->*mMethod)(Move(mData));
-      return NS_OK;
+  // Start to send data to the stream immediately
+  nsRefPtr<DecodedStream> self = this;
+  nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction([=] () {
+    ReentrantMonitorAutoEnter mon(self->GetReentrantMonitor());
+    // Don't send data if playback has ended.
+    if (self->mStartTime.isSome()) {
+      self->SendData();
     }
-  private:
-    nsRefPtr<DecodedStream> mThis;
-    Method mMethod;
-    UniquePtr<DecodedStreamData> mData;
-  };
-
-  // Post a message to ensure |mData| is only updated on the worker thread.
-  // Note this must be done before MDSM's shutdown since dispatch could fail
-  // when the worker thread is shut down.
-  nsCOMPtr<nsIRunnable> r = new R(this, &DecodedStream::OnDataCreated, data);
-  mOwnerThread->Dispatch(r.forget());
+  });
+  // Don't assert success because the owner thread might have begun shutdown
+  // while we are still dealing with jobs on the main thread.
+  mOwnerThread->Dispatch(r.forget(), AbstractThread::DontAssertDispatchSuccess);
 }
 
 bool
@@ -496,22 +479,10 @@ DecodedStream::HasConsumers() const
   return !mOutputStreamManager.IsEmpty();
 }
 
-void
-DecodedStream::OnDataCreated(UniquePtr<DecodedStreamData> aData)
+ReentrantMonitor&
+DecodedStream::GetReentrantMonitor() const
 {
-  AssertOwnerThread();
-  MOZ_ASSERT(!mData, "Already created.");
-
-  // Start to send data to the stream immediately
-  if (mStartTime.isSome()) {
-    aData->SetPlaying(mPlaying);
-    mData = Move(aData);
-    SendData();
-    return;
-  }
-
-  // Playback has ended. Destroy aData which is not needed anymore.
-  DestroyData(Move(aData));
+  return mMonitor;
 }
 
 void
@@ -530,6 +501,7 @@ void
 DecodedStream::SetPlaying(bool aPlaying)
 {
   AssertOwnerThread();
+  ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
   mPlaying = aPlaying;
   if (mData) {
     mData->SetPlaying(aPlaying);
@@ -540,6 +512,7 @@ void
 DecodedStream::SetVolume(double aVolume)
 {
   AssertOwnerThread();
+  ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
   mVolume = aVolume;
 }
 
@@ -547,6 +520,7 @@ void
 DecodedStream::SetSameOrigin(bool aSameOrigin)
 {
   AssertOwnerThread();
+  ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
   mSameOrigin = aSameOrigin;
 }
 
@@ -554,6 +528,7 @@ void
 DecodedStream::InitTracks()
 {
   AssertOwnerThread();
+  GetReentrantMonitor().AssertCurrentThreadIn();
 
   if (mData->mStreamInitialized) {
     return;
@@ -634,6 +609,7 @@ void
 DecodedStream::SendAudio(double aVolume, bool aIsSameOrigin)
 {
   AssertOwnerThread();
+  GetReentrantMonitor().AssertCurrentThreadIn();
 
   if (!mInfo.HasAudio()) {
     return;
@@ -699,6 +675,7 @@ void
 DecodedStream::SendVideo(bool aIsSameOrigin)
 {
   AssertOwnerThread();
+  GetReentrantMonitor().AssertCurrentThreadIn();
 
   if (!mInfo.HasVideo()) {
     return;
@@ -777,6 +754,7 @@ void
 DecodedStream::AdvanceTracks()
 {
   AssertOwnerThread();
+  GetReentrantMonitor().AssertCurrentThreadIn();
 
   StreamTime endPosition = 0;
 
@@ -801,6 +779,7 @@ void
 DecodedStream::SendData()
 {
   AssertOwnerThread();
+  ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
   MOZ_ASSERT(mStartTime.isSome(), "Must be called after StartPlayback()");
 
   // Not yet created on the main thread. MDSM will try again later.
@@ -831,6 +810,7 @@ int64_t
 DecodedStream::AudioEndTime() const
 {
   AssertOwnerThread();
+  ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
   if (mStartTime.isSome() && mInfo.HasAudio() && mData) {
     CheckedInt64 t = mStartTime.ref() +
       FramesToUsecs(mData->mAudioFramesWritten, mInfo.mAudio.mRate);
@@ -845,6 +825,7 @@ int64_t
 DecodedStream::GetPosition() const
 {
   AssertOwnerThread();
+  ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
   // This is only called after MDSM starts playback. So mStartTime is
   // guaranteed to be something.
   MOZ_ASSERT(mStartTime.isSome());
@@ -855,6 +836,7 @@ bool
 DecodedStream::IsFinished() const
 {
   AssertOwnerThread();
+  ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
   return mData && mData->IsFinished();
 }
 
@@ -862,6 +844,7 @@ void
 DecodedStream::ConnectListener()
 {
   AssertOwnerThread();
+  ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
 
   mAudioPushListener = mAudioQueue.PushEvent().Connect(
     mOwnerThread, this, &DecodedStream::SendData);
@@ -877,6 +860,7 @@ void
 DecodedStream::DisconnectListener()
 {
   AssertOwnerThread();
+  ReentrantMonitorAutoEnter mon(GetReentrantMonitor());
 
   mAudioPushListener.Disconnect();
   mVideoPushListener.Disconnect();

dom/media/DecodedStream.h

@@ -106,8 +106,6 @@ public:
                 MediaQueue<MediaData>& aAudioQueue,
                 MediaQueue<MediaData>& aVideoQueue);
 
-  void Shutdown();
-
   // Mimic MDSM::StartAudioThread.
   // Must be called before any calls to SendData().
   //
@@ -134,9 +132,8 @@ protected:
   virtual ~DecodedStream();
 
 private:
+  ReentrantMonitor& GetReentrantMonitor() const;
   void CreateData(MozPromiseHolder<GenericPromise>&& aPromise);
-  void DestroyData(UniquePtr<DecodedStreamData> aData);
-  void OnDataCreated(UniquePtr<DecodedStreamData> aData);
   void InitTracks();
   void AdvanceTracks();
   void SendAudio(double aVolume, bool aIsSameOrigin);
@@ -152,18 +149,18 @@ private:
 
   const nsRefPtr<AbstractThread> mOwnerThread;
 
-  /*
-   * Main thread only members.
-   */
+  UniquePtr<DecodedStreamData> mData;
   // Data about MediaStreams that are being fed by the decoder.
   OutputStreamManager mOutputStreamManager;
-  // True if MDSM has begun shutdown.
-  bool mShuttingDown;
 
-  /*
-   * Worker thread only members.
-   */
-  UniquePtr<DecodedStreamData> mData;
+  // TODO: This is a temp solution to get rid of decoder monitor on the main
+  // thread in MDSM::AddOutputStream and MDSM::RecreateDecodedStream as
+  // required by bug 1146482. DecodedStream needs to release monitor before
+  // calling back into MDSM functions in order to prevent deadlocks.
+  //
+  // Please move all capture-stream related code from MDSM into DecodedStream
+  // and apply "dispatch + mirroring" to get rid of this monitor in the future.
+  mutable ReentrantMonitor mMonitor;
 
   bool mPlaying;
   double mVolume;

dom/media/MediaDecoderStateMachine.h

@@ -171,7 +171,6 @@ public:
 
   void DispatchShutdown()
   {
-    mDecodedStream->Shutdown();
     nsCOMPtr<nsIRunnable> runnable =
       NS_NewRunnableMethod(this, &MediaDecoderStateMachine::Shutdown);
     OwnerThread()->Dispatch(runnable.forget());