Bug 1407542 - Remove back reference to consumer in MediaStreamTrack. r=jib,smaug
authorAndreas Pehrson <apehrson@mozilla.com>
Tue, 10 Oct 2017 20:48:58 +0200
changeset 443123 5cb403db2f66b5418d05604693e8127ed4d1dd8b
parent 443122 21762797d5ec694d319a209ef80c3c94b0229bf4
child 443124 8dd4ccaed43e0e5102c35dab980c14f82c3870a4
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjib, smaug
bugs1407542
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1407542 - Remove back reference to consumer in MediaStreamTrack. r=jib,smaug It doesn't matter that this is traversed by the cycle collector when the track is live and playing. It prevents cycle collection of any number of MediaStreams that contain (thus consume) this track. MozReview-Commit-ID: GvdLfWDTVQQ
dom/media/DOMMediaStream.cpp
dom/media/MediaStreamTrack.cpp
dom/media/MediaStreamTrack.h
--- a/dom/media/DOMMediaStream.cpp
+++ b/dom/media/DOMMediaStream.cpp
@@ -336,19 +336,18 @@ private:
 };
 
 class DOMMediaStream::PlaybackTrackListener : public MediaStreamTrackConsumer
 {
 public:
   explicit PlaybackTrackListener(DOMMediaStream* aStream) :
     mStream(aStream) {}
 
-  NS_DECL_ISUPPORTS_INHERITED
-  NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(PlaybackTrackListener,
-                                           MediaStreamTrackConsumer)
+  NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(PlaybackTrackListener)
+  NS_DECL_CYCLE_COLLECTION_NATIVE_CLASS(PlaybackTrackListener)
 
   void NotifyEnded(MediaStreamTrack* aTrack) override
   {
     if (!mStream) {
       MOZ_ASSERT(false);
       return;
     }
 
@@ -362,25 +361,19 @@ public:
   }
 
 protected:
   virtual ~PlaybackTrackListener() {}
 
   RefPtr<DOMMediaStream> mStream;
 };
 
-NS_IMPL_ADDREF_INHERITED(DOMMediaStream::PlaybackTrackListener,
-                         MediaStreamTrackConsumer)
-NS_IMPL_RELEASE_INHERITED(DOMMediaStream::PlaybackTrackListener,
-                          MediaStreamTrackConsumer)
-NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(DOMMediaStream::PlaybackTrackListener)
-NS_INTERFACE_MAP_END_INHERITING(MediaStreamTrackConsumer)
-NS_IMPL_CYCLE_COLLECTION_INHERITED(DOMMediaStream::PlaybackTrackListener,
-                                   MediaStreamTrackConsumer,
-                                   mStream)
+NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(DOMMediaStream::PlaybackTrackListener, AddRef)
+NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(DOMMediaStream::PlaybackTrackListener, Release)
+NS_IMPL_CYCLE_COLLECTION(DOMMediaStream::PlaybackTrackListener, mStream)
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(DOMMediaStream)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(DOMMediaStream,
                                                 DOMEventTargetHelper)
   tmp->Destroy();
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mWindow)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mOwnedTracks)
--- a/dom/media/MediaStreamTrack.cpp
+++ b/dom/media/MediaStreamTrack.cpp
@@ -52,24 +52,16 @@ MediaStreamTrackSource::ApplyConstraints
 {
   RefPtr<PledgeVoid> p = new PledgeVoid();
   p->Reject(new MediaStreamError(aWindow,
                                  NS_LITERAL_STRING("OverconstrainedError"),
                                  NS_LITERAL_STRING("")));
   return p.forget();
 }
 
-NS_IMPL_CYCLE_COLLECTING_ADDREF(MediaStreamTrackConsumer)
-NS_IMPL_CYCLE_COLLECTING_RELEASE(MediaStreamTrackConsumer)
-NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(MediaStreamTrackConsumer)
-  NS_INTERFACE_MAP_ENTRY(nsISupports)
-NS_INTERFACE_MAP_END
-
-NS_IMPL_CYCLE_COLLECTION_0(MediaStreamTrackConsumer)
-
 /**
  * PrincipalHandleListener monitors changes in PrincipalHandle of the media flowing
  * through the MediaStreamGraph.
  *
  * When the main thread principal for a MediaStreamTrack changes, its principal
  * will be set to the combination of the previous principal and the new one.
  *
  * As a PrincipalHandle change later happens on the MediaStreamGraph thread, we will
@@ -182,27 +174,25 @@ MediaStreamTrack::Destroy()
   }
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(MediaStreamTrack)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(MediaStreamTrack,
                                                 DOMEventTargetHelper)
   tmp->Destroy();
-  NS_IMPL_CYCLE_COLLECTION_UNLINK(mConsumers)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mOwningStream)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mSource)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mOriginalTrack)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mPrincipal)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mPendingPrincipal)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(MediaStreamTrack,
                                                   DOMEventTargetHelper)
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mConsumers)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mOwningStream)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mSource)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mOriginalTrack)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPrincipal)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mPendingPrincipal)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_ADDREF_INHERITED(MediaStreamTrack, DOMEventTargetHelper)
@@ -374,20 +364,24 @@ MediaStreamTrack::NotifyPrincipalHandleC
   }
 }
 
 void
 MediaStreamTrack::NotifyEnded()
 {
   MOZ_ASSERT(mReadyState == MediaStreamTrackState::Ended);
 
-  for (int32_t i = mConsumers.Length() - 1; i >= 0; --i) {
-    // Loop backwards by index in case the consumer removes itself in the
-    // callback.
-    mConsumers[i]->NotifyEnded(this);
+  auto consumers(mConsumers);
+  for (const auto& consumer : consumers) {
+    if (consumer) {
+      consumer->NotifyEnded(this);
+    } else {
+      MOZ_ASSERT_UNREACHABLE("A consumer was not explicitly removed");
+      mConsumers.RemoveElement(consumer);
+    }
   }
 }
 
 bool
 MediaStreamTrack::AddPrincipalChangeObserver(
   PrincipalChangeObserver<MediaStreamTrack>* aObserver)
 {
   return mPrincipalChangeObservers.AppendElement(aObserver) != nullptr;
@@ -400,22 +394,32 @@ MediaStreamTrack::RemovePrincipalChangeO
   return mPrincipalChangeObservers.RemoveElement(aObserver);
 }
 
 void
 MediaStreamTrack::AddConsumer(MediaStreamTrackConsumer* aConsumer)
 {
   MOZ_ASSERT(!mConsumers.Contains(aConsumer));
   mConsumers.AppendElement(aConsumer);
+
+  // Remove destroyed consumers for cleanliness
+  while (mConsumers.RemoveElement(nullptr)) {
+    MOZ_ASSERT_UNREACHABLE("A consumer was not explicitly removed");
+  }
 }
 
 void
 MediaStreamTrack::RemoveConsumer(MediaStreamTrackConsumer* aConsumer)
 {
   mConsumers.RemoveElement(aConsumer);
+
+  // Remove destroyed consumers for cleanliness
+  while (mConsumers.RemoveElement(nullptr)) {
+    MOZ_ASSERT_UNREACHABLE("A consumer was not explicitly removed");
+  }
 }
 
 already_AddRefed<MediaStreamTrack>
 MediaStreamTrack::Clone()
 {
   // MediaStreamTracks are currently governed by streams, so we need a dummy
   // DOMMediaStream to own our track clone. The dummy will never see any
   // dynamically created tracks (no input stream) so no need for a SourceGetter.
--- a/dom/media/MediaStreamTrack.h
+++ b/dom/media/MediaStreamTrack.h
@@ -9,16 +9,17 @@
 #include "MediaTrackConstraints.h"
 #include "PrincipalChangeObserver.h"
 #include "StreamTracks.h"
 #include "mozilla/CORSMode.h"
 #include "mozilla/DOMEventTargetHelper.h"
 #include "mozilla/dom/MediaStreamTrackBinding.h"
 #include "mozilla/dom/MediaTrackSettingsBinding.h"
 #include "mozilla/media/MediaUtils.h"
+#include "mozilla/WeakPtr.h"
 #include "nsError.h"
 #include "nsID.h"
 #include "nsIPrincipal.h"
 
 namespace mozilla {
 
 class DOMMediaStream;
 class MediaEnginePhotoCallback;
@@ -216,31 +217,28 @@ protected:
 
   const MediaSourceEnum mMediaSource;
 };
 
 /**
  * Base class that consumers of a MediaStreamTrack can use to get notifications
  * about state changes in the track.
  */
-class MediaStreamTrackConsumer : public nsISupports
+class MediaStreamTrackConsumer
+  : public SupportsWeakPtr<MediaStreamTrackConsumer>
 {
 public:
-  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
-  NS_DECL_CYCLE_COLLECTION_CLASS(MediaStreamTrackConsumer)
+  MOZ_DECLARE_WEAKREFERENCE_TYPENAME(MediaStreamTrackConsumer)
 
   /**
    * Called when the track's readyState transitions to "ended".
    * Unlike the "ended" event exposed to script this is called for any reason,
    * including MediaStreamTrack::Stop().
    */
   virtual void NotifyEnded(MediaStreamTrack* aTrack) {};
-
-protected:
-  virtual ~MediaStreamTrackConsumer() {}
 };
 
 /**
  * Class representing a track in a DOMMediaStream.
  */
 class MediaStreamTrack : public DOMEventTargetHelper,
                          public MediaStreamTrackSource::Sink
 {
@@ -451,17 +449,17 @@ protected:
    * source as this MediaStreamTrack.
    * aTrackID is the TrackID the new track will have in its owned stream.
    */
   virtual already_AddRefed<MediaStreamTrack> CloneInternal(DOMMediaStream* aOwningStream,
                                                            TrackID aTrackID) = 0;
 
   nsTArray<PrincipalChangeObserver<MediaStreamTrack>*> mPrincipalChangeObservers;
 
-  nsTArray<RefPtr<MediaStreamTrackConsumer>> mConsumers;
+  nsTArray<WeakPtr<MediaStreamTrackConsumer>> mConsumers;
 
   RefPtr<DOMMediaStream> mOwningStream;
   TrackID mTrackID;
   TrackID mInputTrackID;
   RefPtr<MediaStreamTrackSource> mSource;
   RefPtr<MediaStreamTrack> mOriginalTrack;
   nsCOMPtr<nsIPrincipal> mPrincipal;
   nsCOMPtr<nsIPrincipal> mPendingPrincipal;