Bug 1541290 - Close audio input from main thread to ensure that the message is not executed on a closed graph. r=pehrsons

CloseAudioInut method posts a message, to the graph thread, in order to close the input asynchonously. When CloseAudioInput method was being executed from a thread other than the main thread, a runnable would be posted to main thread in order to post the async message from there. That was a risky path because when the graph was shutting down there were no guarantee that the close-input message would reach the graph thread before destroy takes place. By limiting CloseAudioInput to main thread it is ensured that the close-input message will be executed before destroy.

Differential Revision: https://phabricator.services.mozilla.com/D27751

--HG--
extra : moz-landing-system : lando

dom/media/MediaStreamGraph.cpp

@@ -761,14 +761,7 @@ void MediaStreamGraphImpl::OpenAudioInputImpl(CubebUtils::AudioDeviceID aID,
 
 nsresult MediaStreamGraphImpl::OpenAudioInput(CubebUtils::AudioDeviceID aID,
                                               AudioDataListener* aListener) {
-  // So, so, so annoying.  Can't AppendMessage except on Mainthread
-  if (!NS_IsMainThread()) {
-    RefPtr<nsIRunnable> runnable =
-        WrapRunnable(this, &MediaStreamGraphImpl::OpenAudioInput, aID,
-                     RefPtr<AudioDataListener>(aListener));
-    mAbstractMainThread->Dispatch(runnable.forget());
-    return NS_OK;
-  }
+  MOZ_ASSERT(NS_IsMainThread());
   class Message : public ControlMessage {
    public:
     Message(MediaStreamGraphImpl* aGraph, CubebUtils::AudioDeviceID aID,
@@ -845,14 +838,7 @@ void MediaStreamGraphImpl::CloseAudioInputImpl(
 
 void MediaStreamGraphImpl::CloseAudioInput(
     Maybe<CubebUtils::AudioDeviceID>& aID, AudioDataListener* aListener) {
-  // So, so, so annoying.  Can't AppendMessage except on Mainthread
-  if (!NS_IsMainThread()) {
-    RefPtr<nsIRunnable> runnable =
-        WrapRunnable(this, &MediaStreamGraphImpl::CloseAudioInput, aID,
-                     RefPtr<AudioDataListener>(aListener));
-    mAbstractMainThread->Dispatch(runnable.forget());
-    return;
-  }
+  MOZ_ASSERT(NS_IsMainThread());
   class Message : public ControlMessage {
    public:
     Message(MediaStreamGraphImpl* aGraph, Maybe<CubebUtils::AudioDeviceID>& aID,
@@ -2421,25 +2407,32 @@ SourceMediaStream::SourceMediaStream()
 
 nsresult SourceMediaStream::OpenAudioInput(CubebUtils::AudioDeviceID aID,
                                            AudioDataListener* aListener) {
+  MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(GraphImpl());
+  MOZ_ASSERT(!mInputListener);
   mInputListener = aListener;
   return GraphImpl()->OpenAudioInput(aID, aListener);
 }
 
-void SourceMediaStream::CloseAudioInput(Maybe<CubebUtils::AudioDeviceID>& aID,
-                                        AudioDataListener* aListener) {
-  MOZ_ASSERT(mInputListener == aListener);
-  // Destroy() may have run already and cleared this
-  if (GraphImpl() && mInputListener) {
-    GraphImpl()->CloseAudioInput(aID, aListener);
+void SourceMediaStream::CloseAudioInput(Maybe<CubebUtils::AudioDeviceID>& aID) {
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(GraphImpl());
+  if (!mInputListener) {
+    return;
   }
+  GraphImpl()->CloseAudioInput(aID, mInputListener);
   mInputListener = nullptr;
 }
 
-void SourceMediaStream::DestroyImpl() {
+void SourceMediaStream::Destroy() {
+  MOZ_ASSERT(NS_IsMainThread());
   Maybe<CubebUtils::AudioDeviceID> id = Nothing();
-  CloseAudioInput(id, mInputListener);
+  CloseAudioInput(id);
 
+  MediaStream::Destroy();
+}
+
+void SourceMediaStream::DestroyImpl() {
   GraphImpl()->AssertOnGraphThreadOrNotRunning();
   for (int32_t i = mConsumers.Length() - 1; i >= 0; --i) {
     // Disconnect before we come under mMutex's lock since it can call back

dom/media/MediaStreamGraph.h

@@ -655,14 +655,14 @@ class SourceMediaStream : public MediaStream {
 
   // Users of audio inputs go through the stream so it can track when the
   // last stream referencing an input goes away, so it can close the cubeb
-  // input.  Also note: callable on any thread (though it bounces through
-  // MainThread to set the command if needed).
+  // input. Main thread only.
   nsresult OpenAudioInput(CubebUtils::AudioDeviceID aID,
                           AudioDataListener* aListener);
-  // Note: also implied when Destroy() happens
-  void CloseAudioInput(Maybe<CubebUtils::AudioDeviceID>& aID,
-                       AudioDataListener* aListener);
+  // Main thread only.
+  void CloseAudioInput(Maybe<CubebUtils::AudioDeviceID>& aID);
 
+  // Main thread only.
+  void Destroy() override;
   // MediaStreamGraph thread only
   void DestroyImpl() override;
 

dom/media/webrtc/MediaEngineWebRTCAudio.cpp

@@ -601,7 +601,7 @@ nsresult MediaEngineWebRTCMicrophoneSource::Stop() {
             that->mInputProcessing, StartStopMessage::Stop));
         CubebUtils::AudioDeviceID deviceID = that->mDeviceInfo->DeviceID();
         Maybe<CubebUtils::AudioDeviceID> id = Some(deviceID);
-        stream->CloseAudioInput(id, that->mInputProcessing);
+        stream->CloseAudioInput(id);
       }));
 
   MOZ_ASSERT(mState == kStarted, "Should be started when stopping");