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

content/browser/renderer_host/media/in_process_video_capture_device_launcher.cc

 // Copyright 2017 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_device_launcher.h"
 
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/stringprintf.h"
 #include "content/browser/media/capture/desktop_capture_device_uma_types.h"
 #include "content/browser/media/capture/web_contents_video_capture_device.h"
@@ skipping 39 matching lines @@
 // TODO(miu): The value for tab capture should be determined programmatically.
 // http://crbug.com/460318
 const int kMaxNumberOfBuffersForTabCapture = 10;
 
 }  // anonymous namespace
 
 namespace content {
 
 InProcessVideoCaptureDeviceLauncher::InProcessVideoCaptureDeviceLauncher(
     scoped_refptr<base::SingleThreadTaskRunner> device_task_runner,
-    media::VideoCaptureSystem* video_capture_system)
+    media::VideoCaptureSystem* optional_video_capture_system)
     : device_task_runner_(std::move(device_task_runner)),
-      video_capture_system_(video_capture_system),
+      optional_video_capture_system_(optional_video_capture_system),
       state_(State::READY_TO_LAUNCH) {}
 
 InProcessVideoCaptureDeviceLauncher::~InProcessVideoCaptureDeviceLauncher() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(state_ == State::READY_TO_LAUNCH);
 }
 
 void InProcessVideoCaptureDeviceLauncher::LaunchDeviceAsync(
     const std::string& device_id,
     MediaStreamType stream_type,
@@ skipping 12 matching lines @@
 
   base::Closure start_capture_closure;
   // Use of Unretained |this| is safe, because |done_cb| guarantees that |this|
   // stays alive.
   ReceiveDeviceCallback after_start_capture_callback = media::BindToCurrentLoop(
       base::Bind(&InProcessVideoCaptureDeviceLauncher::OnDeviceStarted,
                  base::Unretained(this), callbacks, base::Passed(&done_cb)));
 
   switch (stream_type) {
     case MEDIA_DEVICE_VIDEO_CAPTURE: {
+      if (!optional_video_capture_system_) {
+        callbacks->OnDeviceLaunchFailed();
+        base::ResetAndReturn(&done_cb).Run();
+        return;
+      }
       start_capture_closure =
           base::Bind(&InProcessVideoCaptureDeviceLauncher::
                          DoStartDeviceCaptureOnDeviceThread,
                      base::Unretained(this), device_id, params,
                      base::Passed(std::move(device_client)),
                      std::move(after_start_capture_callback));
       break;
     }
     case MEDIA_TAB_VIDEO_CAPTURE:
       start_capture_closure = base::Bind(
@@ skipping 53 matching lines @@
     Callbacks* callbacks,
     base::OnceClosure done_cb,
     std::unique_ptr<media::VideoCaptureDevice> device) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   State state_copy = state_;
   state_ = State::READY_TO_LAUNCH;
   if (!device) {
     switch (state_copy) {
       case State::DEVICE_START_IN_PROGRESS:
         callbacks->OnDeviceLaunchFailed();
+        base::ResetAndReturn(&done_cb).Run();
         return;
       case State::DEVICE_START_ABORTING:
         callbacks->OnDeviceLaunchAborted();
+        base::ResetAndReturn(&done_cb).Run();
         return;
       case State::READY_TO_LAUNCH:
         NOTREACHED();
         return;
     }
   }
 
   auto launched_device = base::MakeUnique<InProcessLaunchedVideoCaptureDevice>(
       std::move(device), device_task_runner_);
 
   switch (state_copy) {
     case State::DEVICE_START_IN_PROGRESS:
       callbacks->OnDeviceLaunched(std::move(launched_device));
+      base::ResetAndReturn(&done_cb).Run();
       return;
     case State::DEVICE_START_ABORTING:
       launched_device.reset();
       callbacks->OnDeviceLaunchAborted();
+      base::ResetAndReturn(&done_cb).Run();
       return;
     case State::READY_TO_LAUNCH:
       NOTREACHED();
       return;
   }

[mcasas, 2017/05/16 22:22:22]

If we want to run |done_cb| in every possible return path,
consider adding in l.167-ish

base::ScopedClosureRunner scoped_callback(done_cb);

which will call Run() on destruction.

[chfremer, 2017/05/16 22:45:02]

Agreed that this would be better and cleaner. Unfortunately it looks like
ScopedClosureRunner does not (yet) support move-only types like OnceClosure.

-  base::ResetAndReturn(&done_cb).Run();
 }
 
 void InProcessVideoCaptureDeviceLauncher::DoStartDeviceCaptureOnDeviceThread(
     const std::string& device_id,
     const media::VideoCaptureParams& params,
     std::unique_ptr<media::VideoCaptureDeviceClient> device_client,
     ReceiveDeviceCallback result_callback) {
   SCOPED_UMA_HISTOGRAM_TIMER("Media.VideoCaptureManager.StartDeviceTime");
   DCHECK(device_task_runner_->BelongsToCurrentThread());
 
   std::unique_ptr<media::VideoCaptureDevice> video_capture_device =
-      video_capture_system_->CreateDevice(device_id);
+      optional_video_capture_system_->CreateDevice(device_id);

[mcasas, 2017/05/16 22:22:22]

If it's optional, we should check if (optional_video_capture_system_),
right?

[chfremer, 2017/05/16 22:45:01]

True. There is already a check in LaunchDeviceAsync() that should make it so
that this method here never gets called when |optional_video_capture_system_| is
nullptr. I'll add a DCHECK here to make sure.

 
   if (!video_capture_device) {
     result_callback.Run(nullptr);
     return;
   }
 
   video_capture_device->AllocateAndStart(params, std::move(device_client));
   result_callback.Run(std::move(video_capture_device));
 }
 
@@ skipping 64 matching lines @@
   if (!video_capture_device) {
     result_callback.Run(nullptr);
     return;
   }
 
   video_capture_device->AllocateAndStart(params, std::move(device_client));
   result_callback.Run(std::move(video_capture_device));
 }
 
 }  // namespace content