Uses member variable for the callback of RemoveTrack()

This CL puts the callback of RemoveTrack() to a member variable rather
than passing as parameter to fix an issue that UserMediaClientImpl
sometimes cannot process request after MediaStreamVideoSource dies.

Bug: 972017, b/131272798
Test: Manually
Change-Id: I04dc594b23ce981f3443996be4a5ac3538770211
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1660394
Commit-Queue: Wei Lee <wtlee@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#669588}

third_party/blink/public/web/modules/mediastream/media_stream_video_source.h

@@ -273,7 +273,7 @@ class BLINK_MODULES_EXPORT MediaStreamVideoSource
   void StartFrameMonitoring();
   void UpdateTrackSettings(MediaStreamVideoTrack* track,
                            const VideoTrackAdapterSettings& adapter_settings);
-  void DidStopSource(base::OnceClosure callback, RestartResult result);
+  void DidStopSource(RestartResult result);
 
   State state_;
 
@@ -322,6 +322,11 @@ class BLINK_MODULES_EXPORT MediaStreamVideoSource
   // size.
   bool enable_device_rotation_detection_ = false;
 
+  // Callback that needs to trigger after removing the track. If this object
+  // died before this callback is resolved, we still need to trigger the
+  // callback to notify the caller that the request is canceled.
+  base::OnceClosure remove_last_track_callback_;
+
   // NOTE: Weak pointers must be invalidated before all other member variables.
   base::WeakPtrFactory<MediaStreamVideoSource> weak_factory_;
 

third_party/blink/renderer/modules/mediastream/media_stream_video_source.cc

@@ -44,6 +44,9 @@ MediaStreamVideoSource::MediaStreamVideoSource()
 
 MediaStreamVideoSource::~MediaStreamVideoSource() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (remove_last_track_callback_) {
+    std::move(remove_last_track_callback_).Run();
+  }
 }
 
 void MediaStreamVideoSource::AddTrack(
@@ -129,9 +132,9 @@ void MediaStreamVideoSource::RemoveTrack(MediaStreamVideoTrack* video_track,
       // stopping a source with StopSource() can have side effects that affect
       // sources created after that StopSource() call, but before the actual
       // stop takes place. See https://crbug.com/778039.
+      remove_last_track_callback_ = std::move(callback);
       StopForRestart(WTF::Bind(&MediaStreamVideoSource::DidStopSource,
-                               weak_factory_.GetWeakPtr(),
-                               std::move(callback)));
+                               weak_factory_.GetWeakPtr()));
       if (state_ == STOPPING_FOR_RESTART || state_ == STOPPED_FOR_RESTART) {
         // If the source supports restarting, it is necessary to call
         // FinalizeStopSource() to ensure the same behavior as StopSource(),
@@ -155,10 +158,9 @@ void MediaStreamVideoSource::RemoveTrack(MediaStreamVideoTrack* video_track,
   }
 }
 
-void MediaStreamVideoSource::DidStopSource(base::OnceClosure callback,
-                                           RestartResult result) {
+void MediaStreamVideoSource::DidStopSource(RestartResult result) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(callback);
+  DCHECK(remove_last_track_callback_);
   if (result == RestartResult::IS_STOPPED) {
     state_ = ENDED;
   }
@@ -171,7 +173,7 @@ void MediaStreamVideoSource::DidStopSource(base::OnceClosure callback,
                     "sending notification anyway";
     StopSource();
   }
-  std::move(callback).Run();
+  std::move(remove_last_track_callback_).Run();
 }
 
 void MediaStreamVideoSource::ReconfigureTrack(