Make sure ReadyStateEnded is fired when video stream stops

A regression was introduced in the refactoring change in r269020.
VideoCaptureDelegate::started_callback_ is used to notify that the
stream has started and ran into error. However in change r269020 the
callback was reset after sending the started signal. This prevented
error signal from firing.

In this change |started_callback_| is renamed to |running_callback| to
avoid confusion. Also added a test to verify that ReadyStateEnded is
reached when a camera stops.

BUG=374495

Review URL: https://codereview.chromium.org/295483002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271359 0039d316-1c4b-4281-b951-d872f2087c98

content/renderer/media/media_stream_video_capture_source_unittest.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/bind.h"
+#include "base/message_loop/message_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/child/child_process.h"
 #include "content/renderer/media/media_stream_video_capturer_source.h"
@@ -21,8 +22,8 @@ class MockVideoCapturerDelegate : public VideoCapturerDelegate {
   MOCK_METHOD3(StartCapture,
                void(const media::VideoCaptureParams& params,
                     const VideoCaptureDeliverFrameCB& new_frame_callback,
-                    const StartedCallback& started_callback));
-  MOCK_METHOD0(StopCapture,void());
+                    const RunningCallback& running_callback));
+  MOCK_METHOD0(StopCapture, void());
 
  private:
   virtual ~MockVideoCapturerDelegate() {}
@@ -60,14 +61,19 @@ class MediaStreamVideoCapturerSourceTest : public testing::Test {
             enabled);
   }
 
+  MockVideoCapturerDelegate& mock_delegate() {
+    return *static_cast<MockVideoCapturerDelegate*>(delegate_.get());
+  }
+
  protected:
   void OnConstraintsApplied(MediaStreamSource* source, bool success) {
   }
 
+  base::MessageLoop message_loop_;
   scoped_ptr<ChildProcess> child_process_;
   blink::WebMediaStreamSource webkit_source_;
   MediaStreamVideoCapturerSource* source_;  // owned by webkit_source.
-  scoped_refptr<MockVideoCapturerDelegate> delegate_;
+  scoped_refptr<VideoCapturerDelegate> delegate_;
 };
 
 TEST_F(MediaStreamVideoCapturerSourceTest, TabCaptureAllowResolutionChange) {
@@ -75,13 +81,13 @@ TEST_F(MediaStreamVideoCapturerSourceTest, TabCaptureAllowResolutionChange) {
   device_info.device.type = MEDIA_TAB_VIDEO_CAPTURE;
   InitWithDeviceInfo(device_info);
 
-  EXPECT_CALL(*delegate_, StartCapture(
+  EXPECT_CALL(mock_delegate(), StartCapture(
       testing::Field(&media::VideoCaptureParams::allow_resolution_change, true),
       testing::_,
       testing::_)).Times(1);
   blink::WebMediaStreamTrack track = StartSource();
   // When the track goes out of scope, the source will be stopped.
-  EXPECT_CALL(*delegate_, StopCapture());
+  EXPECT_CALL(mock_delegate(), StopCapture());
 }
 
 TEST_F(MediaStreamVideoCapturerSourceTest,
@@ -90,13 +96,39 @@ TEST_F(MediaStreamVideoCapturerSourceTest,
   device_info.device.type = MEDIA_DESKTOP_VIDEO_CAPTURE;
   InitWithDeviceInfo(device_info);
 
-  EXPECT_CALL(*delegate_, StartCapture(
+  EXPECT_CALL(mock_delegate(), StartCapture(
       testing::Field(&media::VideoCaptureParams::allow_resolution_change, true),
       testing::_,
       testing::_)).Times(1);
   blink::WebMediaStreamTrack track = StartSource();
   // When the track goes out of scope, the source will be stopped.
-  EXPECT_CALL(*delegate_, StopCapture());
+  EXPECT_CALL(mock_delegate(), StopCapture());
+}
+
+TEST_F(MediaStreamVideoCapturerSourceTest, Ended) {
+  StreamDeviceInfo device_info;
+  device_info.device.type = MEDIA_DESKTOP_VIDEO_CAPTURE;
+  delegate_ = new VideoCapturerDelegate(device_info);
+  source_ = new MediaStreamVideoCapturerSource(
+      device_info,
+      MediaStreamSource::SourceStoppedCallback(),
+      delegate_);
+  webkit_source_.initialize(base::UTF8ToUTF16("dummy_source_id"),
+                            blink::WebMediaStreamSource::TypeVideo,
+                            base::UTF8ToUTF16("dummy_source_name"));
+  webkit_source_.setExtraData(source_);
+  blink::WebMediaStreamTrack track = StartSource();
+  message_loop_.RunUntilIdle();
+
+  delegate_->OnStateUpdateOnRenderThread(VIDEO_CAPTURE_STATE_STARTED);
+  message_loop_.RunUntilIdle();
+  EXPECT_EQ(blink::WebMediaStreamSource::ReadyStateLive,
+            webkit_source_.readyState());
+
+  delegate_->OnStateUpdateOnRenderThread(VIDEO_CAPTURE_STATE_ERROR);
+  message_loop_.RunUntilIdle();
+  EXPECT_EQ(blink::WebMediaStreamSource::ReadyStateEnded,
+            webkit_source_.readyState());
 }
 
 }  // namespace content

content/renderer/media/media_stream_video_capturer_source.cc

@@ -98,10 +98,10 @@ void VideoCapturerDelegate::GetCurrentSupportedFormats(
 void VideoCapturerDelegate::StartCapture(
     const media::VideoCaptureParams& params,
     const VideoCaptureDeliverFrameCB& new_frame_callback,
-    const StartedCallback& started_callback) {
+    const RunningCallback& running_callback) {
   DCHECK(params.requested_format.IsValid());
   DCHECK(thread_checker_.CalledOnValidThread());
-  started_callback_ = started_callback;
+  running_callback_ = running_callback;
   got_first_frame_ = false;
 
   // NULL in unit test.
@@ -127,7 +127,7 @@ void VideoCapturerDelegate::StopCapture() {
   if (!stop_capture_cb_.is_null()) {
     base::ResetAndReturn(&stop_capture_cb_).Run();
   }
-  started_callback_.Reset();
+  running_callback_.Reset();
   source_formats_callback_.Reset();
 }
 
@@ -135,9 +135,12 @@ void VideoCapturerDelegate::OnStateUpdateOnRenderThread(
     VideoCaptureState state) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(3) << "OnStateUpdateOnRenderThread state = " << state;
-  if (state > VIDEO_CAPTURE_STATE_STARTING && !started_callback_.is_null()) {
-    base::ResetAndReturn(&started_callback_).Run(
-        state == VIDEO_CAPTURE_STATE_STARTED);
+  if (state == VIDEO_CAPTURE_STATE_STARTED && !running_callback_.is_null()) {
+    running_callback_.Run(true);
+    return;
+  }
+  if (state > VIDEO_CAPTURE_STATE_STARTED && !running_callback_.is_null()) {
+    base::ResetAndReturn(&running_callback_).Run(false);
   }
 }
 

content/renderer/media/media_stream_video_capturer_source.h

@@ -6,6 +6,7 @@
 #define CONTENT_RENDERER_MEDIA_MEDIA_STREAM_VIDEO_CAPTURER_SOURCE_H_
 
 #include "base/callback.h"
+#include "base/gtest_prod_util.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/threading/thread_checker.h"
 #include "content/common/media/video_capture.h"
@@ -21,7 +22,7 @@ namespace content {
 class CONTENT_EXPORT VideoCapturerDelegate
     : public base::RefCountedThreadSafe<VideoCapturerDelegate> {
  public:
-  typedef base::Callback<void(bool running)> StartedCallback;
+  typedef base::Callback<void(bool running)> RunningCallback;
 
   explicit VideoCapturerDelegate(
       const StreamDeviceInfo& device_info);
@@ -37,18 +38,21 @@ class CONTENT_EXPORT VideoCapturerDelegate
 
   // Starts capturing frames using the resolution in |params|.
   // |new_frame_callback| is triggered when a new video frame is available.
-  // |started_callback| is triggered before the first video frame is received
-  // or if the underlying video capturer fails to start.
+  // If capturing is started successfully then |running_callback| will be
+  // called with a parameter of true.
+  // If capturing fails to start or stopped due to an external event then
+  // |running_callback| will be called with a parameter of false.
   virtual void StartCapture(
       const media::VideoCaptureParams& params,
       const VideoCaptureDeliverFrameCB& new_frame_callback,
-      const StartedCallback& started_callback);
+      const RunningCallback& running_callback);
 
   // Stops capturing frames and clears all callbacks including the
   // SupportedFormatsCallback callback.
   virtual void StopCapture();
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(MediaStreamVideoCapturerSourceTest, Ended);
   friend class base::RefCountedThreadSafe<VideoCapturerDelegate>;
   friend class MockVideoCapturerDelegate;
 
@@ -68,9 +72,9 @@ class CONTENT_EXPORT VideoCapturerDelegate
   bool is_screen_cast_;
   bool got_first_frame_;
 
-  // |started_callback| is provided to this class in StartCapture and must be
+  // |running_callback| is provided to this class in StartCapture and must be
   // valid until StopCapture is called.
-  StartedCallback started_callback_;
+  RunningCallback running_callback_;
 
   VideoCaptureDeviceFormatsCB source_formats_callback_;