Bug fix: stop mirroring session when tab is closed.

content/renderer/media/media_stream_video_capturer_source.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/media/media_stream_video_capturer_source.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
@@ skipping 215 matching lines @@
   // |session_id_| identifies the capture device used for this capture session.
   const media::VideoCaptureSessionId session_id_;
 
   VideoCaptureImplManager* const manager_;
 
   const base::Closure release_device_cb_;
 
   // Indicates if we are capturing generated content, e.g. Tab or Desktop.
   const bool is_content_capture_;
 
-  // This is run once to report whether the device was successfully started
-  // after a call to StartCapture().
+  // These two are valid between StartCapture() and StopCapture().
+  // |running_call_back_| is run when capture is successfully started, and when
+  // it is stopped or error happens.
   RunningCallback running_callback_;
-
-  // This is valid between StartCapture() and StopCapture().
   base::Closure stop_capture_cb_;
 
   // Placeholder keeping the callback between asynchronous device enumeration
   // calls.
   VideoCaptureDeviceFormatsCB formats_enumerated_callback_;
 
   // Bound to the main render thread.
   base::ThreadChecker thread_checker_;
 
   base::WeakPtrFactory<LocalVideoCapturerSource> weak_factory_;
@@ skipping 97 matching lines @@
   formats_enumerated_callback_.Reset();
 }
 
 void LocalVideoCapturerSource::OnStateUpdate(VideoCaptureState state) {
   DVLOG(3) << __func__ << " state = " << state;
   DCHECK(thread_checker_.CalledOnValidThread());
   if (running_callback_.is_null())
     return;
   switch (state) {
     case VIDEO_CAPTURE_STATE_STARTED:
-      base::ResetAndReturn(&running_callback_).Run(true);
+      running_callback_.Run(true);
       break;
 
     case VIDEO_CAPTURE_STATE_STOPPING:
     case VIDEO_CAPTURE_STATE_STOPPED:
     case VIDEO_CAPTURE_STATE_ERROR:
     case VIDEO_CAPTURE_STATE_ENDED:
       base::ResetAndReturn(&running_callback_).Run(false);
       break;
 
     case VIDEO_CAPTURE_STATE_PAUSED:
     case VIDEO_CAPTURE_STATE_RESUMED:
       // Not applicable to reporting on device start success/failure.

[miu, 2016/10/18 22:22:30]

nit: // Not applicable to reporting on device starts or errors.

[xjz, 2016/10/19 18:12:09]

Done.

       break;
   }
 }
 
 void LocalVideoCapturerSource::OnDeviceFormatsInUseReceived(
     const media::VideoCaptureFormats& formats_in_use) {
   DVLOG(3) << __func__ << ", #formats received: " << formats_in_use.size();
   DCHECK(thread_checker_.CalledOnValidThread());
   // StopCapture() might have destroyed |formats_enumerated_callback_| before
   // arriving here.
@@ skipping 104 matching lines @@
   source_->StartCapture(new_params,
                           frame_callback,
                           base::Bind(&MediaStreamVideoCapturerSource::OnStarted,
                                      base::Unretained(this)));
 }
 
 void MediaStreamVideoCapturerSource::StopSourceImpl() {
   source_->StopCapture();
 }
 
 void MediaStreamVideoCapturerSource::OnStarted(bool result) {

[miu, 2016/10/18 22:22:30]

The original bug was most likely caused because this method is mis-named (or
maybe it was re-purposed but without also making a name change). Can you please
fix this?

Suggestion: void OnRunStateChanged(bool is_running)

[xjz, 2016/10/19 18:12:09]

Done.

   OnStartDone(result ? MEDIA_DEVICE_OK : MEDIA_DEVICE_TRACK_START_FAILURE);

[miu, 2016/10/18 22:22:30]

This is also misleading, but I think there's more than a naming problem here. It
seems like this whole method should be:

  void MediaStreamVideoCapturerSource::OnRunStateChanged(bool is_running) {
    if (is_capture_starting_) {
      // Note: |is_capture_starting_| was set to true by StartSourceImpl().
      OnStartDone(is_running ? MEDIA_DEVICE_OK :
MEDIA_DEVICE_TRACK_START_FAILURE);
      is_capture_starting_ = false;
    } else if (!is_running) {
      StopSource();
    }
  }

[xjz, 2016/10/19 18:12:09]

Done.

 }
 
 const char*
 MediaStreamVideoCapturerSource::GetPowerLineFrequencyForTesting() const {
   return kPowerLineFrequency;
 }
 
 }  // namespace content