Bug 1423241 - Remove a rawptr in MediaStreamTrack. r=padenot
authorAndreas Pehrson <apehrson@mozilla.com>
Fri, 23 Nov 2018 15:02:04 +0000
changeset 507054 897c956c3cf3eb7ca15e246169c0863f9440b4b0
parent 507053 243a33803d16d50cfcc8cf0a5c4739a5ed08dc55
child 507055 76c2bf0ca8a6d6c4b21cdb71b7bf5919838b2799
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1423241
milestone65.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 1423241 - Remove a rawptr in MediaStreamTrack. r=padenot Differential Revision: https://phabricator.services.mozilla.com/D12274
dom/media/MediaStreamTrack.cpp
dom/media/MediaStreamTrack.h
--- a/dom/media/MediaStreamTrack.cpp
+++ b/dom/media/MediaStreamTrack.cpp
@@ -71,64 +71,107 @@ auto MediaStreamTrackSource::ApplyConstr
  *
  * In case of multiple changes to the main thread state, the track's principal
  * will be a combination of its old principal and all the new ones until the
  * latest main thread principal matches the PrincipalHandle on the MSG thread.
  */
 class MediaStreamTrack::PrincipalHandleListener
     : public MediaStreamTrackListener {
  public:
-  explicit PrincipalHandleListener(MediaStreamTrack* aTrack) : mTrack(aTrack) {}
-
-  void Forget() {
-    MOZ_ASSERT(NS_IsMainThread());
-    mTrack = nullptr;
+  explicit PrincipalHandleListener(MediaStreamTrack* aTrack)
+      : mGraph(aTrack->GraphImpl()), mTrack(aTrack) {
+    MOZ_ASSERT(mGraph);
   }
 
   void DoNotifyPrincipalHandleChanged(
       const PrincipalHandle& aNewPrincipalHandle) {
     MOZ_ASSERT(NS_IsMainThread());
 
     if (!mTrack) {
       return;
     }
 
     mTrack->NotifyPrincipalHandleChanged(aNewPrincipalHandle);
   }
 
   void NotifyPrincipalHandleChanged(
       MediaStreamGraph* aGraph,
       const PrincipalHandle& aNewPrincipalHandle) override {
-    aGraph->DispatchToMainThreadAfterStreamStateUpdate(
+    mGraph->DispatchToMainThreadAfterStreamStateUpdate(
         NewRunnableMethod<StoreCopyPassByConstLRef<PrincipalHandle>>(
             "dom::MediaStreamTrack::PrincipalHandleListener::"
             "DoNotifyPrincipalHandleChanged",
             this, &PrincipalHandleListener::DoNotifyPrincipalHandleChanged,
             aNewPrincipalHandle));
   }
 
+  void NotifyRemoved() override {
+    // `mTrack` is a WeakPtr and must be destroyed on main thread.
+    // We dispatch ourselves to main thread here in case the MediaStreamGraph
+    // is holding the last reference to us.
+    mGraph->DispatchToMainThreadAfterStreamStateUpdate(NS_NewRunnableFunction(
+        "MediaStreamTrack::PrincipleHandleListener::mTrackReleaser",
+        [self = RefPtr<PrincipalHandleListener>(this)]() {}));
+  }
+
  protected:
-  // These fields may only be accessed on the main thread
-  MediaStreamTrack* mTrack;
+  const RefPtr<MediaStreamGraphImpl> mGraph;
+
+  // Main thread only.
+  WeakPtr<MediaStreamTrack> mTrack;
+};
+
+class TrackSink : public MediaStreamTrackSource::Sink {
+ public:
+  explicit TrackSink(MediaStreamTrack* aTrack) : mTrack(aTrack) {}
+
+  /**
+   * Keep the track source alive. This track and any clones are controlling the
+   * lifetime of the source by being registered as its sinks.
+   */
+  bool KeepsSourceAlive() const override { return true; }
+
+  bool Enabled() const override {
+    if (!mTrack) {
+      return false;
+    }
+    return mTrack->Enabled();
+  }
+
+  void PrincipalChanged() override {
+    if (mTrack) {
+      mTrack->PrincipalChanged();
+    }
+  }
+
+  void MutedChanged(bool aNewState) override {
+    if (mTrack) {
+      mTrack->MutedChanged(aNewState);
+    }
+  }
+
+ private:
+  WeakPtr<MediaStreamTrack> mTrack;
 };
 
 MediaStreamTrack::MediaStreamTrack(DOMMediaStream* aStream, TrackID aTrackID,
                                    TrackID aInputTrackID,
                                    MediaStreamTrackSource* aSource,
                                    const MediaTrackConstraints& aConstraints)
     : mOwningStream(aStream),
       mTrackID(aTrackID),
       mInputTrackID(aInputTrackID),
       mSource(aSource),
+      mSink(MakeUnique<TrackSink>(this)),
       mPrincipal(aSource->GetPrincipal()),
       mReadyState(MediaStreamTrackState::Live),
       mEnabled(true),
       mMuted(false),
       mConstraints(aConstraints) {
-  GetSource().RegisterSink(this);
+  GetSource().RegisterSink(mSink.get());
 
   if (GetOwnedStream()) {
     mPrincipalHandleListener = new PrincipalHandleListener(this);
     AddListener(mPrincipalHandleListener);
   }
 
   nsresult rv;
   nsCOMPtr<nsIUUIDGenerator> uuidgen =
@@ -143,24 +186,24 @@ MediaStreamTrack::MediaStreamTrack(DOMMe
   char chars[NSID_LENGTH];
   uuid.ToProvidedString(chars);
   mID = NS_ConvertASCIItoUTF16(chars);
 }
 
 MediaStreamTrack::~MediaStreamTrack() { Destroy(); }
 
 void MediaStreamTrack::Destroy() {
+  mReadyState = MediaStreamTrackState::Ended;
   if (mSource) {
-    mSource->UnregisterSink(this);
+    mSource->UnregisterSink(mSink.get());
   }
   if (mPrincipalHandleListener) {
     if (GetOwnedStream()) {
       RemoveListener(mPrincipalHandleListener);
     }
-    mPrincipalHandleListener->Forget();
     mPrincipalHandleListener = nullptr;
   }
   // Remove all listeners -- avoid iterating over the list we're removing from
   const nsTArray<RefPtr<MediaStreamTrackListener>> trackListeners(
       mTrackListeners);
   for (auto listener : trackListeners) {
     RemoveListener(listener);
   }
@@ -233,17 +276,17 @@ void MediaStreamTrack::Stop() {
     return;
   }
 
   if (!mSource) {
     MOZ_ASSERT(false);
     return;
   }
 
-  mSource->UnregisterSink(this);
+  mSource->UnregisterSink(mSink.get());
 
   MOZ_ASSERT(mOwningStream,
              "Every MediaStreamTrack needs an owning DOMMediaStream");
   DOMMediaStream::TrackPort* port = mOwningStream->FindOwnedTrackPort(*this);
   MOZ_ASSERT(port,
              "A MediaStreamTrack must exist in its owning DOMMediaStream");
   RefPtr<Pledge<bool>> p =
       port->BlockSourceTrackId(mInputTrackID, BlockingMode::CREATION);
@@ -366,16 +409,26 @@ void MediaStreamTrack::NotifyPrincipalHa
     SetPrincipal(mPendingPrincipal);
     mPendingPrincipal = nullptr;
   }
 }
 
 void MediaStreamTrack::MutedChanged(bool aNewState) {
   MOZ_ASSERT(NS_IsMainThread());
 
+  /**
+   * 4.3.1 Life-cycle and Media flow - Media flow
+   * To set a track's muted state to newState, the User Agent MUST run the
+   * following steps:
+   *  1. Let track be the MediaStreamTrack in question.
+   *  2. Set track's muted attribute to newState.
+   *  3. If newState is true let eventName be mute, otherwise unmute.
+   *  4. Fire a simple event named eventName on track.
+   */
+
   if (mMuted == aNewState) {
     MOZ_ASSERT_UNREACHABLE("Muted state didn't actually change");
     return;
   }
 
   LOG(LogLevel::Info,
       ("MediaStreamTrack %p became %s", this, aNewState ? "muted" : "unmuted"));
 
@@ -443,17 +496,17 @@ already_AddRefed<MediaStreamTrack> Media
 
 void MediaStreamTrack::SetReadyState(MediaStreamTrackState aState) {
   MOZ_ASSERT(!(mReadyState == MediaStreamTrackState::Ended &&
                aState == MediaStreamTrackState::Live),
              "We don't support overriding the ready state from ended to live");
 
   if (mReadyState == MediaStreamTrackState::Live &&
       aState == MediaStreamTrackState::Ended && mSource) {
-    mSource->UnregisterSink(this);
+    mSource->UnregisterSink(mSink.get());
   }
 
   mReadyState = aState;
 }
 
 void MediaStreamTrack::OverrideEnded() {
   MOZ_ASSERT(NS_IsMainThread());
 
@@ -463,17 +516,22 @@ void MediaStreamTrack::OverrideEnded() {
 
   LOG(LogLevel::Info, ("MediaStreamTrack %p ended", this));
 
   if (!mSource) {
     MOZ_ASSERT(false);
     return;
   }
 
-  mSource->UnregisterSink(this);
+  mSource->UnregisterSink(mSink.get());
+
+  if (mPrincipalHandleListener) {
+    RemoveListener(mPrincipalHandleListener);
+  }
+  mPrincipalHandleListener = nullptr;
 
   mReadyState = MediaStreamTrackState::Ended;
 
   NotifyEnded();
 
   DispatchTrustedEvent(NS_LITERAL_STRING("ended"));
 }
 
--- a/dom/media/MediaStreamTrack.h
+++ b/dom/media/MediaStreamTrack.h
@@ -36,16 +36,17 @@ class ProcessedMediaStream;
 class RemoteSourceStreamInfo;
 class SourceStreamInfo;
 
 namespace dom {
 
 class AudioStreamTrack;
 class VideoStreamTrack;
 class MediaStreamError;
+class TrackSink;
 enum class CallerType : uint32_t;
 
 /**
  * Common interface through which a MediaStreamTrack can communicate with its
  * producer on the main thread.
  *
  * Kept alive by a strong ref in all MediaStreamTracks (original and clones)
  * sharing this source.
@@ -81,18 +82,30 @@ class MediaStreamTrackSource : public ns
      * call MediaStreamTrackSource::SinkEnabledStateChanged().
      *
      * Typically MediaStreamTrack returns the track's enabled state and other
      * Sinks (like HTMLMediaElement::StreamCaptureTrackSource) return false so
      * control over device state remains with tracks and their enabled state.
      */
     virtual bool Enabled() const = 0;
 
+    /**
+     * Called when the principal of the MediaStreamTrackSource where this sink
+     * is registered has changed.
+     */
     virtual void PrincipalChanged() = 0;
+
+    /**
+     * Called when the muted state of the MediaStreamTrackSource where this sink
+     * is registered has changed.
+     */
     virtual void MutedChanged(bool aNewState) = 0;
+
+   protected:
+    virtual ~Sink() = default;
   };
 
   MediaStreamTrackSource(nsIPrincipal* aPrincipal, const nsString& aLabel)
       : mPrincipal(aPrincipal), mLabel(aLabel), mStopped(false) {}
 
   /**
    * Use to clean up any resources that have to be cleaned before the
    * destructor is called. It is often too late in the destructor because
@@ -333,17 +346,17 @@ class MediaStreamTrackConsumer
    */
   virtual void NotifyEnded(MediaStreamTrack* aTrack){};
 };
 
 /**
  * Class representing a track in a DOMMediaStream.
  */
 class MediaStreamTrack : public DOMEventTargetHelper,
-                         public MediaStreamTrackSource::Sink {
+                         public SupportsWeakPtr<MediaStreamTrack> {
   // DOMMediaStream owns MediaStreamTrack instances, and requires access to
   // some internal state, e.g., GetInputStream(), GetOwnedStream().
   friend class mozilla::DOMMediaStream;
 
   // PeerConnection and friends need to know our owning DOMStream and track id.
   friend class mozilla::PeerConnectionImpl;
   friend class mozilla::PeerConnectionMedia;
   friend class mozilla::SourceStreamInfo;
@@ -360,33 +373,35 @@ class MediaStreamTrack : public DOMEvent
       DOMMediaStream* aStream, TrackID aTrackID, TrackID aInputTrackID,
       MediaStreamTrackSource* aSource,
       const MediaTrackConstraints& aConstraints = MediaTrackConstraints());
 
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(MediaStreamTrack,
                                            DOMEventTargetHelper)
 
+  MOZ_DECLARE_WEAKREFERENCE_TYPENAME(MediaStreamTrack)
+
   nsPIDOMWindowInner* GetParentObject() const;
   JSObject* WrapObject(JSContext* aCx,
                        JS::Handle<JSObject*> aGivenProto) override;
 
   virtual AudioStreamTrack* AsAudioStreamTrack() { return nullptr; }
   virtual VideoStreamTrack* AsVideoStreamTrack() { return nullptr; }
 
   virtual const AudioStreamTrack* AsAudioStreamTrack() const { return nullptr; }
   virtual const VideoStreamTrack* AsVideoStreamTrack() const { return nullptr; }
 
   // WebIDL
   virtual void GetKind(nsAString& aKind) = 0;
   void GetId(nsAString& aID) const;
   virtual void GetLabel(nsAString& aLabel, CallerType /* aCallerType */) {
     GetSource().GetLabel(aLabel);
   }
-  bool Enabled() const override { return mEnabled; }
+  bool Enabled() const { return mEnabled; }
   void SetEnabled(bool aEnabled);
   bool Muted() { return mMuted; }
   void Stop();
   void GetConstraints(dom::MediaTrackConstraints& aResult);
   void GetSettings(dom::MediaTrackSettings& aResult, CallerType aCallerType);
 
   already_AddRefed<Promise> ApplyConstraints(
       const dom::MediaTrackConstraints& aConstraints, CallerType aCallerType,
@@ -457,36 +472,25 @@ class MediaStreamTrack : public DOMEvent
                        "The track source is only removed on destruction");
     return *mSource;
   }
 
   // Webrtc allows the remote side to name tracks whatever it wants, and we
   // need to surface this to content.
   void AssignId(const nsAString& aID) { mID = aID; }
 
-  // Implementation of MediaStreamTrackSource::Sink
+  /**
+   * Called when mSource's principal has changed.
+   */
+  void PrincipalChanged();
 
   /**
-   * Keep the track source alive. This track and any clones are controlling the
-   * lifetime of the source by being registered as its sinks.
+   * Called when mSource's muted state has changed.
    */
-  bool KeepsSourceAlive() const override { return true; }
-
-  void PrincipalChanged() override;
-
-  /**
-   * 4.3.1 Life-cycle and Media flow - Media flow
-   * To set a track's muted state to newState, the User Agent MUST run the
-   * following steps:
-   *  1. Let track be the MediaStreamTrack in question.
-   *  2. Set track's muted attribute to newState.
-   *  3. If newState is true let eventName be mute, otherwise unmute.
-   *  4. Fire a simple event named eventName on track.
-   */
-  void MutedChanged(bool aNewState) override;
+  void MutedChanged(bool aNewState);
 
   /**
    * Add a PrincipalChangeObserver to this track.
    *
    * Returns true if it was successfully added.
    *
    * Ownership of the PrincipalChangeObserver remains with the caller, and it's
    * the caller's responsibility to remove the observer before it dies.
@@ -589,16 +593,17 @@ class MediaStreamTrack : public DOMEvent
       mPrincipalChangeObservers;
 
   nsTArray<WeakPtr<MediaStreamTrackConsumer>> mConsumers;
 
   RefPtr<DOMMediaStream> mOwningStream;
   TrackID mTrackID;
   TrackID mInputTrackID;
   RefPtr<MediaStreamTrackSource> mSource;
+  const UniquePtr<TrackSink> mSink;
   RefPtr<MediaStreamTrack> mOriginalTrack;
   nsCOMPtr<nsIPrincipal> mPrincipal;
   nsCOMPtr<nsIPrincipal> mPendingPrincipal;
   RefPtr<PrincipalHandleListener> mPrincipalHandleListener;
   // Keep tracking MediaStreamTrackListener and DirectMediaStreamTrackListener,
   // so we can remove them in |Destory|.
   nsTArray<RefPtr<MediaStreamTrackListener>> mTrackListeners;
   nsTArray<RefPtr<DirectMediaStreamTrackListener>> mDirectTrackListeners;