Bug 1512958 - Properly clean up produced tracks in DecodedStream. r=jya, a=lizzard
authorAndreas Pehrson <apehrson@mozilla.com>
Fri, 21 Dec 2018 16:23:57 +0000
changeset 509254 3b1807d65b232e7e491c3f7cb8889bfb8763d031
parent 509253 7103a3f5a88a2d8e835eb56e9e23ad6413a0d574
child 509255 45373d310ef956aea7d0f1b72ba95e5c07e6e530
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)
reviewersjya, lizzard
bugs1512958
milestone65.0
Bug 1512958 - Properly clean up produced tracks in DecodedStream. r=jya, a=lizzard Differential Revision: https://phabricator.services.mozilla.com/D14584
dom/media/mediasink/DecodedStream.cpp
dom/media/mediasink/DecodedStream.h
--- a/dom/media/mediasink/DecodedStream.cpp
+++ b/dom/media/mediasink/DecodedStream.cpp
@@ -56,52 +56,50 @@ class DecodedStreamTrackListener : publi
 class DecodedStreamGraphListener {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecodedStreamGraphListener)
  public:
   DecodedStreamGraphListener(SourceMediaStream* aStream, TrackID aAudioTrackID,
                              MozPromiseHolder<GenericPromise>&& aAudioEndHolder,
                              TrackID aVideoTrackID,
                              MozPromiseHolder<GenericPromise>&& aVideoEndHolder,
                              AbstractThread* aMainThread)
-      : mMutex("DecodedStreamGraphListener::mMutex"),
-        mAudioTrackListener(IsTrackIDExplicit(aAudioTrackID)
+      : mAudioTrackListener(IsTrackIDExplicit(aAudioTrackID)
                                 ? MakeRefPtr<DecodedStreamTrackListener>(
                                       this, aStream, aAudioTrackID)
                                 : nullptr),
+        mAudioTrackID(aAudioTrackID),
+        mAudioEndHolder(std::move(aAudioEndHolder)),
         mVideoTrackListener(IsTrackIDExplicit(aVideoTrackID)
                                 ? MakeRefPtr<DecodedStreamTrackListener>(
                                       this, aStream, aVideoTrackID)
                                 : nullptr),
-        mAudioTrackID(aAudioTrackID),
-        mAudioEndHolder(std::move(aAudioEndHolder)),
         mVideoTrackID(aVideoTrackID),
         mVideoEndHolder(std::move(aVideoEndHolder)),
+        mStream(aStream),
         mAbstractMainThread(aMainThread) {
+    MOZ_ASSERT(NS_IsMainThread());
     if (mAudioTrackListener) {
-      aStream->AddTrackListener(mAudioTrackListener, mAudioTrackID);
+      mStream->AddTrackListener(mAudioTrackListener, mAudioTrackID);
     } else {
       mAudioEndHolder.ResolveIfExists(true, __func__);
     }
 
     if (mVideoTrackListener) {
-      aStream->AddTrackListener(mVideoTrackListener, mVideoTrackID);
+      mStream->AddTrackListener(mVideoTrackListener, mVideoTrackID);
     } else {
       mVideoEndHolder.ResolveIfExists(true, __func__);
     }
   }
 
-  void NotifyOutput(const RefPtr<SourceMediaStream>& aStream, TrackID aTrackID,
-                    StreamTime aCurrentTrackTime) {
+  void NotifyOutput(TrackID aTrackID, StreamTime aCurrentTrackTime) {
     if (aTrackID != mAudioTrackID && mAudioTrackID != TRACK_NONE) {
       // Only audio playout drives the clock forward, if present.
       return;
     }
-    if (aStream) {
-      mOnOutput.Notify(aStream->StreamTimeToMicroseconds(aCurrentTrackTime));
-    }
+    mOnOutput.Notify(mStream->StreamTimeToMicroseconds(aCurrentTrackTime));
   }
 
   TrackID AudioTrackID() const { return mAudioTrackID; }
 
   TrackID VideoTrackID() const { return mVideoTrackID; }
 
   void DoNotifyTrackEnded(TrackID aTrackID) {
     MOZ_ASSERT(NS_IsMainThread());
@@ -110,59 +108,63 @@ class DecodedStreamGraphListener {
     } else if (aTrackID == mVideoTrackID) {
       mVideoEndHolder.ResolveIfExists(true, __func__);
     } else {
       MOZ_CRASH("Unexpected track id");
     }
   }
 
   void Forget() {
-    RefPtr<DecodedStreamGraphListener> self = this;
-    mAbstractMainThread->Dispatch(
-        NS_NewRunnableFunction("DecodedStreamGraphListener::Forget", [self]() {
-          MOZ_ASSERT(NS_IsMainThread());
-          self->mAudioEndHolder.ResolveIfExists(false, __func__);
-          self->mVideoEndHolder.ResolveIfExists(false, __func__);
-        }));
-    MutexAutoLock lock(mMutex);
+    MOZ_ASSERT(NS_IsMainThread());
+
+    if (mAudioTrackListener && !mStream->IsDestroyed()) {
+      mStream->EndTrack(mAudioTrackID);
+      mStream->RemoveTrackListener(mAudioTrackListener, mAudioTrackID);
+    }
     mAudioTrackListener = nullptr;
+    mAudioEndHolder.ResolveIfExists(false, __func__);
+
+    if (mVideoTrackListener && !mStream->IsDestroyed()) {
+      mStream->EndTrack(mVideoTrackID);
+      mStream->RemoveTrackListener(mVideoTrackListener, mVideoTrackID);
+    }
     mVideoTrackListener = nullptr;
+    mVideoEndHolder.ResolveIfExists(false, __func__);
   }
 
   MediaEventSource<int64_t>& OnOutput() { return mOnOutput; }
 
  private:
   ~DecodedStreamGraphListener() {
     MOZ_ASSERT(mAudioEndHolder.IsEmpty());
     MOZ_ASSERT(mVideoEndHolder.IsEmpty());
   }
 
   MediaEventProducer<int64_t> mOnOutput;
 
-  Mutex mMutex;
-  // Members below are protected by mMutex.
+  // Main thread only.
   RefPtr<DecodedStreamTrackListener> mAudioTrackListener;
-  RefPtr<DecodedStreamTrackListener> mVideoTrackListener;
-  // Main thread only.
   const TrackID mAudioTrackID;
   MozPromiseHolder<GenericPromise> mAudioEndHolder;
+  RefPtr<DecodedStreamTrackListener> mVideoTrackListener;
   const TrackID mVideoTrackID;
   MozPromiseHolder<GenericPromise> mVideoEndHolder;
 
+  const RefPtr<SourceMediaStream> mStream;
   const RefPtr<AbstractThread> mAbstractMainThread;
 };
 
 DecodedStreamTrackListener::DecodedStreamTrackListener(
     DecodedStreamGraphListener* aGraphListener, SourceMediaStream* aStream,
     mozilla::TrackID aTrackID)
     : mGraphListener(aGraphListener), mStream(aStream), mTrackID(aTrackID) {}
 
 void DecodedStreamTrackListener::NotifyOutput(MediaStreamGraph* aGraph,
                                               StreamTime aCurrentTrackTime) {
-  mGraphListener->NotifyOutput(mStream, mTrackID, aCurrentTrackTime);
+  mGraphListener->NotifyOutput(mTrackID, aCurrentTrackTime);
 }
 
 void DecodedStreamTrackListener::NotifyEnded() {
   mStream->Graph()->DispatchToMainThreadAfterStreamStateUpdate(
       NewRunnableMethod<mozilla::TrackID>(
           "DecodedStreamGraphListener::DoNotifyTrackEnded", mGraphListener,
           &DecodedStreamGraphListener::DoNotifyTrackEnded, mTrackID));
 }
@@ -446,30 +448,28 @@ bool DecodedStream::IsPlaying() const {
 }
 
 void DecodedStream::Shutdown() {
   AssertOwnerThread();
   mPrincipalHandle.DisconnectIfConnected();
   mWatchManager.Shutdown();
 }
 
-void DecodedStream::DestroyData(UniquePtr<DecodedStreamData> aData) {
+void DecodedStream::DestroyData(UniquePtr<DecodedStreamData>&& aData) {
   AssertOwnerThread();
 
   if (!aData) {
     return;
   }
 
   mOutputListener.Disconnect();
 
-  DecodedStreamData* data = aData.release();
-  data->Forget();
-  nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction("DecodedStream::DestroyData",
-                                                   [=]() { delete data; });
-  NS_DispatchToMainThread(r.forget());
+  NS_DispatchToMainThread(
+      NS_NewRunnableFunction("DecodedStream::DestroyData",
+                             [data = std::move(aData)]() { data->Forget(); }));
 }
 
 void DecodedStream::SetPlaying(bool aPlaying) {
   AssertOwnerThread();
 
   // Resume/pause matters only when playback started.
   if (mStartTime.isNothing()) {
     return;
--- a/dom/media/mediasink/DecodedStream.h
+++ b/dom/media/mediasink/DecodedStream.h
@@ -71,17 +71,17 @@ class DecodedStream : public media::Medi
 
  protected:
   virtual ~DecodedStream();
 
  private:
   media::TimeUnit FromMicroseconds(int64_t aTime) {
     return media::TimeUnit::FromMicroseconds(aTime);
   }
-  void DestroyData(UniquePtr<DecodedStreamData> aData);
+  void DestroyData(UniquePtr<DecodedStreamData>&& aData);
   void SendAudio(double aVolume, bool aIsSameOrigin,
                  const PrincipalHandle& aPrincipalHandle);
   void SendVideo(bool aIsSameOrigin, const PrincipalHandle& aPrincipalHandle);
   StreamTime SentDuration();
   void SendData();
   void NotifyOutput(int64_t aTime);
 
   void AssertOwnerThread() const {