Bug 1103188 - Always call MediaManager::NotifyFinished/NotifyRemoved on main thread. r=jib a=lizzard
authorAndreas Pehrson <pehrsons@gmail.com>
Wed, 30 Sep 2015 00:38:57 +0800
changeset 296739 5ca6857c26e5
parent 296738 98d9576c7d13
child 296740 807e612c17ef
push id5315
push userkwierso@gmail.com
push date2015-11-16 19:39 +0000
treeherdermozilla-beta@5ca6857c26e5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjib, lizzard
bugs1103188
milestone43.0
Bug 1103188 - Always call MediaManager::NotifyFinished/NotifyRemoved on main thread. r=jib a=lizzard
dom/media/MediaManager.cpp
dom/media/MediaManager.h
--- a/dom/media/MediaManager.cpp
+++ b/dom/media/MediaManager.cpp
@@ -2391,16 +2391,17 @@ MediaManager::GetBackend(uint64_t aWindo
 }
 
 static void
 StopSharingCallback(MediaManager *aThis,
                     uint64_t aWindowID,
                     StreamListeners *aListeners,
                     void *aData)
 {
+  MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread");
   if (aListeners) {
     auto length = aListeners->Length();
     for (size_t i = 0; i < length; ++i) {
       GetUserMediaCallbackMediaStreamListener *listener = aListeners->ElementAt(i);
 
       if (listener->Stream()) { // aka HasBeenActivate()ed
         listener->Invalidate();
       }
@@ -3212,16 +3213,17 @@ GetUserMediaCallbackMediaStreamListener:
   return p.forget();
 }
 
 // Stop backend for track
 
 void
 GetUserMediaCallbackMediaStreamListener::StopTrack(TrackID aTrackID, bool aIsAudio)
 {
+  MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread");
   if (((aIsAudio && mAudioDevice) ||
        (!aIsAudio && mVideoDevice)) && !mStopped)
   {
     // XXX to support multiple tracks of a type in a stream, this should key off
     // the TrackID and not just the type
     AudioDevice* audioDevice = aIsAudio  && !mAudioStopped ? mAudioDevice.get() : nullptr;
     VideoDevice* videoDevice = !aIsAudio && !mVideoStopped ? mVideoDevice.get() : nullptr;
     MediaManager::PostTask(FROM_HERE,
@@ -3232,50 +3234,50 @@ GetUserMediaCallbackMediaStreamListener:
     mAudioStopped = !!audioDevice;
     mVideoStopped = !!videoDevice;
   } else {
     LOG(("gUM track %d ended, but we don't have type %s",
          aTrackID, aIsAudio ? "audio" : "video"));
   }
 }
 
-// Called from the MediaStreamGraph thread
 void
-GetUserMediaCallbackMediaStreamListener::NotifyFinished(MediaStreamGraph* aGraph)
+GetUserMediaCallbackMediaStreamListener::NotifyFinished()
 {
+  MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread");
   mFinished = true;
   Invalidate(); // we know it's been activated
-  NS_DispatchToMainThread(do_AddRef(new GetUserMediaListenerRemove(mWindowID, this)));
+
+  nsRefPtr<MediaManager> manager(MediaManager::GetInstance());
+  manager->RemoveFromWindowList(mWindowID, this);
 }
 
 // Called from the MediaStreamGraph thread
 void
 GetUserMediaCallbackMediaStreamListener::NotifyDirectListeners(MediaStreamGraph* aGraph,
                                                                bool aHasListeners)
 {
   MediaManager::PostTask(FROM_HERE,
     new MediaOperationTask(MEDIA_DIRECT_LISTENERS,
                            this, nullptr, nullptr,
                            mAudioDevice, mVideoDevice,
                            aHasListeners, mWindowID, nullptr));
 }
 
-// Called from the MediaStreamGraph thread
 // this can be in response to our own RemoveListener() (via ::Remove()), or
 // because the DOM GC'd the DOMLocalMediaStream/etc we're attached to.
 void
-GetUserMediaCallbackMediaStreamListener::NotifyRemoved(MediaStreamGraph* aGraph)
+GetUserMediaCallbackMediaStreamListener::NotifyRemoved()
 {
-  {
-    MutexAutoLock lock(mLock); // protect access to mRemoved
-    MM_LOG(("Listener removed by DOM Destroy(), mFinished = %d", (int) mFinished));
-    mRemoved = true;
-  }
+  MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread");
+  MM_LOG(("Listener removed by DOM Destroy(), mFinished = %d", (int) mFinished));
+  mRemoved = true;
+
   if (!mFinished) {
-    NotifyFinished(aGraph);
+    NotifyFinished();
   }
 }
 
 NS_IMETHODIMP
 GetUserMediaNotificationEvent::Run()
 {
   NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
   // Make sure mStream is cleared and our reference to the DOMMediaStream
--- a/dom/media/MediaManager.h
+++ b/dom/media/MediaManager.h
@@ -122,21 +122,20 @@ class GetUserMediaCallbackMediaStreamLis
 {
 public:
   // Create in an inactive state
   GetUserMediaCallbackMediaStreamListener(base::Thread *aThread,
     uint64_t aWindowID)
     : mMediaThread(aThread)
     , mWindowID(aWindowID)
     , mStopped(false)
+    , mFinished(false)
+    , mRemoved(false)
     , mAudioStopped(false)
-    , mVideoStopped(false)
-    , mFinished(false)
-    , mLock("mozilla::GUMCMSL")
-    , mRemoved(false) {}
+    , mVideoStopped(false) {}
 
   ~GetUserMediaCallbackMediaStreamListener()
   {
     unused << mMediaThread;
     // It's OK to release mStream on any thread; they have thread-safe
     // refcounts.
   }
 
@@ -233,101 +232,106 @@ public:
               int32_t aPlayoutDelay);
 
   void
   Remove()
   {
     NS_ASSERTION(NS_IsMainThread(), "Only call on main thread");
     // allow calling even if inactive (!mStream) for easier cleanup
     // Caller holds strong reference to us, so no death grip required
-    MutexAutoLock lock(mLock); // protect access to mRemoved
     if (mStream && !mRemoved) {
       MM_LOG(("Listener removed on purpose, mFinished = %d", (int) mFinished));
       mRemoved = true; // RemoveListener is async, avoid races
       // If it's destroyed, don't call - listener will be removed and we'll be notified!
       if (!mStream->IsDestroyed()) {
         mStream->RemoveListener(this);
       }
     }
   }
 
   // Proxy NotifyPull() to sources
-  virtual void
+  void
   NotifyPull(MediaStreamGraph* aGraph, StreamTime aDesiredTime) override
   {
     // Currently audio sources ignore NotifyPull, but they could
     // watch it especially for fake audio.
     if (mAudioDevice) {
       mAudioDevice->GetSource()->NotifyPull(aGraph, mStream, kAudioTrack,
                                             aDesiredTime);
     }
     if (mVideoDevice) {
       mVideoDevice->GetSource()->NotifyPull(aGraph, mStream, kVideoTrack,
                                             aDesiredTime);
     }
   }
 
-  virtual void
+  void
   NotifyEvent(MediaStreamGraph* aGraph,
               MediaStreamListener::MediaStreamGraphEvent aEvent) override
   {
     switch (aEvent) {
       case EVENT_FINISHED:
-        NotifyFinished(aGraph);
+        NS_DispatchToMainThread(
+          NS_NewRunnableMethod(this, &GetUserMediaCallbackMediaStreamListener::NotifyFinished));
         break;
       case EVENT_REMOVED:
-        NotifyRemoved(aGraph);
+        NS_DispatchToMainThread(
+          NS_NewRunnableMethod(this, &GetUserMediaCallbackMediaStreamListener::NotifyRemoved));
         break;
       case EVENT_HAS_DIRECT_LISTENERS:
         NotifyDirectListeners(aGraph, true);
         break;
       case EVENT_HAS_NO_DIRECT_LISTENERS:
         NotifyDirectListeners(aGraph, false);
         break;
       default:
         break;
     }
   }
 
-  virtual void
-  NotifyFinished(MediaStreamGraph* aGraph);
+  void
+  NotifyFinished();
 
-  virtual void
-  NotifyRemoved(MediaStreamGraph* aGraph);
+  void
+  NotifyRemoved();
 
-  virtual void
+  void
   NotifyDirectListeners(MediaStreamGraph* aGraph, bool aHasListeners);
 
 private:
   // Set at construction
   base::Thread* mMediaThread;
   uint64_t mWindowID;
 
-  bool mStopped; // MainThread only
+  // true after this listener has sent MEDIA_STOP. MainThread only.
+  bool mStopped;
+
+  // true after the stream this listener is listening to has finished in the
+  // MediaStreamGraph. MainThread only.
+  bool mFinished;
+
+  // true after this listener has been removed from its MediaStream.
+  // MainThread only.
+  bool mRemoved;
 
   // true if we have sent MEDIA_STOP or MEDIA_STOP_TRACK for mAudioDevice.
   // MainThread only.
   bool mAudioStopped;
 
   // true if we have sent MEDIA_STOP or MEDIA_STOP_TRACK for mAudioDevice.
   // MainThread only.
   bool mVideoStopped;
 
   // Set at Activate on MainThread
 
   // Accessed from MediaStreamGraph thread, MediaManager thread, and MainThread
   // No locking needed as they're only addrefed except on the MediaManager thread
   nsRefPtr<AudioDevice> mAudioDevice; // threadsafe refcnt
   nsRefPtr<VideoDevice> mVideoDevice; // threadsafe refcnt
   nsRefPtr<SourceMediaStream> mStream; // threadsafe refcnt
-  bool mFinished;
-
-  // Accessed from MainThread and MSG thread
-  Mutex mLock; // protects mRemoved access from MainThread
-  bool mRemoved;
 };
 
 class GetUserMediaNotificationEvent: public nsRunnable
 {
   public:
     enum GetUserMediaStatus {
       STARTING,
       STOPPING,