[Mojo Video Capture] Fix VideoCaptureManager exposing implementation details to clients

content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc

Index: content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
diff --git a/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc b/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
index 44e034bf089649e3be43a36327ab4bcce017e5bc..f4a56e534d2e1026e402e2accde7f80d0a350853 100644
--- a/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
+++ b/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
@@ -53,12 +53,19 @@ using ::testing::_;
 using ::testing::DeleteArg;
 using ::testing::DoAll;
 using ::testing::InSequence;
+using ::testing::Invoke;
 using ::testing::Return;
 using ::testing::SaveArg;
 
 const int kProcessId = 5;
 const int kRenderId = 6;
 const int kPageRequestId = 7;
+static const char* kRegularVideoDeviceId = "stub_device_0";

[miu, 2017/04/03 21:31:23]

Prefer constexpr to static const here.

Also, all these constants (old and new) should be moved into the anonymous
namespace (below).

[chfremer, 2017/04/04 21:59:32]

Done.

+static const char* kDepthVideoDeviceId = "stub_device_1 (depth)";
+static const double kStubFocalLengthX = 135.0;
+static const double kStubFocalLengthY = 135.6;
+static const double kStubDepthNear = 0.0;
+static const double kStubDepthFar = 65.535;
 
 namespace content {
 
@@ -73,7 +80,7 @@ void AudioInputDevicesEnumerated(base::Closure quit_closure,
   quit_closure.Run();
 }
 
-}  // namespace
+}  // anonymous namespace
 
 class MockMediaStreamDispatcherHost : public MediaStreamDispatcherHost,
                                       public TestContentBrowserClient {
@@ -228,6 +235,18 @@ class MockMediaStreamUIProxy : public FakeMediaStreamUIProxy {
            const MediaStreamUIProxy::WindowIdCallback& window_id_callback));
 };
 
+class MockVideoCaptureSystem : public media::VideoCaptureSystem {
+ public:
+  MOCK_METHOD1(GetDeviceInfosAsync,
+               void(const base::Callback<
+                    void(const std::vector<media::VideoCaptureDeviceInfo>&)>&
+                        result_callback));
+
+  MOCK_METHOD1(
+      CreateDevice,
+      std::unique_ptr<media::VideoCaptureDevice>(const std::string& device_id));
+};
+
 class MediaStreamDispatcherHostTest : public testing::Test {
  public:
   MediaStreamDispatcherHostTest()
@@ -240,19 +259,13 @@ class MediaStreamDispatcherHostTest : public testing::Test {
     // Make sure we use fake devices to avoid long delays.
     base::CommandLine::ForCurrentProcess()->AppendSwitch(
         switches::kUseFakeDeviceForMediaStream);
+    auto mock_video_capture_system = base::MakeUnique<MockVideoCaptureSystem>();
+    mock_video_capture_system_ = mock_video_capture_system.get();
     // Create our own MediaStreamManager.
     media_stream_manager_ =
-        base::MakeUnique<MediaStreamManager>(audio_system_.get());
-    video_capture_device_factory_ =
-        static_cast<media::FakeVideoCaptureDeviceFactory*>(
-            media_stream_manager_->video_capture_manager()
-            ->video_capture_device_factory());
-    DCHECK(video_capture_device_factory_);
-#if defined(OS_WIN)
-    // Override the Video Capture Thread that MediaStreamManager constructs.
-    media_stream_manager_->video_capture_manager()->set_device_task_runner(
-        base::ThreadTaskRunnerHandle::Get());
-#endif
+        MediaStreamManager::CreateWithCustomVideoCaptureSystem(
+            audio_system_.get(), std::move(mock_video_capture_system),
+            base::ThreadTaskRunnerHandle::Get());
 
     MockResourceContext* mock_resource_context =
         static_cast<MockResourceContext*>(
@@ -279,9 +292,32 @@ class MediaStreamDispatcherHostTest : public testing::Test {
   }
 
   void SetUp() override {
-    video_capture_device_factory_->GetDeviceDescriptors(
-        &physical_video_devices_);
-    ASSERT_GT(physical_video_devices_.size(), 0u);
+    stub_video_device_ids_.emplace_back(kRegularVideoDeviceId);
+    stub_video_device_ids_.emplace_back(kDepthVideoDeviceId);
+    const media::VideoCaptureApi stub_capture_api =

[miu, 2017/04/03 21:31:23]

Seems that this constant defined here, then captured by the lambda, and only
used within the lambda. Could it just be used directly in the lambda? Meaning,
L309 would just be:

  info.descriptor.capture_api = media::VCApi::LINUX_V4L2_SINGLE_PLANE;

[chfremer, 2017/04/04 21:59:32]

Done.

+        media::VideoCaptureApi::LINUX_V4L2_SINGLE_PLANE;
+    ON_CALL(*mock_video_capture_system_, GetDeviceInfosAsync(_))
+        .WillByDefault(
+            Invoke([this, stub_capture_api](
+                       const base::Callback<void(
+                           const std::vector<media::VideoCaptureDeviceInfo>&)>&
+                           result_callback) {
+              std::vector<media::VideoCaptureDeviceInfo> result;
+              for (const auto& device_id : stub_video_device_ids_) {
+                media::VideoCaptureDeviceInfo info;
+                info.descriptor.device_id = device_id;
+                info.descriptor.capture_api = stub_capture_api;
+                if (device_id == kDepthVideoDeviceId) {
+                  info.descriptor.camera_calibration.emplace();
+                  info.descriptor.camera_calibration->focal_length_x = 135.0;
+                  info.descriptor.camera_calibration->focal_length_y = 135.6;
+                  info.descriptor.camera_calibration->depth_near = 0.0;
+                  info.descriptor.camera_calibration->depth_far = 65.535;
+                }
+                result.push_back(info);
+              }
+              result_callback.Run(result);
+            }));
 
     base::RunLoop run_loop;
     MediaDevicesManager::BoolDeviceTypes devices_to_enumerate;
@@ -314,7 +350,7 @@ class MediaStreamDispatcherHostTest : public testing::Test {
     int expected_audio_array_size =
         (controls.audio.requested && !physical_audio_devices_.empty()) ? 1 : 0;
     int expected_video_array_size =
-        (controls.video.requested && !physical_video_devices_.empty()) ? 1 : 0;
+        (controls.video.requested && !stub_video_device_ids_.empty()) ? 1 : 0;
     EXPECT_CALL(*host_.get(), OnStreamGenerated(render_frame_id,
                                                 page_request_id,
                                                 expected_audio_array_size,
@@ -366,8 +402,8 @@ class MediaStreamDispatcherHostTest : public testing::Test {
             return true;
         }
       }
-      for (const auto& video_device : physical_video_devices_) {
-        if (video_device.device_id == devices[i].device.id)
+      for (const std::string& device_id : stub_video_device_ids_) {
+        if (device_id == devices[i].device.id)
           return true;
       }
     }
@@ -390,12 +426,10 @@ class MediaStreamDispatcherHostTest : public testing::Test {
           found_match = true;
         }
       }
-      media::VideoCaptureDeviceDescriptors::const_iterator video_it =
-          physical_video_devices_.begin();
-      for (; video_it != physical_video_devices_.end(); ++video_it) {
+      for (const std::string& device_id : stub_video_device_ids_) {
         if (content::DoesMediaDeviceIDMatchHMAC(
                 browser_context_.GetResourceContext()->GetMediaDeviceIDSalt(),
-                origin, devices[i].device.id, video_it->device_id)) {
+                origin, devices[i].device.id, device_id)) {
           EXPECT_FALSE(found_match);
           found_match = true;
         }
@@ -435,9 +469,9 @@ class MediaStreamDispatcherHostTest : public testing::Test {
   std::unique_ptr<ContentClient> content_client_;
   content::TestBrowserContext browser_context_;
   media::AudioDeviceDescriptions physical_audio_devices_;

[miu, 2017/04/03 21:31:23]

s/physical_audio/stub_audio/ too, or are they really physical during the unit
tests?

[chfremer, 2017/04/04 21:59:32]

I had the same question, but since it is fairly non-obvious to figure out, I
decided to not touch this. I believe what is being used is "fake" audio devices.

Renaming to "audio_device_descriptions_". 

-  media::VideoCaptureDeviceDescriptors physical_video_devices_;
+  std::vector<std::string> stub_video_device_ids_;
   url::Origin origin_;
-  media::FakeVideoCaptureDeviceFactory* video_capture_device_factory_;
+  MockVideoCaptureSystem* mock_video_capture_system_;
 };
 
 TEST_F(MediaStreamDispatcherHostTest, GenerateStreamWithVideoOnly) {
@@ -481,26 +515,21 @@ TEST_F(MediaStreamDispatcherHostTest, GenerateStreamWithAudioAndVideo) {
 }
 
 TEST_F(MediaStreamDispatcherHostTest, GenerateStreamWithDepthVideo) {
-  // Video device on index 1 is depth video capture device.  The number of fake
-  // devices is 2.
-  physical_video_devices_.clear();
-  video_capture_device_factory_->SetToDefaultDevicesConfig(2);
-  video_capture_device_factory_->GetDeviceDescriptors(&physical_video_devices_);
   // We specify to generate both audio and video stream.
   StreamControls controls(true, true);
   std::string source_id = content::GetHMACForMediaDeviceID(
       browser_context_.GetResourceContext()->GetMediaDeviceIDSalt(), origin_,
-      physical_video_devices_[1].device_id);
-  // |source_id| is related to depth device (the device on index 1). As we can
-  // generate only one video stream using GenerateStreamAndWaitForResult, we
-  // use controls.video.source_id to specify that the stream is depth video.
+      kDepthVideoDeviceId);
+  // |source_id| corresponds to the depth device. As we can generate only one
+  // video stream using GenerateStreamAndWaitForResult, we use
+  // controls.video.source_id to specify that the stream is depth video.
   // See also MediaStreamManager::GenerateStream and other tests here.
   controls.video.device_id = source_id;
 
   SetupFakeUI(true);
   GenerateStreamAndWaitForResult(kRenderId, kPageRequestId, controls);
 
-  // There are two fake devices. We specified the generation and expect to get
+  // We specified the generation and expect to get
   // one audio and one depth video stream.
   EXPECT_EQ(host_->audio_devices_.size(), 1u);
   EXPECT_EQ(host_->video_devices_.size(), 1u);
@@ -509,10 +538,10 @@ TEST_F(MediaStreamDispatcherHostTest, GenerateStreamWithDepthVideo) {
   const base::Optional<CameraCalibration> calibration =
       host_->video_devices_[0].device.camera_calibration;
   EXPECT_TRUE(calibration);
-  EXPECT_EQ(calibration->focal_length_x, 135.0);
-  EXPECT_EQ(calibration->focal_length_y, 135.6);
-  EXPECT_EQ(calibration->depth_near, 0.0);
-  EXPECT_EQ(calibration->depth_far, 65.535);
+  EXPECT_EQ(calibration->focal_length_x, kStubFocalLengthX);
+  EXPECT_EQ(calibration->focal_length_y, kStubFocalLengthY);
+  EXPECT_EQ(calibration->depth_near, kStubDepthNear);
+  EXPECT_EQ(calibration->depth_far, kStubDepthFar);
 }
 
 // This test generates two streams with video only using the same render frame
@@ -637,7 +666,7 @@ TEST_F(MediaStreamDispatcherHostTest, GenerateStreamsWithoutWaiting) {
 // the request.
 TEST_F(MediaStreamDispatcherHostTest, GenerateStreamsWithSourceId) {
   ASSERT_GE(physical_audio_devices_.size(), 1u);
-  ASSERT_GE(physical_video_devices_.size(), 1u);
+  ASSERT_GE(stub_video_device_ids_.size(), 1u);
 
   media::AudioDeviceDescriptions::const_iterator audio_it =
       physical_audio_devices_.begin();
@@ -655,12 +684,10 @@ TEST_F(MediaStreamDispatcherHostTest, GenerateStreamsWithSourceId) {
     EXPECT_EQ(host_->audio_devices_[0].device.id, source_id);
   }
 
-  media::VideoCaptureDeviceDescriptors::const_iterator video_it =
-      physical_video_devices_.begin();
-  for (; video_it != physical_video_devices_.end(); ++video_it) {
+  for (const std::string& device_id : stub_video_device_ids_) {
     std::string source_id = content::GetHMACForMediaDeviceID(
         browser_context_.GetResourceContext()->GetMediaDeviceIDSalt(), origin_,
-        video_it->device_id);
+        device_id);
     ASSERT_FALSE(source_id.empty());
     StreamControls controls(true, true);
     controls.video.device_id = source_id;
@@ -690,9 +717,7 @@ TEST_F(MediaStreamDispatcherHostTest, GenerateStreamsWithInvalidAudioSourceId) {
 }
 
 TEST_F(MediaStreamDispatcherHostTest, GenerateStreamsNoAvailableVideoDevice) {
-  physical_video_devices_.clear();
-  video_capture_device_factory_->SetToDefaultDevicesConfig(0);
-  video_capture_device_factory_->GetDeviceDescriptors(&physical_video_devices_);
+  stub_video_device_ids_.clear();
   StreamControls controls(true, true);
 
   SetupFakeUI(false);
@@ -848,7 +873,7 @@ TEST_F(MediaStreamDispatcherHostTest, VideoDeviceUnplugged) {
   EXPECT_EQ(host_->audio_devices_.size(), 1u);
   EXPECT_EQ(host_->video_devices_.size(), 1u);
 
-  video_capture_device_factory_->SetToDefaultDevicesConfig(0);
+  stub_video_device_ids_.clear();
 
   base::RunLoop run_loop;
   EXPECT_CALL(*host_.get(), OnDeviceStopped(kRenderId))