Bug 1550955 - Make StopTrack() API require a hard reference. r=pehrsons
authorJan-Ivar Bruaroey <jib@mozilla.com>
Fri, 17 May 2019 19:38:44 +0000
changeset 474383 f12fb7f69d5180c2c133b441e23b0b8b8d4fe123
parent 474382 a18f289abc5d8034c9cdf85992a646fff47966ed
child 474384 02f6bb0dec8a9c95ed4a01235ee684b24fd95251
push id113152
push userdluca@mozilla.com
push dateSat, 18 May 2019 10:33:03 +0000
treeherdermozilla-inbound@9b2f851979cb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspehrsons
bugs1550955
milestone68.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 1550955 - Make StopTrack() API require a hard reference. r=pehrsons Differential Revision: https://phabricator.services.mozilla.com/D31555
dom/media/MediaManager.cpp
--- a/dom/media/MediaManager.cpp
+++ b/dom/media/MediaManager.cpp
@@ -317,27 +317,29 @@ class SourceListener : public SupportsWe
    */
   RefPtr<SourceListenerPromise> InitializeAsync();
 
   /**
    * Stops all live tracks, finishes the associated MediaStream and cleans up
    * the weak reference to the associated window listener.
    * This will also tell the window listener to remove its hard reference to
    * this SourceListener, so any caller will need to keep its own hard ref.
+   * We enforce this by requiring a reference to a hard ref.
    */
-  void Stop();
+  void Stop(const RefPtr<SourceListener>& self);
 
   /**
    * Posts a task to stop the device associated with aTrackID and notifies the
    * associated window listener that a track was stopped.
    * Should this track be the last live one to be stopped, we'll also call Stop.
    * This might tell the window listener to remove its hard reference to this
    * SourceListener, so any caller will need to keep its own hard ref.
+   * We enforce this by requiring a reference to a hard ref.
    */
-  void StopTrack(TrackID aTrackID);
+  void StopTrack(const RefPtr<SourceListener>& self, TrackID aTrackID);
 
   /**
    * Gets the main thread MediaTrackSettings from the MediaEngineSource
    * associated with aTrackID.
    */
   void GetSettingsFor(TrackID aTrackID,
                       dom::MediaTrackSettings& aOutSettings) const;
 
@@ -548,17 +550,17 @@ class GetUserMediaWindowListener {
                "A SourceListener should only be once in one of "
                "mInactiveListeners and mActiveListeners");
     MOZ_ASSERT(!mActiveListeners.Contains(aListener),
                "A SourceListener should only be once in one of "
                "mInactiveListeners and mActiveListeners");
 
     LOG("GUMWindowListener %p stopping SourceListener %p.", this,
         aListener.get());
-    aListener->Stop();
+    aListener->Stop(aListener);
 
     if (MediaDevice* removedDevice = aListener->GetVideoDevice()) {
       bool revokeVideoPermission = true;
       nsString removedRawId;
       nsString removedSourceType;
       removedDevice->GetRawId(removedRawId);
       removedDevice->GetMediaSource(removedSourceType);
       for (const auto& l : mActiveListeners) {
@@ -1162,18 +1164,17 @@ class GetUserMediaStreamRunnable : publi
         void GetSettings(dom::MediaTrackSettings& aOutSettings) override {
           if (mListener) {
             mListener->GetSettingsFor(mTrackID, aOutSettings);
           }
         }
 
         void Stop() override {
           if (mListener) {
-            RefPtr<SourceListener> deathGrip(mListener);
-            deathGrip->StopTrack(mTrackID);
+            mListener->StopTrack(RefPtr<SourceListener>(mListener), mTrackID);
             mListener = nullptr;
           }
         }
 
         void Disable() override {
           if (mListener) {
             mListener->SetEnabledFor(mTrackID, false);
           }
@@ -4186,48 +4187,49 @@ SourceListener::InitializeAsync() {
 
               state->mStopped = true;
             }
             return SourceListenerPromise::CreateAndReject(std::move(aResult),
                                                           __func__);
           });
 }
 
-void SourceListener::Stop() {
+void SourceListener::Stop(const RefPtr<SourceListener>& self) {
   MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread");
 
   // StopSharing() has some special logic, at least for audio capture.
   // It must be called when all tracks have stopped, before setting mStopped.
   StopSharing();
 
   if (mStopped) {
     return;
   }
   mStopped = true;
 
   LOG("SourceListener %p stopping", this);
 
   if (mAudioDeviceState) {
     mAudioDeviceState->mDisableTimer->Cancel();
     if (!mAudioDeviceState->mStopped) {
-      StopTrack(kAudioTrack);
+      StopTrack(self, kAudioTrack);
     }
   }
   if (mVideoDeviceState) {
     mVideoDeviceState->mDisableTimer->Cancel();
     if (!mVideoDeviceState->mStopped) {
-      StopTrack(kVideoTrack);
+      StopTrack(self, kVideoTrack);
     }
   }
 
   mWindowListener->Remove(this);
   mWindowListener = nullptr;
 }
 
-void SourceListener::StopTrack(TrackID aTrackID) {
+void SourceListener::StopTrack(const RefPtr<SourceListener>& self,
+                               TrackID aTrackID) {
   MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread");
   MOZ_ASSERT(Activated(), "No device to stop");
   MOZ_ASSERT(aTrackID == kAudioTrack || aTrackID == kVideoTrack,
              "Unknown track id");
   DeviceState& state = GetDeviceStateFor(aTrackID);
 
   LOG("SourceListener %p stopping %s track %d", this,
       aTrackID == kAudioTrack ? "audio" : "video", aTrackID);
@@ -4246,17 +4248,17 @@ void SourceListener::StopTrack(TrackID a
   }));
 
   MOZ_ASSERT(mWindowListener, "Should still have window listener");
   mWindowListener->ChromeAffectingStateChanged();
 
   if ((!mAudioDeviceState || mAudioDeviceState->mStopped) &&
       (!mVideoDeviceState || mVideoDeviceState->mStopped)) {
     LOG("SourceListener %p this was the last track stopped", this);
-    Stop();
+    Stop(self);
   }
 }
 
 void SourceListener::GetSettingsFor(
     TrackID aTrackID, dom::MediaTrackSettings& aOutSettings) const {
   MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread");
   GetDeviceStateFor(aTrackID).mDevice->GetSettings(aOutSettings);
 }
@@ -4384,17 +4386,17 @@ void SourceListener::SetEnabledFor(Track
             if (NS_FAILED(aResult) && aResult != NS_ERROR_ABORT) {
               // This path handles errors from starting or stopping the device.
               // NS_ERROR_ABORT are for cases where *we* aborted. They need
               // graceful handling.
               if (aEnable) {
                 // Starting the device failed. Stopping the track here will make
                 // the MediaStreamTrack end after a pass through the
                 // MediaStreamGraph.
-                StopTrack(aTrackID);
+                StopTrack(self, aTrackID);
               } else {
                 // Stopping the device failed. This is odd, but not fatal.
                 MOZ_ASSERT_UNREACHABLE("The device should be stoppable");
 
                 // To keep our internal state sane in this case, we disallow
                 // future stops due to disable.
                 state.mOffWhileDisabled = false;
               }
@@ -4428,24 +4430,25 @@ void SourceListener::StopSharing() {
 
   if (mStopped) {
     return;
   }
 
   MOZ_RELEASE_ASSERT(mWindowListener);
   LOG("SourceListener %p StopSharing", this);
 
+  RefPtr<SourceListener> self(this);
   if (mVideoDeviceState && (mVideoDeviceState->mDevice->GetMediaSource() ==
                                 MediaSourceEnum::Screen ||
                             mVideoDeviceState->mDevice->GetMediaSource() ==
                                 MediaSourceEnum::Window)) {
     // We want to stop the whole stream if there's no audio;
     // just the video track if we have both.
     // StopTrack figures this out for us.
-    StopTrack(kVideoTrack);
+    StopTrack(self, kVideoTrack);
   }
   if (mAudioDeviceState && mAudioDeviceState->mDevice->GetMediaSource() ==
                                MediaSourceEnum::AudioCapture) {
     uint64_t windowID = mWindowListener->WindowID();
     auto* window = nsGlobalWindowInner::GetInnerWindowWithId(windowID);
     MOZ_RELEASE_ASSERT(window);
     window->SetAudioCapture(false);
     MediaStreamGraph* graph = mStream->Graph();
@@ -4609,17 +4612,17 @@ void GetUserMediaWindowListener::StopRaw
       nsString id;
       source->GetVideoDevice()->GetRawId(id);
       if (removedDeviceID.Equals(id)) {
         matches.AppendElement(MakePair(source, TrackID(kVideoTrack)));
       }
     }
   }
   for (auto& pair : matches) {
-    pair.first()->StopTrack(pair.second());
+    pair.first()->StopTrack(pair.first(), pair.second());
   }
 }
 
 void GetUserMediaWindowListener::ChromeAffectingStateChanged() {
   MOZ_ASSERT(NS_IsMainThread());
 
   // We wait until stable state before notifying chrome so chrome only does one
   // update if more updates happen in this event loop.