Spec compliant video constraints for getUserMedia behind flag.

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 55f54b7efb2115473cddc2baf21c74706624c6ca..71dc58a2e6865b0ce813833cad1e5e7d844af89a 100644
--- a/content/renderer/media/media_stream_video_capturer_source_unittest.cc
+++ b/content/renderer/media/media_stream_video_capturer_source_unittest.cc
@@ -12,10 +12,13 @@
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/scoped_feature_list.h"
 #include "content/child/child_process.h"
+#include "content/public/common/content_features.h"
 #include "content/public/renderer/media_stream_video_sink.h"
 #include "content/renderer/media/media_stream_video_track.h"
 #include "content/renderer/media/mock_constraint_factory.h"
+#include "content/renderer/media/video_track_adapter.h"
 #include "media/base/bind_to_current_loop.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -104,6 +107,21 @@ class MediaStreamVideoCapturerSourceTest : public testing::Test {
         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(&MediaStreamVideoCapturerSourceTest::OnConstraintsApplied,
+                   base::Unretained(this)),
+        enabled);
+  }
+
   MockVideoCapturerSource& mock_delegate() { return *delegate_; }
 
   const char* GetPowerLineFrequencyForTesting() const {
@@ -134,7 +152,11 @@ class MediaStreamVideoCapturerSourceTest : public testing::Test {
   MockConstraintFactory constraint_factory_;
 };
 
+// This test does not apply with spec-compliant constraints.
 TEST_F(MediaStreamVideoCapturerSourceTest, TabCaptureFixedResolutionByDefault) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      features::kMediaStreamOldVideoConstraints);
   StreamDeviceInfo device_info;
   device_info.device.type = MEDIA_TAB_VIDEO_CAPTURE;
   InitWithDeviceInfo(device_info);
@@ -159,8 +181,12 @@ TEST_F(MediaStreamVideoCapturerSourceTest, TabCaptureFixedResolutionByDefault) {
   EXPECT_CALL(mock_delegate(), StopCapture());
 }
 
+// This test does not apply with spec-compliant constraints.

[hbos_chromium, 2017/04/05 12:37:28]

For all of these add a TODO to remove when the old one goes away?
Also to every test that only applies to the old version, add
"WithOldConstraints" or something to the end?

[Guido Urdaneta, 2017/04/05 16:17:16]

Done.

 TEST_F(MediaStreamVideoCapturerSourceTest,
        DesktopCaptureAllowAnyResolutionChangeByDefault) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      features::kMediaStreamOldVideoConstraints);
   StreamDeviceInfo device_info;
   device_info.device.type = MEDIA_DESKTOP_VIDEO_CAPTURE;
   InitWithDeviceInfo(device_info);
@@ -185,8 +211,12 @@ TEST_F(MediaStreamVideoCapturerSourceTest,
   EXPECT_CALL(mock_delegate(), StopCapture());
 }
 
+// This test does not apply with spec-compliant constraints.
 TEST_F(MediaStreamVideoCapturerSourceTest,
        TabCaptureConstraintsImplyFixedAspectRatio) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      features::kMediaStreamOldVideoConstraints);
   StreamDeviceInfo device_info;
   device_info.device.type = MEDIA_TAB_VIDEO_CAPTURE;
   InitWithDeviceInfo(device_info);
@@ -218,8 +248,12 @@ TEST_F(MediaStreamVideoCapturerSourceTest,
   EXPECT_CALL(mock_delegate(), StopCapture());
 }
 
+// This test does not apply with spec-compliant constraints.
 TEST_F(MediaStreamVideoCapturerSourceTest,
        TabCaptureConstraintsImplyAllowingAnyResolutionChange) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      features::kMediaStreamOldVideoConstraints);
   StreamDeviceInfo device_info;
   device_info.device.type = MEDIA_TAB_VIDEO_CAPTURE;
   InitWithDeviceInfo(device_info);
@@ -251,8 +285,12 @@ TEST_F(MediaStreamVideoCapturerSourceTest,
   EXPECT_CALL(mock_delegate(), StopCapture());
 }
 
+// This test does not apply with spec-compliant constraints.
 TEST_F(MediaStreamVideoCapturerSourceTest,
        DeviceCaptureConstraintsSupportPowerLineFrequency) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      features::kMediaStreamOldVideoConstraints);
   for (int frequency = -100; frequency < 100; ++frequency) {
     StreamDeviceInfo device_info;
     device_info.device.type = MEDIA_DEVICE_VIDEO_CAPTURE;
@@ -292,6 +330,9 @@ TEST_F(MediaStreamVideoCapturerSourceTest,
 }
 
 TEST_F(MediaStreamVideoCapturerSourceTest, Ended) {

[hbos_chromium, 2017/04/05 12:37:28]

nit: Add here too
// This test does not apply with spec-compliant constraints.

[Guido Urdaneta, 2017/04/05 16:17:16]

Done. Actually this test does apply (hence the New version).
Did add the comment to remove the old tests, removed the New suffix for the
tests that are going to survive, and added the OldConstraints suffix for the
tests that are scheduled for removal.

+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      features::kMediaStreamOldVideoConstraints);
   std::unique_ptr<MockVideoCapturerSource> delegate(
       new MockVideoCapturerSource());
   delegate_ = delegate.get();
@@ -328,6 +369,47 @@ TEST_F(MediaStreamVideoCapturerSourceTest, Ended) {
   EXPECT_TRUE(source_stopped_);
 }
 
+TEST_F(MediaStreamVideoCapturerSourceTest, EndedNew) {

[hbos_chromium, 2017/04/05 12:37:28]

Since this covers all of interest (with constraints code being tested elsewhere
and not needing a "...New" for every test here) can you change the name to
something else?

StartAndStopCaptureWithNewConstraints

[Guido Urdaneta, 2017/04/05 16:17:16]

Renamed to StartAndStop.
No suffix implies new constraints now, and since this is a capturer source, no
need to add Capture.

[hbos_chromium, 2017/04/05 16:56:31]

Acknowledged.

+  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,
@@ -363,6 +445,9 @@ class FakeMediaStreamVideoSink : public MediaStreamVideoSink {
 };
 
 TEST_F(MediaStreamVideoCapturerSourceTest, CaptureTimeAndMetadataPlumbing) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(
+      features::kMediaStreamOldVideoConstraints);
   StreamDeviceInfo device_info;
   device_info.device.type = MEDIA_DESKTOP_VIDEO_CAPTURE;
   InitWithDeviceInfo(device_info);
@@ -403,4 +488,60 @@ TEST_F(MediaStreamVideoCapturerSourceTest, CaptureTimeAndMetadataPlumbing) {
   EXPECT_EQ(30.0, metadata_value);
 }
 
+TEST_F(MediaStreamVideoCapturerSourceTest, CaptureTimeAndMetadataPlumbingNew) {

[hbos_chromium, 2017/04/05 12:37:28]

...WithOldConstraints
and
...WithNewConstraints
?

[Guido Urdaneta, 2017/04/05 16:17:16]

Using OldConstraints for tests that use the old algorithm, and no suffix for
tests that use the new algorithm.

[hbos_chromium, 2017/04/05 16:56:31]

Acknowledged.

+  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