[Mojo Video Capture] Do not instantiate in-process VideoCaptureSystem when using service

content/browser/renderer_host/media/in_process_video_capture_provider.cc

Index: content/browser/renderer_host/media/in_process_video_capture_provider.cc
diff --git a/content/browser/renderer_host/media/in_process_video_capture_provider.cc b/content/browser/renderer_host/media/in_process_video_capture_provider.cc
index ff639713e83a4dd1ccf983036f58017eb7997a0c..9c62f8ee354e56710d05077f0102036848e71788 100644
--- a/content/browser/renderer_host/media/in_process_video_capture_provider.cc
+++ b/content/browser/renderer_host/media/in_process_video_capture_provider.cc
@@ -9,30 +9,53 @@
 namespace content {
 
 InProcessVideoCaptureProvider::InProcessVideoCaptureProvider(
-    std::unique_ptr<media::VideoCaptureSystem> video_capture_system,
+    std::unique_ptr<media::VideoCaptureSystem> optional_video_capture_system,
     scoped_refptr<base::SingleThreadTaskRunner> device_task_runner)
-    : video_capture_system_(std::move(video_capture_system)),
+    : optional_video_capture_system_(std::move(optional_video_capture_system)),
       device_task_runner_(std::move(device_task_runner)) {}
 
 InProcessVideoCaptureProvider::~InProcessVideoCaptureProvider() = default;
 
+// static
+std::unique_ptr<VideoCaptureProvider>
+InProcessVideoCaptureProvider::CreateInstanceForNonDeviceCapture(
+    scoped_refptr<base::SingleThreadTaskRunner> device_task_runner) {
+  return base::MakeUnique<InProcessVideoCaptureProvider>(
+      nullptr, std::move(device_task_runner));
+}
+
+// static
+std::unique_ptr<VideoCaptureProvider>
+InProcessVideoCaptureProvider::CreateInstance(
+    std::unique_ptr<media::VideoCaptureSystem> optional_video_capture_system,

[mcasas, 2017/05/18 17:42:12]

in l.29 of the .h, this parameter is called |video_capture_system|,
but here is called |optional_...|, which is misleading, since then
there's no difference with l.21, and you could rearrange this to
be a single factory called:

std::unique_ptr<VideoCaptureProvider>
InProcessVideoCaptureProvider::CreateInstance(
    scoped_refptr<base::SingleThreadTaskRunner> device_task_runner,
   std::unique_ptr<media::VideoCaptureSystem> video_capture_system = nullptr);

or similar, correct?

[chfremer, 2017/05/18 18:28:31]

Correct, thanks for catching this! I forgot to remove the "optional_" prefix in
this class.

I guess, technically, we could just use a public constructor like
InProcessVideoCaptureProvider(
    scoped_refptr<base::SingleThreadTaskRunner> device_task_runner,
    std::unique_ptr<media::VideoCaptureSystem> video_capture_system = nullptr);

The sole reason I went for the (rather verbose) two factory methods, is to be
able to convey intent via their method name. Please let me know if you would
prefer the public constructor.

Done.

+    scoped_refptr<base::SingleThreadTaskRunner> device_task_runner) {
+  return base::MakeUnique<InProcessVideoCaptureProvider>(
+      std::move(optional_video_capture_system), std::move(device_task_runner));
+}
+
 void InProcessVideoCaptureProvider::Uninitialize() {}
 
 void InProcessVideoCaptureProvider::GetDeviceInfosAsync(
-    const base::Callback<void(
-        const std::vector<media::VideoCaptureDeviceInfo>&)>& result_callback) {
-  // Using Unretained() is safe because |this| owns |video_capture_system_| and
-  // |result_callback| has ownership of |this|.
+    const GetDeviceInfosCallback& result_callback) {
+  if (!optional_video_capture_system_) {
+    std::vector<media::VideoCaptureDeviceInfo> empty_result;
+    result_callback.Run(empty_result);
+    return;
+  }
+  // Using Unretained() is safe because |this| owns
+  // |optional_video_capture_system_| and |result_callback| has ownership of
+  // |this|.
   device_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&media::VideoCaptureSystem::GetDeviceInfosAsync,
-                            base::Unretained(video_capture_system_.get()),
-                            result_callback));
+      FROM_HERE,
+      base::Bind(&media::VideoCaptureSystem::GetDeviceInfosAsync,
+                 base::Unretained(optional_video_capture_system_.get()),
+                 result_callback));
 }
 
 std::unique_ptr<VideoCaptureDeviceLauncher>
 InProcessVideoCaptureProvider::CreateDeviceLauncher() {
   return base::MakeUnique<InProcessVideoCaptureDeviceLauncher>(
-      device_task_runner_, video_capture_system_.get());
+      device_task_runner_, optional_video_capture_system_.get());
 }
 
 }  // namespace content