Bug 1230004. Part 2 - have MDSM::BeginShutdown return a promise and remove MDSM::mDecoder. r=cpearce.

dom/media/MediaDecoder.cpp

@@ -615,13 +615,16 @@ MediaDecoder::Shutdown()
   // necessary to unblock the state machine thread if it's blocked, so
   // the asynchronous shutdown in nsDestroyStateMachine won't deadlock.
   if (mDecoderStateMachine) {
-    mDecoderStateMachine->DispatchShutdown();
     mTimedMetadataListener.Disconnect();
     mMetadataLoadedListener.Disconnect();
     mFirstFrameLoadedListener.Disconnect();
     mOnPlaybackEvent.Disconnect();
     mOnSeekingStart.Disconnect();
     mOnMediaNotSeekable.Disconnect();
+
+    mDecoderStateMachine->BeginShutdown()->Then(
+      AbstractThread::MainThread(), __func__, this,
+      &MediaDecoder::FinishShutdown, &MediaDecoder::FinishShutdown);
   }
 
   // Force any outstanding seek and byterange requests to complete
@@ -668,6 +671,14 @@ MediaDecoder::OnPlaybackEvent(MediaEventType aEvent)
   }
 }
 
+void
+MediaDecoder::FinishShutdown()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  mDecoderStateMachine->BreakCycles();
+  SetStateMachine(nullptr);
+}
+
 MediaResourceCallback*
 MediaDecoder::GetResourceCallback() const
 {
@@ -1539,13 +1550,6 @@ MediaDecoder::GetStateMachine() const {
   return mDecoderStateMachine;
 }
 
-// Drop reference to state machine.  Only called during shutdown dance.
-void
-MediaDecoder::BreakCycles() {
-  MOZ_ASSERT(NS_IsMainThread());
-  SetStateMachine(nullptr);
-}
-
 void
 MediaDecoder::FireTimeUpdate()
 {

dom/media/MediaDecoder.h

@@ -636,9 +636,6 @@ private:
   // position.
   int64_t GetDownloadPosition();
 
-  // Drop reference to state machine.  Only called during shutdown dance.
-  virtual void BreakCycles();
-
   // Notifies the element that decoding has failed.
   void DecodeError();
 
@@ -815,6 +812,8 @@ private:
     SetMediaSeekable(false);
   }
 
+  void FinishShutdown();
+
   MediaEventProducer<void> mDataArrivedEvent;
 
   // The state machine object for handling the decoding. It is safe to

dom/media/MediaDecoderStateMachine.cpp

@@ -199,7 +199,6 @@ static void InitVideoQueuePrefs() {
 MediaDecoderStateMachine::MediaDecoderStateMachine(MediaDecoder* aDecoder,
                                                    MediaDecoderReader* aReader,
                                                    bool aRealTime) :
-  mDecoder(aDecoder),
   mDecoderID(aDecoder),
   mFrameStats(&aDecoder->GetFrameStatistics()),
   mVideoFrameContainer(aDecoder->GetVideoFrameContainer()),
@@ -1304,7 +1303,8 @@ MediaDecoderStateMachine::SetDormant(bool aDormant)
   }
 }
 
-void MediaDecoderStateMachine::Shutdown()
+RefPtr<ShutdownPromise>
+MediaDecoderStateMachine::Shutdown()
 {
   MOZ_ASSERT(OnTaskQueue());
 
@@ -1335,13 +1335,16 @@ void MediaDecoderStateMachine::Shutdown()
     mStartTimeRendezvous->Destroy();
   }
 
+  DECODER_LOG("Shutdown started");
+
   // Put a task in the decode queue to shutdown the reader.
   // the queue to spin down.
-  InvokeAsync(DecodeTaskQueue(), mReader.get(), __func__, &MediaDecoderReader::Shutdown)
+  return InvokeAsync(DecodeTaskQueue(), mReader.get(), __func__,
+                     &MediaDecoderReader::Shutdown)
     ->Then(OwnerThread(), __func__, this,
            &MediaDecoderStateMachine::FinishShutdown,
-           &MediaDecoderStateMachine::FinishShutdown);
-  DECODER_LOG("Shutdown started");
+           &MediaDecoderStateMachine::FinishShutdown)
+    ->CompletionPromise();
 }
 
 void MediaDecoderStateMachine::StartDecoding()
@@ -2159,40 +2162,15 @@ MediaDecoderStateMachine::SeekCompleted()
   }
 }
 
-class DecoderDisposer
-{
-public:
-  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecoderDisposer)
-  DecoderDisposer(MediaDecoder* aDecoder, MediaDecoderStateMachine* aStateMachine)
-    : mDecoder(aDecoder), mStateMachine(aStateMachine) {}
-
-  void OnTaskQueueShutdown()
-  {
-    MOZ_ASSERT(NS_IsMainThread());
-    MOZ_ASSERT(mStateMachine);
-    MOZ_ASSERT(mDecoder);
-    mStateMachine->BreakCycles();
-    mDecoder->BreakCycles();
-    mStateMachine = nullptr;
-    mDecoder = nullptr;
-  }
-
-private:
-  virtual ~DecoderDisposer() {}
-  RefPtr<MediaDecoder> mDecoder;
-  RefPtr<MediaDecoderStateMachine> mStateMachine;
-};
-
-void
-MediaDecoderStateMachine::DispatchShutdown()
+RefPtr<ShutdownPromise>
+MediaDecoderStateMachine::BeginShutdown()
 {
   mStreamSink->BeginShutdown();
-  nsCOMPtr<nsIRunnable> runnable =
-    NS_NewRunnableMethod(this, &MediaDecoderStateMachine::Shutdown);
-  OwnerThread()->Dispatch(runnable.forget());
+  return InvokeAsync(OwnerThread(), this, __func__,
+                     &MediaDecoderStateMachine::Shutdown);
 }
 
-void
+RefPtr<ShutdownPromise>
 MediaDecoderStateMachine::FinishShutdown()
 {
   MOZ_ASSERT(OnTaskQueue());
@@ -2233,25 +2211,8 @@ MediaDecoderStateMachine::FinishShutdown()
 
   MOZ_ASSERT(mState == DECODER_STATE_SHUTDOWN,
              "How did we escape from the shutdown state?");
-  // We must daisy-chain these events to destroy the decoder. We must
-  // destroy the decoder on the main thread, but we can't destroy the
-  // decoder while this thread holds the decoder monitor. We can't
-  // dispatch an event to the main thread to destroy the decoder from
-  // here, as the event may run before the dispatch returns, and we
-  // hold the decoder monitor here. We also want to guarantee that the
-  // state machine is destroyed on the main thread, and so the
-  // event runner running this function (which holds a reference to the
-  // state machine) needs to finish and be released in order to allow
-  // that. So we dispatch an event to run after this event runner has
-  // finished and released its monitor/references. That event then will
-  // dispatch an event to the main thread to release the decoder and
-  // state machine.
   DECODER_LOG("Shutting down state machine task queue");
-  RefPtr<DecoderDisposer> disposer = new DecoderDisposer(mDecoder, this);
-  OwnerThread()->BeginShutdown()->Then(AbstractThread::MainThread(), __func__,
-                                       disposer.get(),
-                                       &DecoderDisposer::OnTaskQueueShutdown,
-                                       &DecoderDisposer::OnTaskQueueShutdown);
+  return OwnerThread()->BeginShutdown();
 }
 
 nsresult MediaDecoderStateMachine::RunStateMachine()

dom/media/MediaDecoderStateMachine.h

@@ -167,7 +167,7 @@ public:
   // Set/Unset dormant state.
   void DispatchSetDormant(bool aDormant);
 
-  void DispatchShutdown();
+  RefPtr<ShutdownPromise> BeginShutdown();
 
   void DispatchStartBuffering()
   {
@@ -218,14 +218,13 @@ public:
     OwnerThread()->Dispatch(r.forget());
   }
 
-  // Drop reference to decoder.  Only called during shutdown dance.
+  // Drop reference to mReader and mResource. Only called during shutdown dance.
   void BreakCycles() {
     MOZ_ASSERT(NS_IsMainThread());
     if (mReader) {
       mReader->BreakCycles();
     }
     mResource = nullptr;
-    mDecoder = nullptr;
   }
 
   TimedMetadataEventSource& TimedMetadataEvent() {
@@ -284,9 +283,9 @@ private:
 
   RefPtr<MediaDecoder::SeekPromise> Seek(SeekTarget aTarget);
 
-  void Shutdown();
+  RefPtr<ShutdownPromise> Shutdown();
 
-  void FinishShutdown();
+  RefPtr<ShutdownPromise> FinishShutdown();
 
   // Update the playback position. This can result in a timeupdate event
   // and an invalidate of the frame being dispatched asynchronously if
@@ -666,16 +665,6 @@ private:
 
   void AdjustAudioThresholds();
 
-  // The decoder object that created this state machine. The state machine
-  // holds a strong reference to the decoder to ensure that the decoder stays
-  // alive once media element has started the decoder shutdown process, and has
-  // dropped its reference to the decoder. This enables the state machine to
-  // keep using the decoder's monitor until the state machine has finished
-  // shutting down, without fear of the monitor being destroyed. After
-  // shutting down, the state machine will then release this reference,
-  // causing the decoder to be destroyed. This is accessed on the decode,
-  // state machine, audio and main threads.
-  RefPtr<MediaDecoder> mDecoder;
   void* const mDecoderID;
   const RefPtr<FrameStatistics> mFrameStats;
   const RefPtr<VideoFrameContainer> mVideoFrameContainer;