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

content/browser/renderer_host/media/in_process_video_capture_provider.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/media/in_process_video_capture_provider.h"
 
 #include "content/browser/renderer_host/media/in_process_video_capture_device_launcher.h"
 
 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