Bug 1402728 - Use RefPtr<TrackBuffersManager> instead of raw pointers - r?jya draft
authorGerald Squelart <gsquelart@mozilla.com>
Mon, 25 Sep 2017 11:19:05 +1300
changeset 669671 8cce3e57979af16c2e22ca98c7ec549d6b234ebd
parent 669596 7e962631ba4298bcefa571008661983d77c3e652
child 733017 ec1ec0371bc68890ac3ced82e7bc72539b8bd2ff
push id81393
push usergsquelart@mozilla.com
push dateMon, 25 Sep 2017 07:26:56 +0000
reviewersjya
bugs1402728
milestone58.0a1
Bug 1402728 - Use RefPtr<TrackBuffersManager> instead of raw pointers - r?jya All in MediaSourceDemuxer: - GetManager returns a RefPtr<TBM>. Probably unneeded as the only call stores the pointer in a RefPtr, but I'm adding it for peace of mind, and it's very low cost. - AttachSourceBuffer takes a RefPtr& and explicitly stores the given TBM in a RefPtr in the dispatched task, then DoAttachSourceBuffer takes a RefPtr<TBM>&&. This should not be needed, as it should already be done implicitly. But now it's more obvious, and a bit more optimized thanks to move semantics. - DetachSourceBuffer takes a RefPtr& and explicitly stores the given TBM in a RefPtr in the dispatched task, then DoDetachSourceBuffer takes a RefPtr<TBM>&&; And changed the loop to use RemoveElementsBy. Same as above, probably not needed, but it's now obvious that we keep a RefPtr. MozReview-Commit-ID: 9aOZtV7uS1P
dom/media/mediasource/MediaSourceDemuxer.cpp
dom/media/mediasource/MediaSourceDemuxer.h
dom/media/mediasource/SourceBuffer.cpp
--- a/dom/media/mediasource/MediaSourceDemuxer.cpp
+++ b/dom/media/mediasource/MediaSourceDemuxer.cpp
@@ -53,17 +53,17 @@ MediaSourceDemuxer::AddSizeOfResources(
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   // NB: The track buffers must only be accessed on the TaskQueue.
   RefPtr<MediaSourceDemuxer> self = this;
   RefPtr<MediaSourceDecoder::ResourceSizes> sizes = aSizes;
   nsCOMPtr<nsIRunnable> task = NS_NewRunnableFunction(
     "MediaSourceDemuxer::AddSizeOfResources", [self, sizes]() {
-      for (TrackBuffersManager* manager : self->mSourceBuffers) {
+      for (const RefPtr<TrackBuffersManager>& manager : self->mSourceBuffers) {
         manager->AddSizeOfResources(sizes);
       }
     });
 
   GetTaskQueue()->Dispatch(task.forget());
 }
 
 void MediaSourceDemuxer::NotifyInitDataArrived()
@@ -162,54 +162,57 @@ MediaSourceDemuxer::GetCrypto()
 {
   MonitorAutoLock mon(mMonitor);
   auto crypto = MakeUnique<EncryptionInfo>();
   *crypto = mInfo.mCrypto;
   return crypto;
 }
 
 void
-MediaSourceDemuxer::AttachSourceBuffer(TrackBuffersManager* aSourceBuffer)
+MediaSourceDemuxer::AttachSourceBuffer(
+  RefPtr<TrackBuffersManager>& aSourceBuffer)
 {
-  nsCOMPtr<nsIRunnable> task = NewRunnableMethod<TrackBuffersManager*>(
+  nsCOMPtr<nsIRunnable> task = NewRunnableMethod<RefPtr<TrackBuffersManager>&&>(
     "MediaSourceDemuxer::DoAttachSourceBuffer",
     this,
     &MediaSourceDemuxer::DoAttachSourceBuffer,
     aSourceBuffer);
   GetTaskQueue()->Dispatch(task.forget());
 }
 
 void
-MediaSourceDemuxer::DoAttachSourceBuffer(mozilla::TrackBuffersManager* aSourceBuffer)
+MediaSourceDemuxer::DoAttachSourceBuffer(
+  RefPtr<mozilla::TrackBuffersManager>&& aSourceBuffer)
 {
   MOZ_ASSERT(OnTaskQueue());
-  mSourceBuffers.AppendElement(aSourceBuffer);
+  mSourceBuffers.AppendElement(Move(aSourceBuffer));
   ScanSourceBuffersForContent();
 }
 
 void
-MediaSourceDemuxer::DetachSourceBuffer(TrackBuffersManager* aSourceBuffer)
+MediaSourceDemuxer::DetachSourceBuffer(
+  RefPtr<TrackBuffersManager>& aSourceBuffer)
 {
-  nsCOMPtr<nsIRunnable> task = NewRunnableMethod<TrackBuffersManager*>(
+  nsCOMPtr<nsIRunnable> task = NewRunnableMethod<RefPtr<TrackBuffersManager>&&>(
     "MediaSourceDemuxer::DoDetachSourceBuffer",
     this,
     &MediaSourceDemuxer::DoDetachSourceBuffer,
     aSourceBuffer);
   GetTaskQueue()->Dispatch(task.forget());
 }
 
 void
-MediaSourceDemuxer::DoDetachSourceBuffer(TrackBuffersManager* aSourceBuffer)
+MediaSourceDemuxer::DoDetachSourceBuffer(
+  RefPtr<TrackBuffersManager>&& aSourceBuffer)
 {
   MOZ_ASSERT(OnTaskQueue());
-  for (uint32_t i = 0; i < mSourceBuffers.Length(); i++) {
-    if (mSourceBuffers[i].get() == aSourceBuffer) {
-      mSourceBuffers.RemoveElementAt(i);
-    }
-  }
+  mSourceBuffers.RemoveElementsBy(
+    [&aSourceBuffer](const RefPtr<TrackBuffersManager> aLinkedSourceBuffer) {
+      return aLinkedSourceBuffer == aSourceBuffer;
+    });
   {
     MonitorAutoLock mon(mMonitor);
     if (aSourceBuffer == mAudioTrack) {
       mAudioTrack = nullptr;
     }
     if (aSourceBuffer == mVideoTrack) {
       mVideoTrack = nullptr;
     }
@@ -226,17 +229,17 @@ MediaSourceDemuxer::GetTrackInfo(TrackTy
       return &mInfo.mAudio;
     case TrackType::kVideoTrack:
       return &mInfo.mVideo;
     default:
       return nullptr;
   }
 }
 
-TrackBuffersManager*
+RefPtr<TrackBuffersManager>
 MediaSourceDemuxer::GetManager(TrackType aTrack)
 {
   MonitorAutoLock mon(mMonitor);
   switch (aTrack) {
     case TrackType::kAudioTrack:
       return mAudioTrack;
     case TrackType::kVideoTrack:
       return mVideoTrack;
--- a/dom/media/mediasource/MediaSourceDemuxer.h
+++ b/dom/media/mediasource/MediaSourceDemuxer.h
@@ -39,18 +39,18 @@ public:
 
   bool IsSeekable() const override;
 
   UniquePtr<EncryptionInfo> GetCrypto() override;
 
   bool ShouldComputeStartTime() const override { return false; }
 
   /* interface for TrackBuffersManager */
-  void AttachSourceBuffer(TrackBuffersManager* aSourceBuffer);
-  void DetachSourceBuffer(TrackBuffersManager* aSourceBuffer);
+  void AttachSourceBuffer(RefPtr<TrackBuffersManager>& aSourceBuffer);
+  void DetachSourceBuffer(RefPtr<TrackBuffersManager>& aSourceBuffer);
   AutoTaskQueue* GetTaskQueue() { return mTaskQueue; }
   void NotifyInitDataArrived();
 
   // Returns a string describing the state of the MediaSource internal
   // buffered data. Used for debugging purposes.
   void GetMozDebugReaderData(nsACString& aString);
 
   void AddSizeOfResources(MediaSourceDecoder::ResourceSizes* aSizes);
@@ -61,20 +61,20 @@ public:
   static constexpr media::TimeUnit EOS_FUZZ =
     media::TimeUnit::FromMicroseconds(500000);
 
 private:
   ~MediaSourceDemuxer();
   friend class MediaSourceTrackDemuxer;
   // Scan source buffers and update information.
   bool ScanSourceBuffersForContent();
-  TrackBuffersManager* GetManager(TrackInfo::TrackType aType);
+  RefPtr<TrackBuffersManager> GetManager(TrackInfo::TrackType aType);
   TrackInfo* GetTrackInfo(TrackInfo::TrackType);
-  void DoAttachSourceBuffer(TrackBuffersManager* aSourceBuffer);
-  void DoDetachSourceBuffer(TrackBuffersManager* aSourceBuffer);
+  void DoAttachSourceBuffer(RefPtr<TrackBuffersManager>&& aSourceBuffer);
+  void DoDetachSourceBuffer(RefPtr<TrackBuffersManager>&& aSourceBuffer);
   bool OnTaskQueue()
   {
     return !GetTaskQueue() || GetTaskQueue()->IsCurrentThreadIn();
   }
 
   RefPtr<AutoTaskQueue> mTaskQueue;
   nsTArray<RefPtr<MediaSourceTrackDemuxer>> mDemuxers;
 
--- a/dom/media/mediasource/SourceBuffer.cpp
+++ b/dom/media/mediasource/SourceBuffer.cpp
@@ -277,17 +277,17 @@ SourceBuffer::Detach()
   if (!mMediaSource) {
     MSE_DEBUG("Already detached");
     return;
   }
   AbortBufferAppend();
   if (mTrackBuffersManager) {
     mTrackBuffersManager->Detach();
     mMediaSource->GetDecoder()->GetDemuxer()->DetachSourceBuffer(
-      mTrackBuffersManager.get());
+      mTrackBuffersManager);
   }
   mTrackBuffersManager = nullptr;
   mMediaSource = nullptr;
 }
 
 void
 SourceBuffer::Ended()
 {
@@ -319,17 +319,17 @@ SourceBuffer::SourceBuffer(MediaSource* 
 
   ErrorResult dummy;
   if (mCurrentAttributes.mGenerateTimestamps) {
     SetMode(SourceBufferAppendMode::Sequence, dummy);
   } else {
     SetMode(SourceBufferAppendMode::Segments, dummy);
   }
   mMediaSource->GetDecoder()->GetDemuxer()->AttachSourceBuffer(
-    mTrackBuffersManager.get());
+    mTrackBuffersManager);
 }
 
 SourceBuffer::~SourceBuffer()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(!mMediaSource);
   MSE_DEBUG("");
 }