Bug 1410829 - Signal listeners removed on shutdown so they can clean up. r?padenot draft
authorAndreas Pehrson <pehrsons@mozilla.com>
Thu, 26 Oct 2017 16:13:38 +0200
changeset 688569 e4dfa94215fc4ffff725af83931285db5aaf1eb5
parent 679173 bc7a5be76b723cf6aac1a919156e74997c5f4902
child 688570 cdf8016f54d27a2e4787dd0d8d5199af8efd14ea
push id86788
push userbmo:apehrson@mozilla.com
push dateMon, 30 Oct 2017 08:55:34 +0000
reviewerspadenot
bugs1410829
milestone58.0a1
Bug 1410829 - Signal listeners removed on shutdown so they can clean up. r?padenot When shutting down we shut modules down in the order of [media, gfx, cycleCollector]. At the same time we rely on destructors to clean up resources for MediaStreams and MediaStreamTracks, but these objects may be held until cycleCollector shutdown. Gfx resources are not allowed to be released after gfx shutdown, which is where we this approach hits a wall. This patch will signal them through the three available listener types to clean up during media shutdown. MozReview-Commit-ID: FwsG3ukV29P
dom/media/MediaStreamGraph.cpp
dom/media/MediaStreamGraph.h
--- a/dom/media/MediaStreamGraph.cpp
+++ b/dom/media/MediaStreamGraph.cpp
@@ -1599,29 +1599,31 @@ public:
     // which requires shutdown to proceed.
 
     // mGraph's thread is not running so it's OK to do whatever here
     if (mGraph->IsEmpty()) {
       // mGraph is no longer needed, so delete it.
       mGraph->Destroy();
     } else {
       // The graph is not empty.  We must be in a forced shutdown, or a
-      // non-realtime graph that has finished processing.  Some later
-      // AppendMessage will detect that the manager has been emptied, and
+      // non-realtime graph that has finished processing. Some later
+      // AppendMessage will detect that the graph has been emptied, and
       // delete it.
       NS_ASSERTION(mGraph->mForceShutDown || !mGraph->mRealtime,
                    "Not in forced shutdown?");
       for (MediaStream* stream : mGraph->AllStreams()) {
         // Clean up all MediaSegments since we cannot release Images too
-        // late during shutdown.
+        // late during shutdown. Also notify listeners that they were removed
+        // so they can clean up any gfx resources.
         if (SourceMediaStream* source = stream->AsSourceStream()) {
           // Finishing a SourceStream prevents new data from being appended.
           source->Finish();
         }
         stream->GetStreamTracks().Clear();
+        stream->RemoveAllListenersImpl();
       }
 
       mGraph->mLifecycleState =
         MediaStreamGraphImpl::LIFECYCLE_WAITING_FOR_STREAM_DESTRUCTION;
     }
     return NS_OK;
   }
 private:
@@ -2072,21 +2074,33 @@ MediaStream::EnsureTrack(TrackID aTrackI
     track = &mTracks.AddTrack(aTrackId, 0, segment.forget());
   }
   return track;
 }
 
 void
 MediaStream::RemoveAllListenersImpl()
 {
-  for (int32_t i = mListeners.Length() - 1; i >= 0; --i) {
-    RefPtr<MediaStreamListener> listener = mListeners[i].forget();
-    listener->NotifyEvent(GraphImpl(), MediaStreamGraphEvent::EVENT_REMOVED);
+  GraphImpl()->AssertOnGraphThreadOrNotRunning();
+
+  auto streamListeners(mListeners);
+  for (auto& l : streamListeners) {
+    l->NotifyEvent(GraphImpl(), MediaStreamGraphEvent::EVENT_REMOVED);
   }
   mListeners.Clear();
+
+  auto trackListeners(mTrackListeners);
+  for (auto& l : trackListeners) {
+    l.mListener->NotifyRemoved();
+  }
+  mTrackListeners.Clear();
+
+  if (SourceMediaStream* source = AsSourceStream()) {
+    source->RemoveAllDirectListeners();
+  }
 }
 
 void
 MediaStream::DestroyImpl()
 {
   for (int32_t i = mConsumers.Length() - 1; i >= 0; --i) {
     mConsumers[i]->Disconnect();
   }
@@ -3117,16 +3131,28 @@ SourceMediaStream::EndAllTrackAndFinish(
     SourceMediaStream::TrackData* data = &mUpdateTracks[i];
     data->mCommands |= TrackEventCommand::TRACK_EVENT_ENDED;
   }
   mPendingTracks.Clear();
   FinishWithLockHeld();
   // we will call NotifyEvent() to let GetUserMedia know
 }
 
+void
+SourceMediaStream::RemoveAllDirectListeners()
+{
+  GraphImpl()->AssertOnGraphThreadOrNotRunning();
+
+  auto directListeners(mDirectTrackListeners);
+  for (auto& l : directListeners) {
+    l.mListener->NotifyDirectListenerUninstalled();
+  }
+  mDirectTrackListeners.Clear();
+}
+
 SourceMediaStream::~SourceMediaStream()
 {
 }
 
 void
 SourceMediaStream::RegisterForAudioMixing()
 {
   MutexAutoLock lock(mMutex);
--- a/dom/media/MediaStreamGraph.h
+++ b/dom/media/MediaStreamGraph.h
@@ -779,16 +779,22 @@ public:
   }
 
   /**
    * End all tracks and Finish() this stream.  Used to voluntarily revoke access
    * to a LocalMediaStream.
    */
   void EndAllTrackAndFinish();
 
+  /**
+   * Removes all direct listeners and signals to them that they have been
+   * uninstalled.
+   */
+  void RemoveAllDirectListeners();
+
   void RegisterForAudioMixing();
 
   /**
    * Returns true if this SourceMediaStream contains at least one audio track
    * that is in pending state.
    * This is thread safe, and takes the SourceMediaStream mutex.
    */
   bool HasPendingAudioTrack();