Fix races in MediaStream video constraints tests.

content/renderer/media/media_stream_video_capturer_source_unittest.cc

Index: content/renderer/media/media_stream_video_capturer_source_unittest.cc
diff --git a/content/renderer/media/media_stream_video_capturer_source_unittest.cc b/content/renderer/media/media_stream_video_capturer_source_unittest.cc
index bdae3481b36f5b0c3e77428376dcffc2bbbc142e..b33d5e2cb725a0c3d04a722eaac331d22f475710 100644
--- a/content/renderer/media/media_stream_video_capturer_source_unittest.cc
+++ b/content/renderer/media/media_stream_video_capturer_source_unittest.cc
@@ -31,6 +31,8 @@ using ::testing::WithArgs;
 
 namespace content {
 
+namespace {
+
 class MockVideoCapturerSource : public media::VideoCapturerSource {
  public:
   MockVideoCapturerSource() {
@@ -64,13 +66,51 @@ class MockVideoCapturerSource : public media::VideoCapturerSource {
   }
 };
 
+class FakeMediaStreamVideoSink : public MediaStreamVideoSink {
+ public:
+  FakeMediaStreamVideoSink(base::TimeTicks* capture_time,
+                           media::VideoFrameMetadata* metadata,
+                           base::Closure got_frame_cb)
+      : capture_time_(capture_time),
+        metadata_(metadata),
+        got_frame_cb_(got_frame_cb) {}
+
+  void ConnectToTrack(const blink::WebMediaStreamTrack& track) {
+    MediaStreamVideoSink::ConnectToTrack(
+        track,
+        base::Bind(&FakeMediaStreamVideoSink::OnVideoFrame,
+                   base::Unretained(this)),
+        true);
+  }
+
+  void DisconnectFromTrack() { MediaStreamVideoSink::DisconnectFromTrack(); }
+
+  void OnVideoFrame(const scoped_refptr<media::VideoFrame>& frame,
+                    base::TimeTicks capture_time) {
+    *capture_time_ = capture_time;
+    metadata_->Clear();
+    metadata_->MergeMetadataFrom(frame->metadata());
+    base::ResetAndReturn(&got_frame_cb_).Run();
+  }
+
+ private:
+  base::TimeTicks* const capture_time_;
+  media::VideoFrameMetadata* const metadata_;
+  base::Closure got_frame_cb_;
+};
+
+}  // namespace
+
 class MediaStreamVideoCapturerSourceTest : public testing::Test {
  public:
   MediaStreamVideoCapturerSourceTest()
       : child_process_(new ChildProcess()),
         source_(nullptr),
         delegate_(nullptr),
-        source_stopped_(false) {}
+        source_stopped_(false) {
+    scoped_feature_list_.InitAndDisableFeature(
+        features::kMediaStreamOldVideoConstraints);
+  }
 
   void TearDown() override {
     webkit_source_.reset();
@@ -150,15 +190,197 @@ class MediaStreamVideoCapturerSourceTest : public testing::Test {
   blink::WebString webkit_source_id_;
   bool source_stopped_;
   MockConstraintFactory constraint_factory_;
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+TEST_F(MediaStreamVideoCapturerSourceTest, StartAndStop) {
+  std::unique_ptr<MockVideoCapturerSource> delegate(
+      new MockVideoCapturerSource());
+  delegate_ = delegate.get();
+  EXPECT_CALL(*delegate_, GetPreferredFormats());
+  source_ = new MediaStreamVideoCapturerSource(
+      base::Bind(&MediaStreamVideoCapturerSourceTest::OnSourceStopped,
+                 base::Unretained(this)),
+      std::move(delegate));
+  webkit_source_.initialize(blink::WebString::fromASCII("dummy_source_id"),
+                            blink::WebMediaStreamSource::TypeVideo,
+                            blink::WebString::fromASCII("dummy_source_name"),
+                            false /* remote */);
+  webkit_source_.setExtraData(source_);
+  webkit_source_id_ = webkit_source_.id();
+
+  InSequence s;
+  EXPECT_CALL(mock_delegate(), StartCapture(_, _, _));
+  blink::WebMediaStreamTrack track = StartSource(
+      VideoTrackAdapterSettings(), base::Optional<bool>(), false, 0.0);
+  base::RunLoop().RunUntilIdle();
+
+  OnStarted(true);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(blink::WebMediaStreamSource::ReadyStateLive,
+            webkit_source_.getReadyState());
+
+  EXPECT_FALSE(source_stopped_);
+
+  EXPECT_CALL(mock_delegate(), StopCapture());
+  OnStarted(false);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(blink::WebMediaStreamSource::ReadyStateEnded,
+            webkit_source_.getReadyState());
+  // Verify that MediaStreamSource::SourceStoppedCallback has been triggered.
+  EXPECT_TRUE(source_stopped_);
+}
+
+TEST_F(MediaStreamVideoCapturerSourceTest, CaptureTimeAndMetadataPlumbing) {
+  std::unique_ptr<MockVideoCapturerSource> delegate(
+      new MockVideoCapturerSource());
+  delegate_ = delegate.get();
+  EXPECT_CALL(*delegate_, GetPreferredFormats());
+  source_ = new MediaStreamVideoCapturerSource(
+      base::Bind(&MediaStreamVideoCapturerSourceTest::OnSourceStopped,
+                 base::Unretained(this)),
+      std::move(delegate));
+  webkit_source_.initialize(blink::WebString::fromASCII("dummy_source_id"),
+                            blink::WebMediaStreamSource::TypeVideo,
+                            blink::WebString::fromASCII("dummy_source_name"),
+                            false /* remote */);
+  webkit_source_.setExtraData(source_);
+  webkit_source_id_ = webkit_source_.id();
+
+  VideoCaptureDeliverFrameCB deliver_frame_cb;
+  media::VideoCapturerSource::RunningCallback running_cb;
+
+  InSequence s;
+  //  EXPECT_CALL(mock_delegate(), GetCurrentSupportedFormats(_, _, _, _));
+  EXPECT_CALL(mock_delegate(), StartCapture(_, _, _))
+      .WillOnce(testing::DoAll(testing::SaveArg<1>(&deliver_frame_cb),
+                               testing::SaveArg<2>(&running_cb)));
+  EXPECT_CALL(mock_delegate(), RequestRefreshFrame());
+  EXPECT_CALL(mock_delegate(), StopCapture());
+  blink::WebMediaStreamTrack track = StartSource(
+      VideoTrackAdapterSettings(), base::Optional<bool>(), false, 0.0);
+  running_cb.Run(true);
+
+  base::RunLoop run_loop;
+  base::TimeTicks reference_capture_time =
+      base::TimeTicks::FromInternalValue(60013);
+  base::TimeTicks capture_time;
+  media::VideoFrameMetadata metadata;
+  FakeMediaStreamVideoSink fake_sink(
+      &capture_time, &metadata,
+      media::BindToCurrentLoop(run_loop.QuitClosure()));
+  fake_sink.ConnectToTrack(track);
+  const scoped_refptr<media::VideoFrame> frame =
+      media::VideoFrame::CreateBlackFrame(gfx::Size(2, 2));
+  frame->metadata()->SetDouble(media::VideoFrameMetadata::FRAME_RATE, 30.0);
+  child_process_->io_task_runner()->PostTask(
+      FROM_HERE, base::Bind(deliver_frame_cb, frame, reference_capture_time));
+  run_loop.Run();
+  fake_sink.DisconnectFromTrack();
+  EXPECT_EQ(reference_capture_time, capture_time);
+  double metadata_value;
+  EXPECT_TRUE(metadata.GetDouble(media::VideoFrameMetadata::FRAME_RATE,
+                                 &metadata_value));
+  EXPECT_EQ(30.0, metadata_value);
+}
+
+class MediaStreamVideoCapturerSourceOldConstraintsTest : public testing::Test {
+ public:
+  MediaStreamVideoCapturerSourceOldConstraintsTest()
+      : child_process_(new ChildProcess()),
+        source_(nullptr),
+        delegate_(nullptr),
+        source_stopped_(false) {
+    scoped_feature_list_.InitAndEnableFeature(
+        features::kMediaStreamOldVideoConstraints);
+  }
+
+  void TearDown() override {
+    webkit_source_.reset();
+    blink::WebHeap::collectAllGarbageForTesting();
+  }
+
+  void InitWithDeviceInfo(const StreamDeviceInfo& device_info) {
+    std::unique_ptr<MockVideoCapturerSource> delegate(
+        new MockVideoCapturerSource());
+    delegate_ = delegate.get();
+    source_ = new MediaStreamVideoCapturerSource(
+        base::Bind(
+            &MediaStreamVideoCapturerSourceOldConstraintsTest::OnSourceStopped,
+            base::Unretained(this)),
+        std::move(delegate));
+    source_->SetDeviceInfo(device_info);
+
+    webkit_source_.initialize(blink::WebString::fromASCII("dummy_source_id"),
+                              blink::WebMediaStreamSource::TypeVideo,
+                              blink::WebString::fromASCII("dummy_source_name"),
+                              false /* remote */);
+    webkit_source_.setExtraData(source_);
+    webkit_source_id_ = webkit_source_.id();
+  }
+
+  MockConstraintFactory* constraint_factory() { return &constraint_factory_; }
+
+  blink::WebMediaStreamTrack StartSource() {
+    bool enabled = true;
+    // CreateVideoTrack will trigger OnConstraintsApplied.
+    return MediaStreamVideoTrack::CreateVideoTrack(
+        source_, constraint_factory_.CreateWebMediaConstraints(),
+        base::Bind(&MediaStreamVideoCapturerSourceOldConstraintsTest::
+                       OnConstraintsApplied,
+                   base::Unretained(this)),
+        enabled);
+  }
+
+  blink::WebMediaStreamTrack StartSource(
+      const VideoTrackAdapterSettings& adapter_settings,
+      const base::Optional<bool>& noise_reduction,
+      bool is_screencast,
+      double min_frame_rate) {
+    bool enabled = true;
+    // CreateVideoTrack will trigger OnConstraintsApplied.
+    return MediaStreamVideoTrack::CreateVideoTrack(
+        source_, adapter_settings, noise_reduction, is_screencast,
+        min_frame_rate,
+        base::Bind(&MediaStreamVideoCapturerSourceOldConstraintsTest::
+                       OnConstraintsApplied,
+                   base::Unretained(this)),
+        enabled);
+  }
+
+  MockVideoCapturerSource& mock_delegate() { return *delegate_; }
+
+  const char* GetPowerLineFrequencyForTesting() const {
+    return source_->GetPowerLineFrequencyForTesting();
+  }
+
+  void OnSourceStopped(const blink::WebMediaStreamSource& source) {
+    source_stopped_ = true;
+    EXPECT_EQ(source.id(), webkit_source_id_);
+  }
+  void OnStarted(bool result) { source_->OnRunStateChanged(result); }
+
+ protected:
+  void OnConstraintsApplied(MediaStreamSource* source,
+                            MediaStreamRequestResult result,
+                            const blink::WebString& result_name) {}
+
+  // A ChildProcess and a MessageLoopForUI are both needed to fool the Tracks
+  // and Sources below into believing they are on the right threads.
+  base::MessageLoopForUI message_loop_;
+  std::unique_ptr<ChildProcess> child_process_;
+
+  blink::WebMediaStreamSource webkit_source_;
+  MediaStreamVideoCapturerSource* source_;  // owned by |webkit_source_|.
+  MockVideoCapturerSource* delegate_;       // owned by |source|.
+  blink::WebString webkit_source_id_;
+  bool source_stopped_;
+  MockConstraintFactory constraint_factory_;
+  base::test::ScopedFeatureList scoped_feature_list_;
 };
 
-// This test does not apply with spec-compliant constraints.
-// TODO(guidou): Remove this test. http://crbug.com/706408
-TEST_F(MediaStreamVideoCapturerSourceTest,
-       TabCaptureFixedResolutionByDefaultOldConstraints) {
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeature(
-      features::kMediaStreamOldVideoConstraints);
+TEST_F(MediaStreamVideoCapturerSourceOldConstraintsTest,
+       TabCaptureFixedResolutionByDefault) {
   StreamDeviceInfo device_info;
   device_info.device.type = MEDIA_TAB_VIDEO_CAPTURE;
   InitWithDeviceInfo(device_info);
@@ -183,13 +405,8 @@ TEST_F(MediaStreamVideoCapturerSourceTest,
   EXPECT_CALL(mock_delegate(), StopCapture());
 }
 
-// This test does not apply with spec-compliant constraints.
-// TODO(guidou): Remove this test. http://crbug.com/706408
-TEST_F(MediaStreamVideoCapturerSourceTest,
-       DesktopCaptureAllowAnyResolutionChangeByDefaultOldConstraints) {
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeature(
-      features::kMediaStreamOldVideoConstraints);
+TEST_F(MediaStreamVideoCapturerSourceOldConstraintsTest,
+       DesktopCaptureAllowAnyResolutionChangeByDefault) {
   StreamDeviceInfo device_info;
   device_info.device.type = MEDIA_DESKTOP_VIDEO_CAPTURE;
   InitWithDeviceInfo(device_info);
@@ -214,13 +431,8 @@ TEST_F(MediaStreamVideoCapturerSourceTest,
   EXPECT_CALL(mock_delegate(), StopCapture());
 }
 
-// This test does not apply with spec-compliant constraints.
-// TODO(guidou): Remove this test. http://crbug.com/706408
-TEST_F(MediaStreamVideoCapturerSourceTest,
-       TabCaptureConstraintsImplyFixedAspectRatioOldConstraints) {
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeature(
-      features::kMediaStreamOldVideoConstraints);
+TEST_F(MediaStreamVideoCapturerSourceOldConstraintsTest,
+       TabCaptureConstraintsImplyFixedAspectRatio) {
   StreamDeviceInfo device_info;
   device_info.device.type = MEDIA_TAB_VIDEO_CAPTURE;
   InitWithDeviceInfo(device_info);
@@ -252,13 +464,8 @@ TEST_F(MediaStreamVideoCapturerSourceTest,
   EXPECT_CALL(mock_delegate(), StopCapture());
 }
 
-// This test does not apply with spec-compliant constraints.
-// TODO(guidou): Remove this test. http://crbug.com/706408
-TEST_F(MediaStreamVideoCapturerSourceTest,
-       TabCaptureConstraintsImplyAllowingAnyResolutionChangeOldConstraints) {
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeature(
-      features::kMediaStreamOldVideoConstraints);
+TEST_F(MediaStreamVideoCapturerSourceOldConstraintsTest,
+       TabCaptureConstraintsImplyAllowingAnyResolutionChange) {
   StreamDeviceInfo device_info;
   device_info.device.type = MEDIA_TAB_VIDEO_CAPTURE;
   InitWithDeviceInfo(device_info);
@@ -290,13 +497,8 @@ TEST_F(MediaStreamVideoCapturerSourceTest,
   EXPECT_CALL(mock_delegate(), StopCapture());
 }
 
-// This test does not apply with spec-compliant constraints.
-// TODO(guidou): Remove this test. http://crbug.com/706408
-TEST_F(MediaStreamVideoCapturerSourceTest,
-       DeviceCaptureConstraintsSupportPowerLineFrequencyOldConstraints) {
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeature(
-      features::kMediaStreamOldVideoConstraints);
+TEST_F(MediaStreamVideoCapturerSourceOldConstraintsTest,
+       DeviceCaptureConstraintsSupportPowerLineFrequency) {
   for (int frequency = -100; frequency < 100; ++frequency) {
     StreamDeviceInfo device_info;
     device_info.device.type = MEDIA_DEVICE_VIDEO_CAPTURE;
@@ -335,17 +537,14 @@ TEST_F(MediaStreamVideoCapturerSourceTest,
   }
 }
 
-// TODO(guidou): Remove this test. http://crbug.com/706408
-TEST_F(MediaStreamVideoCapturerSourceTest, EndedOldConstraints) {
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeature(
-      features::kMediaStreamOldVideoConstraints);
+TEST_F(MediaStreamVideoCapturerSourceOldConstraintsTest, Ended) {
   std::unique_ptr<MockVideoCapturerSource> delegate(
       new MockVideoCapturerSource());
   delegate_ = delegate.get();
   source_ = new MediaStreamVideoCapturerSource(
-      base::Bind(&MediaStreamVideoCapturerSourceTest::OnSourceStopped,
-                 base::Unretained(this)),
+      base::Bind(
+          &MediaStreamVideoCapturerSourceOldConstraintsTest::OnSourceStopped,
+          base::Unretained(this)),
       std::move(delegate));
   webkit_source_.initialize(blink::WebString::fromASCII("dummy_source_id"),
                             blink::WebMediaStreamSource::TypeVideo,
@@ -376,86 +575,8 @@ TEST_F(MediaStreamVideoCapturerSourceTest, EndedOldConstraints) {
   EXPECT_TRUE(source_stopped_);
 }
 
-TEST_F(MediaStreamVideoCapturerSourceTest, StartAndStop) {
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndDisableFeature(
-      features::kMediaStreamOldVideoConstraints);
-  std::unique_ptr<MockVideoCapturerSource> delegate(
-      new MockVideoCapturerSource());
-  delegate_ = delegate.get();
-  EXPECT_CALL(*delegate_, GetPreferredFormats());
-  source_ = new MediaStreamVideoCapturerSource(
-      base::Bind(&MediaStreamVideoCapturerSourceTest::OnSourceStopped,
-                 base::Unretained(this)),
-      std::move(delegate));
-  webkit_source_.initialize(blink::WebString::fromASCII("dummy_source_id"),
-                            blink::WebMediaStreamSource::TypeVideo,
-                            blink::WebString::fromASCII("dummy_source_name"),
-                            false /* remote */);
-  webkit_source_.setExtraData(source_);
-  webkit_source_id_ = webkit_source_.id();
-
-  InSequence s;
-  EXPECT_CALL(mock_delegate(), StartCapture(_, _, _));
-  blink::WebMediaStreamTrack track = StartSource(
-      VideoTrackAdapterSettings(), base::Optional<bool>(), false, 0.0);
-  base::RunLoop().RunUntilIdle();
-
-  OnStarted(true);
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(blink::WebMediaStreamSource::ReadyStateLive,
-            webkit_source_.getReadyState());
-
-  EXPECT_FALSE(source_stopped_);
-
-  EXPECT_CALL(mock_delegate(), StopCapture());
-  OnStarted(false);
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(blink::WebMediaStreamSource::ReadyStateEnded,
-            webkit_source_.getReadyState());
-  // Verify that MediaStreamSource::SourceStoppedCallback has been triggered.
-  EXPECT_TRUE(source_stopped_);
-}
-
-class FakeMediaStreamVideoSink : public MediaStreamVideoSink {
- public:
-  FakeMediaStreamVideoSink(base::TimeTicks* capture_time,
-                           media::VideoFrameMetadata* metadata,
-                           base::Closure got_frame_cb)
-      : capture_time_(capture_time),
-        metadata_(metadata),
-        got_frame_cb_(got_frame_cb) {}
-
-  void ConnectToTrack(const blink::WebMediaStreamTrack& track) {
-    MediaStreamVideoSink::ConnectToTrack(
-        track, base::Bind(&FakeMediaStreamVideoSink::OnVideoFrame,
-                          base::Unretained(this)),
-        true);
-  }
-
-  void DisconnectFromTrack() {
-    MediaStreamVideoSink::DisconnectFromTrack();
-  }
-
-  void OnVideoFrame(const scoped_refptr<media::VideoFrame>& frame,
-                    base::TimeTicks capture_time) {
-    *capture_time_ = capture_time;
-    metadata_->Clear();
-    metadata_->MergeMetadataFrom(frame->metadata());
-    base::ResetAndReturn(&got_frame_cb_).Run();
-  }
-
- private:
-  base::TimeTicks* const capture_time_;
-  media::VideoFrameMetadata* const metadata_;
-  base::Closure got_frame_cb_;
-};
-
-TEST_F(MediaStreamVideoCapturerSourceTest,
-       CaptureTimeAndMetadataPlumbingOldConstraints) {
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeature(
-      features::kMediaStreamOldVideoConstraints);
+TEST_F(MediaStreamVideoCapturerSourceOldConstraintsTest,
+       CaptureTimeAndMetadataPlumbing) {
   StreamDeviceInfo device_info;
   device_info.device.type = MEDIA_DESKTOP_VIDEO_CAPTURE;
   InitWithDeviceInfo(device_info);
@@ -496,60 +617,4 @@ TEST_F(MediaStreamVideoCapturerSourceTest,
   EXPECT_EQ(30.0, metadata_value);
 }
 
-TEST_F(MediaStreamVideoCapturerSourceTest, CaptureTimeAndMetadataPlumbing) {
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndDisableFeature(
-      features::kMediaStreamOldVideoConstraints);
-  std::unique_ptr<MockVideoCapturerSource> delegate(
-      new MockVideoCapturerSource());
-  delegate_ = delegate.get();
-  EXPECT_CALL(*delegate_, GetPreferredFormats());
-  source_ = new MediaStreamVideoCapturerSource(
-      base::Bind(&MediaStreamVideoCapturerSourceTest::OnSourceStopped,
-                 base::Unretained(this)),
-      std::move(delegate));
-  webkit_source_.initialize(blink::WebString::fromASCII("dummy_source_id"),
-                            blink::WebMediaStreamSource::TypeVideo,
-                            blink::WebString::fromASCII("dummy_source_name"),
-                            false /* remote */);
-  webkit_source_.setExtraData(source_);
-  webkit_source_id_ = webkit_source_.id();
-
-  VideoCaptureDeliverFrameCB deliver_frame_cb;
-  media::VideoCapturerSource::RunningCallback running_cb;
-
-  InSequence s;
-  //  EXPECT_CALL(mock_delegate(), GetCurrentSupportedFormats(_, _, _, _));
-  EXPECT_CALL(mock_delegate(), StartCapture(_, _, _))
-      .WillOnce(testing::DoAll(testing::SaveArg<1>(&deliver_frame_cb),
-                               testing::SaveArg<2>(&running_cb)));
-  EXPECT_CALL(mock_delegate(), RequestRefreshFrame());
-  EXPECT_CALL(mock_delegate(), StopCapture());
-  blink::WebMediaStreamTrack track = StartSource(
-      VideoTrackAdapterSettings(), base::Optional<bool>(), false, 0.0);
-  running_cb.Run(true);
-
-  base::RunLoop run_loop;
-  base::TimeTicks reference_capture_time =
-      base::TimeTicks::FromInternalValue(60013);
-  base::TimeTicks capture_time;
-  media::VideoFrameMetadata metadata;
-  FakeMediaStreamVideoSink fake_sink(
-      &capture_time, &metadata,
-      media::BindToCurrentLoop(run_loop.QuitClosure()));
-  fake_sink.ConnectToTrack(track);
-  const scoped_refptr<media::VideoFrame> frame =
-      media::VideoFrame::CreateBlackFrame(gfx::Size(2, 2));
-  frame->metadata()->SetDouble(media::VideoFrameMetadata::FRAME_RATE, 30.0);
-  child_process_->io_task_runner()->PostTask(
-      FROM_HERE, base::Bind(deliver_frame_cb, frame, reference_capture_time));
-  run_loop.Run();
-  fake_sink.DisconnectFromTrack();
-  EXPECT_EQ(reference_capture_time, capture_time);
-  double metadata_value;
-  EXPECT_TRUE(metadata.GetDouble(media::VideoFrameMetadata::FRAME_RATE,
-                                 &metadata_value));
-  EXPECT_EQ(30.0, metadata_value);
-}
-
 }  // namespace content