Bug 1404977 - Part 13 - Remove useless mutex, and assert why they were useless, in WebRTCAudioDataListeners. r=pehrsons

MozReview-Commit-ID: 2Mb5WZXbYgS

--HG--
extra : rebase_source : c4a7c8874919901eb11327cfb5f86d6b185be388

dom/media/MediaStreamGraph.cpp

@@ -878,6 +878,10 @@ MediaStreamGraphImpl::CloseAudioInputImpl(Maybe<CubebUtils::AudioDeviceID>& aID,
   MOZ_ASSERT(listeners);
   DebugOnly<bool> wasPresent = listeners->RemoveElement(aListener);
   MOZ_ASSERT(wasPresent);
+
+  // Breaks the cycle between the MSG and the listener.
+  aListener->Disconnect(this);
+
   if (!listeners->IsEmpty()) {
     // There is still a consumer for this audio input device
     return;
@@ -990,7 +994,7 @@ void MediaStreamGraphImpl::DeviceChangedImpl()
   nsTArray<RefPtr<AudioDataListener>>* listeners =
     mInputDeviceUsers.GetValue(mInputDeviceID);
   for (auto& listener : *listeners) {
-    listener->DeviceChanged();
+    listener->DeviceChanged(this);
   }
 }
 

dom/media/MediaStreamGraph.h

@@ -106,26 +106,31 @@ public:
    * cancellation.  This is not guaranteed to be in any particular size
    * chunks.
    */
-  virtual void NotifyOutputData(MediaStreamGraph* aGraph,
+  virtual void NotifyOutputData(MediaStreamGraphImpl* aGraph,
                                 AudioDataValue* aBuffer, size_t aFrames,
                                 TrackRate aRate, uint32_t aChannels) = 0;
   /**
    * Input data from a microphone (or other audio source.  This is not
    * guaranteed to be in any particular size chunks.
    */
-  virtual void NotifyInputData(MediaStreamGraph* aGraph,
+  virtual void NotifyInputData(MediaStreamGraphImpl* aGraph,
                                const AudioDataValue* aBuffer, size_t aFrames,
                                TrackRate aRate, uint32_t aChannels) = 0;
 
   /**
    * Number of audio input channels.
    */
-  virtual uint32_t InputChannelCount() = 0;
+  virtual uint32_t RequestedInputChannelCount(MediaStreamGraphImpl* aGraph) = 0;
 
   /**
    * Called when the underlying audio device has changed.
    */
-  virtual void DeviceChanged() = 0;
+  virtual void DeviceChanged(MediaStreamGraphImpl* aGraph) = 0;
+
+  /**
+   * Called when the underlying audio device is being closed.
+   */
+  virtual void Disconnect(MediaStreamGraphImpl* aGraph) = 0;
 };
 
 class AudioDataListener : public AudioDataListenerInterface {

dom/media/MediaStreamGraphImpl.h

@@ -484,7 +484,7 @@ public:
     MOZ_ASSERT(listeners);
     for (const auto& listener : *listeners) {
       maxInputChannels =
-        std::max(maxInputChannels, listener->InputChannelCount());
+        std::max(maxInputChannels, listener->RequestedInputChannelCount(this));
     }
     return maxInputChannels;
   }

dom/media/webrtc/MediaEngineWebRTC.h

@@ -160,6 +160,8 @@ private:
   bool mManualInvalidation;
 };
 
+// This class is instantiated on the MediaManager thread, and is then sent and
+// only ever access again on the MediaStreamGraph.
 class WebRTCAudioDataListener : public AudioDataListener
 {
 protected:
@@ -168,31 +170,29 @@ protected:
 
 public:
   explicit WebRTCAudioDataListener(MediaEngineWebRTCMicrophoneSource* aAudioSource)
-    : mMutex("WebRTCAudioDataListener::mMutex")
+    : mAudioSource(aAudioSource)
-    , mAudioSource(aAudioSource)
   {}
 
   // AudioDataListenerInterface methods
-  void NotifyOutputData(MediaStreamGraph* aGraph,
+  void NotifyOutputData(MediaStreamGraphImpl* aGraph,
                         AudioDataValue* aBuffer,
                         size_t aFrames,
                         TrackRate aRate,
                         uint32_t aChannels) override;
 
-  void NotifyInputData(MediaStreamGraph* aGraph,
+  void NotifyInputData(MediaStreamGraphImpl* aGraph,
                        const AudioDataValue* aBuffer,
                        size_t aFrames,
                        TrackRate aRate,
                        uint32_t aChannels) override;
 
-  uint32_t InputChannelCount() override;
+  uint32_t RequestedInputChannelCount(MediaStreamGraphImpl* aGraph) override;
 
-  void DeviceChanged() override;
+  void DeviceChanged(MediaStreamGraphImpl* aGraph) override;
 
-  void Shutdown();
+  void Disconnect(MediaStreamGraphImpl* aGraph) override;
 
 private:
-  Mutex mMutex;
   RefPtr<MediaEngineWebRTCMicrophoneSource> mAudioSource;
 };
 
@@ -247,20 +247,22 @@ public:
             const PrincipalHandle& aPrincipalHandle) override;
 
   // AudioDataListenerInterface methods
-  void NotifyOutputData(MediaStreamGraph* aGraph,
+  void NotifyOutputData(MediaStreamGraphImpl* aGraph,
                         AudioDataValue* aBuffer, size_t aFrames,
                         TrackRate aRate, uint32_t aChannels) override;
-  void NotifyInputData(MediaStreamGraph* aGraph,
+  void NotifyInputData(MediaStreamGraphImpl* aGraph,
                        const AudioDataValue* aBuffer, size_t aFrames,
                        TrackRate aRate, uint32_t aChannels) override;
 
-  void DeviceChanged() override;
+  void DeviceChanged(MediaStreamGraphImpl* aGraph) override;
 
   uint32_t RequestedInputChannelCount(MediaStreamGraphImpl* aGraph) override
   {
     return GetRequestedInputChannelCount(aGraph);
   }
 
+  void Disconnect(MediaStreamGraphImpl* aGraph) override;
+
   dom::MediaSourceEnum GetMediaSource() const override
   {
     return dom::MediaSourceEnum::Microphone;
@@ -364,7 +366,7 @@ private:
                      size_t aFrames,
                      uint32_t aChannels);
 
-  void PacketizeAndProcess(MediaStreamGraph* aGraph,
+  void PacketizeAndProcess(MediaStreamGraphImpl* aGraph,
                            const AudioDataValue* aBuffer,
                            size_t aFrames,
                            TrackRate aRate,
@@ -381,7 +383,9 @@ private:
   uint32_t GetRequestedInputChannelCount(MediaStreamGraphImpl* aGraphImpl);
   void SetRequestedInputChannelCount(uint32_t aRequestedInputChannelCount);
 
-  // Owning thread only.
+  // mListener is created on the MediaManager thread, and then sent to the MSG
+  // thread. On shutdown, we send this pointer to the MSG thread again, telling
+  // it to clean up.
   RefPtr<WebRTCAudioDataListener> mListener;
 
   // Can be shared on any thread.

dom/media/webrtc/MediaEngineWebRTCAudio.cpp

@@ -58,49 +58,52 @@ WebRTCAudioDataListener::NotifyOutputData(MediaStreamGraph* aGraph,
                                           TrackRate aRate,
                                           uint32_t aChannels)
 {
-  MutexAutoLock lock(mMutex);
+  MOZ_ASSERT(aGraph->CurrentDriver()->OnThread());
   if (mAudioSource) {
     mAudioSource->NotifyOutputData(aGraph, aBuffer, aFrames, aRate, aChannels);
   }
 }
 
 void
-WebRTCAudioDataListener::NotifyInputData(MediaStreamGraph* aGraph,
+WebRTCAudioDataListener::NotifyInputData(MediaStreamGraphImpl* aGraph,
                                          const AudioDataValue* aBuffer,
                                          size_t aFrames,
                                          TrackRate aRate,
                                          uint32_t aChannels)
 {
-  MutexAutoLock lock(mMutex);
+  MOZ_ASSERT(aGraph->CurrentDriver()->OnThread());
   if (mAudioSource) {
     mAudioSource->NotifyInputData(aGraph, aBuffer, aFrames, aRate, aChannels);
   }
 }
 
 void
-WebRTCAudioDataListener::DeviceChanged()
+WebRTCAudioDataListener::DeviceChanged(MediaStreamGraphImpl* aGraph)
 {
-  MutexAutoLock lock(mMutex);
+  MOZ_ASSERT(aGraph->CurrentDriver()->OnThread());
   if (mAudioSource) {
-    mAudioSource->DeviceChanged();
+    mAudioSource->DeviceChanged(aGraph);
   }
 }
 
 uint32_t
-WebRTCAudioDataListener::InputChannelCount()
+WebRTCAudioDataListener::RequestedInputChannelCount(MediaStreamGraphImpl* aGraph)
 {
-  MutexAutoLock lock(mMutex);
+  MOZ_ASSERT(aGraph->CurrentDriver()->OnThread());
   if (mAudioSource) {
-    return mAudioSource->InputChannelCount();
+    return mAudioSource->RequestedInputChannelCount(aGraph);
   }
   return 0;
 }
 
 void
-WebRTCAudioDataListener::Shutdown()
+WebRTCAudioDataListener::Disconnect(MediaStreamGraphImpl* aGraph)
 {
-  MutexAutoLock lock(mMutex);
+  MOZ_ASSERT(aGraph->CurrentDriver()->OnThread());
-  mAudioSource = nullptr;
+  if (mAudioSource) {
+    mAudioSource->Disconnect(aGraph);
+    mAudioSource = nullptr;
+  }
 }
 
 /**
@@ -529,6 +532,9 @@ MediaEngineWebRTCMicrophoneSource::SetPassThrough(bool aPassThrough)
 uint32_t
 MediaEngineWebRTCMicrophoneSource::GetRequestedInputChannelCount(MediaStreamGraphImpl* aGraphImpl)
 {
+  MOZ_ASSERT(aGraphImpl->CurrentDriver()->OnThread(),
+             "Wrong calling pattern, don't call this before ::SetTrack.");
+
   if (mState == kReleased) {
     // This source has been released, and is waiting for collection. Simply
     // return 0, this source won't contribute to the channel count decision.
@@ -544,6 +550,9 @@ MediaEngineWebRTCMicrophoneSource::SetRequestedInputChannelCount(
   uint32_t aRequestedInputChannelCount)
 {
   MutexAutoLock lock(mMutex);
+
+  MOZ_ASSERT(mAllocations.Length() <= 1);
+
   if (mAllocations.IsEmpty()) {
       return;
   }
@@ -561,6 +570,12 @@ MediaEngineWebRTCMicrophoneSource::ApplySettings(const MediaEnginePrefs& aPrefs,
 {
   AssertIsOnOwningThread();
   MOZ_DIAGNOSTIC_ASSERT(aGraph);
+#ifdef DEBUG
+  {
+    MutexAutoLock lock(mMutex);
+    MOZ_ASSERT(mAllocations.Length() <= 1);
+  }
+#endif
 
   RefPtr<MediaEngineWebRTCMicrophoneSource> that = this;
   NS_DispatchToMainThread(media::NewRunnableFrom([that, graph = std::move(aGraph), aPrefs]() mutable {
@@ -616,6 +631,12 @@ MediaEngineWebRTCMicrophoneSource::Allocate(const dom::MediaTrackConstraints &aC
   auto handle = MakeRefPtr<AllocationHandle>(aConstraints, aPrincipalInfo,
                                              aDeviceId);
 
+#ifdef DEBUG
+  {
+    MutexAutoLock lock(mMutex);
+    MOZ_ASSERT(mAllocations.Length() <= 1);
+  }
+#endif
   LOG(("Mic source %p allocation %p Allocate()", this, handle.get()));
 
   nsresult rv = ReevaluateAllocation(handle, nullptr, aPrefs, aDeviceId,
@@ -626,6 +647,7 @@ MediaEngineWebRTCMicrophoneSource::Allocate(const dom::MediaTrackConstraints &aC
 
   {
     MutexAutoLock lock(mMutex);
+    MOZ_ASSERT(mAllocations.IsEmpty(), "Only allocate once.");
     mAllocations.AppendElement(Allocation(handle));
   }
 
@@ -638,6 +660,8 @@ MediaEngineWebRTCMicrophoneSource::Deallocate(const RefPtr<const AllocationHandl
 {
   AssertIsOnOwningThread();
 
+  MOZ_ASSERT(mState == kStopped);
+
   size_t i = mAllocations.IndexOf(aHandle, 0, AllocationHandleComparator());
   MOZ_DIAGNOSTIC_ASSERT(i != mAllocations.NoIndex);
   MOZ_DIAGNOSTIC_ASSERT(!mAllocations[i].mEnabled,
@@ -649,6 +673,7 @@ MediaEngineWebRTCMicrophoneSource::Deallocate(const RefPtr<const AllocationHandl
 
   {
     MutexAutoLock lock(mMutex);
+    MOZ_ASSERT(mAllocations.Length() == 1, "Only allocate once.");
     mAllocations.RemoveElementAt(i);
   }
 
@@ -682,6 +707,8 @@ MediaEngineWebRTCMicrophoneSource::SetTrack(const RefPtr<const AllocationHandle>
     return NS_ERROR_NOT_AVAILABLE;
   }
 
+  MOZ_ASSERT(mAllocations.Length() == 1, "Only allocate once.");
+
   size_t i = mAllocations.IndexOf(aHandle, 0, AllocationHandleComparator());
   MOZ_DIAGNOSTIC_ASSERT(i != mAllocations.NoIndex);
   MOZ_ASSERT(!mAllocations[i].mStream);
@@ -766,6 +793,7 @@ MediaEngineWebRTCMicrophoneSource::Start(const RefPtr<const AllocationHandle>& a
 nsresult
 MediaEngineWebRTCMicrophoneSource::Stop(const RefPtr<const AllocationHandle>& aHandle)
 {
+  MOZ_ASSERT(mAllocations.Length() <= 1);
   AssertIsOnOwningThread();
 
   LOG(("Mic source %p allocation %p Stop()", this, aHandle.get()));
@@ -774,6 +802,7 @@ MediaEngineWebRTCMicrophoneSource::Stop(const RefPtr<const AllocationHandle>& aH
   MOZ_DIAGNOSTIC_ASSERT(i != mAllocations.NoIndex,
                         "Cannot stop track that we don't know about");
   Allocation& allocation = mAllocations[i];
+  MOZ_ASSERT(allocation.mStream, "SetTrack must have been called before ::Stop");
 
   if (!allocation.mEnabled) {
     // Already stopped - this is allowed
@@ -788,6 +817,7 @@ MediaEngineWebRTCMicrophoneSource::Stop(const RefPtr<const AllocationHandle>& aH
     CubebUtils::AudioDeviceID deviceID = mDeviceInfo->DeviceID();
     Maybe<CubebUtils::AudioDeviceID> id = Some(deviceID);
     allocation.mStream->CloseAudioInput(id, mListener);
+    mListener = nullptr;
 
     if (HasEnabledTrack()) {
       // Another track is keeping us from stopping
@@ -798,12 +828,6 @@ MediaEngineWebRTCMicrophoneSource::Stop(const RefPtr<const AllocationHandle>& aH
     mState = kStopped;
   }
 
-  if (mListener) {
-    // breaks a cycle, since the WebRTCAudioDataListener has a RefPtr to us
-    mListener->Shutdown();
-    mListener = nullptr;
-  }
-
   return NS_OK;
 }
 
@@ -884,12 +908,14 @@ MediaEngineWebRTCMicrophoneSource::Pull(const RefPtr<const AllocationHandle>& aH
 }
 
 void
-MediaEngineWebRTCMicrophoneSource::NotifyOutputData(MediaStreamGraph* aGraph,
+MediaEngineWebRTCMicrophoneSource::NotifyOutputData(MediaStreamGraphImpl* aGraph,
                                                     AudioDataValue* aBuffer,
                                                     size_t aFrames,
                                                     TrackRate aRate,
                                                     uint32_t aChannels)
 {
+  MOZ_ASSERT(aGraph->CurrentDriver()->OnThread());
+
   if (!mPacketizerOutput ||
       mPacketizerOutput->PacketSize() != aRate/100u ||
       mPacketizerOutput->Channels() != aChannels) {
@@ -977,7 +1003,7 @@ MediaEngineWebRTCMicrophoneSource::NotifyOutputData(MediaStreamGraph* aGraph,
 
 // Only called if we're not in passthrough mode
 void
-MediaEngineWebRTCMicrophoneSource::PacketizeAndProcess(MediaStreamGraph* aGraph,
+MediaEngineWebRTCMicrophoneSource::PacketizeAndProcess(MediaStreamGraphImpl* aGraph,
                                                        const AudioDataValue* aBuffer,
                                                        size_t aFrames,
                                                        TrackRate aRate,
@@ -1202,12 +1228,13 @@ MediaEngineWebRTCMicrophoneSource::InsertInGraph(const T* aBuffer,
 // Called back on GraphDriver thread!
 // Note this can be called back after ::Shutdown()
 void
-MediaEngineWebRTCMicrophoneSource::NotifyInputData(MediaStreamGraph* aGraph,
+MediaEngineWebRTCMicrophoneSource::NotifyInputData(MediaStreamGraphImpl* aGraph,
                                                    const AudioDataValue* aBuffer,
                                                    size_t aFrames,
                                                    TrackRate aRate,
                                                    uint32_t aChannels)
 {
+  MOZ_ASSERT(aGraph->CurrentDriver()->OnThread());
   TRACE_AUDIO_CALLBACK();
 
   {
@@ -1253,14 +1280,23 @@ do {                                                                \
 }  while(0)
 
 void
-MediaEngineWebRTCMicrophoneSource::DeviceChanged()
+MediaEngineWebRTCMicrophoneSource::DeviceChanged(MediaStreamGraphImpl* aGraph)
 {
+  MOZ_ASSERT(aGraph->CurrentDriver()->OnThread());
   // Reset some processing
   ResetProcessingIfNeeded(gain_control);
   ResetProcessingIfNeeded(echo_cancellation);
   ResetProcessingIfNeeded(noise_suppression);
 }
 
+void
+MediaEngineWebRTCMicrophoneSource::Disconnect(MediaStreamGraphImpl* aGraph)
+{
+  // This method is just for asserts.
+  MOZ_ASSERT(aGraph->CurrentDriver()->OnThread());
+  MOZ_ASSERT(!mListener);
+}
+
 void
 MediaEngineWebRTCMicrophoneSource::Shutdown()
 {