Do not stop MediaStreamVideoSource for good after a failed restart.

After a successful StopForRestart(), if Restart() fails, the source
should return to the previous state, which is STOPPED_FOR_RESTART,
instead of being stopped for good.

Bug: 763319
Change-Id: I907c320d0d4664d3b35fe89d141eb936381e78c7
Reviewed-on: https://chromium-review.googlesource.com/695306
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506785}

content/renderer/media/media_stream_video_source.cc

@@ -190,8 +190,7 @@ void MediaStreamVideoSource::OnRestartDone(bool did_restart) {
     StartFrameMonitoring();
     FinalizeAddPendingTracks();
   } else {
-    StopSource();
-    return;
+    state_ = STOPPED_FOR_RESTART;
   }
 
   RestartResult result =

content/renderer/media/media_stream_video_source_unittest.cc

@@ -48,6 +48,7 @@ class MediaStreamVideoSourceTest : public ::testing::Test {
                                       1000.0,
                                       media::PIXEL_FORMAT_I420),
             false)) {
+    mock_source_->DisableStopForRestart();
     media::VideoCaptureFormats formats;
     formats.push_back(media::VideoCaptureFormat(gfx::Size(1280, 720), 30,
                                                 media::PIXEL_FORMAT_I420));
@@ -109,8 +110,6 @@ class MediaStreamVideoSourceTest : public ::testing::Test {
 
     EXPECT_EQ(0, NumberOfSuccessConstraintsCallbacks());
     mock_source_->StartMockedSource();
-    // The mock source is not a capturer source.
-    EXPECT_FALSE(mock_source_->GetCurrentCaptureParams().has_value());
     // Once the source has started successfully we expect that the
     // ConstraintsCallback in MediaStreamSource::AddTrack completes.
     EXPECT_EQ(1, NumberOfSuccessConstraintsCallbacks());
@@ -540,4 +539,102 @@ TEST_F(MediaStreamVideoSourceTest, FailedRestart) {
       }));
 }
 
+// Test that restart succeeds on a source with restart support.
+TEST_F(MediaStreamVideoSourceTest, SuccessfulRestart) {
+  blink::WebMediaStreamTrack track = CreateTrack("123");
+  mock_source()->EnableStopForRestart();
+  mock_source()->EnableRestart();
+  mock_source()->StartMockedSource();
+  EXPECT_EQ(NumberOfSuccessConstraintsCallbacks(), 1);
+  EXPECT_EQ(track.Source().GetReadyState(),
+            blink::WebMediaStreamSource::kReadyStateLive);
+
+  mock_source()->StopForRestart(
+      base::BindOnce([](MediaStreamVideoSource::RestartResult result) {
+        EXPECT_EQ(result, MediaStreamVideoSource::RestartResult::IS_STOPPED);
+      }));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(track.Source().GetReadyState(),
+            blink::WebMediaStreamSource::kReadyStateLive);
+
+  // Verify that StopForRestart() fails with INVALID_STATE called after the
+  // source is already stopped.
+  mock_source()->StopForRestart(
+      base::BindOnce([](MediaStreamVideoSource::RestartResult result) {
+        EXPECT_EQ(result, MediaStreamVideoSource::RestartResult::INVALID_STATE);
+      }));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(track.Source().GetReadyState(),
+            blink::WebMediaStreamSource::kReadyStateLive);
+
+  mock_source()->Restart(
+      media::VideoCaptureFormat(),
+      base::BindOnce([](MediaStreamVideoSource::RestartResult result) {
+        EXPECT_EQ(result, MediaStreamVideoSource::RestartResult::IS_RUNNING);
+      }));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(track.Source().GetReadyState(),
+            blink::WebMediaStreamSource::kReadyStateLive);
+
+  // Verify that Restart() fails with INVALID_STATE if the source has already
+  // started.
+  mock_source()->Restart(
+      media::VideoCaptureFormat(),
+      base::BindOnce([](MediaStreamVideoSource::RestartResult result) {
+        EXPECT_EQ(result, MediaStreamVideoSource::RestartResult::INVALID_STATE);
+      }));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(track.Source().GetReadyState(),
+            blink::WebMediaStreamSource::kReadyStateLive);
+
+  mock_source()->StopSource();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(track.Source().GetReadyState(),
+            blink::WebMediaStreamSource::kReadyStateEnded);
+}
+
+// Test that restart fails on a source without restart support.
+TEST_F(MediaStreamVideoSourceTest, FailedRestartAfterStopForRestart) {
+  blink::WebMediaStreamTrack track = CreateTrack("123");
+  mock_source()->EnableStopForRestart();
+  mock_source()->DisableRestart();
+  mock_source()->StartMockedSource();
+  EXPECT_EQ(NumberOfSuccessConstraintsCallbacks(), 1);
+  EXPECT_EQ(track.Source().GetReadyState(),
+            blink::WebMediaStreamSource::kReadyStateLive);
+
+  mock_source()->StopForRestart(
+      base::BindOnce([](MediaStreamVideoSource::RestartResult result) {
+        EXPECT_EQ(result, MediaStreamVideoSource::RestartResult::IS_STOPPED);
+      }));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(track.Source().GetReadyState(),
+            blink::WebMediaStreamSource::kReadyStateLive);
+
+  mock_source()->Restart(
+      media::VideoCaptureFormat(),
+      base::BindOnce([](MediaStreamVideoSource::RestartResult result) {
+        EXPECT_EQ(result, MediaStreamVideoSource::RestartResult::IS_STOPPED);
+      }));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(track.Source().GetReadyState(),
+            blink::WebMediaStreamSource::kReadyStateLive);
+
+  // Another failed attempt to verify that the source remains in the correct
+  // state.
+  mock_source()->Restart(
+      media::VideoCaptureFormat(),
+      base::BindOnce([](MediaStreamVideoSource::RestartResult result) {
+        EXPECT_EQ(result, MediaStreamVideoSource::RestartResult::IS_STOPPED);
+      }));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(track.Source().GetReadyState(),
+            blink::WebMediaStreamSource::kReadyStateLive);
+
+  mock_source()->StopSource();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(track.Source().GetReadyState(),
+            blink::WebMediaStreamSource::kReadyStateEnded);
+}
+
 }  // namespace content

content/renderer/media/mock_media_stream_video_source.cc

@@ -72,11 +72,37 @@ MockMediaStreamVideoSource::GetCurrentFormat() const {
   return base::Optional<media::VideoCaptureFormat>(format_);
 }
 
+base::Optional<media::VideoCaptureParams>
+MockMediaStreamVideoSource::GetCurrentCaptureParams() const {
+  media::VideoCaptureParams params;
+  params.requested_format = format_;
+  return params;
+}
+
 void MockMediaStreamVideoSource::DeliverVideoFrame(
     const scoped_refptr<media::VideoFrame>& frame) {
+  DCHECK(!is_stopped_for_restart_);
   DCHECK(!frame_callback_.is_null());
   io_task_runner()->PostTask(
       FROM_HERE, base::BindOnce(frame_callback_, frame, base::TimeTicks()));
 }
 
+void MockMediaStreamVideoSource::StopSourceForRestartImpl() {
+  if (can_stop_for_restart_)
+    is_stopped_for_restart_ = true;
+  OnStopForRestartDone(is_stopped_for_restart_);
+}
+
+void MockMediaStreamVideoSource::RestartSourceImpl(
+    const media::VideoCaptureFormat& new_format) {
+  DCHECK(is_stopped_for_restart_);
+  if (!can_restart_) {
+    OnRestartDone(false);
+    return;
+  }
+  is_stopped_for_restart_ = false;
+  format_ = new_format;
+  OnRestartDone(true);
+}
+
 }  // namespace content

content/renderer/media/mock_media_stream_video_source.h

@@ -37,8 +37,6 @@ class MockMediaStreamVideoSource : public MediaStreamVideoSource {
   // frame has been delivered.
   void DeliverVideoFrame(const scoped_refptr<media::VideoFrame>& frame);
 
-  void CompleteGetSupportedFormats();
-
   const media::VideoCaptureFormat& start_format() const { return format_; }
   int max_requested_height() const { return max_requested_height_; }
   int max_requested_width() const { return max_requested_width_; }
@@ -49,8 +47,16 @@ class MockMediaStreamVideoSource : public MediaStreamVideoSource {
     DoSetMutedState(muted_state);
   }
 
+  void EnableStopForRestart() { can_stop_for_restart_ = true; }
+  void DisableStopForRestart() { can_stop_for_restart_ = false; }
+
+  void EnableRestart() { can_restart_ = true; }
+  void DisableRestart() { can_restart_ = false; }
+
   // Implements MediaStreamVideoSource.
   void RequestRefreshFrame() override;
+  base::Optional<media::VideoCaptureParams> GetCurrentCaptureParams()
+      const override;
 
  protected:
   // Implements MediaStreamVideoSource.
@@ -58,6 +64,8 @@ class MockMediaStreamVideoSource : public MediaStreamVideoSource {
       const VideoCaptureDeliverFrameCB& frame_callback) override;
   void StopSourceImpl() override;
   base::Optional<media::VideoCaptureFormat> GetCurrentFormat() const override;
+  void StopSourceForRestartImpl() override;
+  void RestartSourceImpl(const media::VideoCaptureFormat& new_format) override;
 
  private:
   media::VideoCaptureFormat format_;
@@ -66,6 +74,9 @@ class MockMediaStreamVideoSource : public MediaStreamVideoSource {
   int max_requested_width_;
   double max_requested_frame_rate_;
   bool attempted_to_start_;
+  bool is_stopped_for_restart_ = false;
+  bool can_stop_for_restart_ = true;
+  bool can_restart_ = true;
   VideoCaptureDeliverFrameCB frame_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(MockMediaStreamVideoSource);