[Mojo Video Capture] Implement a VideoCaptureProvider using the Mojo service (part 2)

content/browser/renderer_host/media/service_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/service_video_capture_device_launcher.h"
 
+#include "content/browser/renderer_host/media/service_launched_video_capture_device.h"
+#include "content/public/browser/browser_thread.h"
+#include "media/capture/video/video_frame_receiver_on_task_runner.h"
+#include "mojo/public/cpp/bindings/strong_binding.h"
+#include "services/video_capture/public/cpp/receiver_media_to_mojo_adapter.h"
+
 namespace content {
 
 ServiceVideoCaptureDeviceLauncher::ServiceVideoCaptureDeviceLauncher(
     video_capture::mojom::DeviceFactoryPtr* device_factory)
-    : device_factory_(device_factory) {}
+    : device_factory_(device_factory), state_(State::READY_TO_LAUNCH) {}
 
-ServiceVideoCaptureDeviceLauncher::~ServiceVideoCaptureDeviceLauncher() {}
+ServiceVideoCaptureDeviceLauncher::~ServiceVideoCaptureDeviceLauncher() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(state_ == State::READY_TO_LAUNCH);
+}
 
 void ServiceVideoCaptureDeviceLauncher::LaunchDeviceAsync(
     const std::string& device_id,
     MediaStreamType stream_type,
     const media::VideoCaptureParams& params,
     base::WeakPtr<media::VideoFrameReceiver> receiver,
     Callbacks* callbacks,
     base::OnceClosure done_cb) {
-  NOTIMPLEMENTED();
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(state_ == State::READY_TO_LAUNCH);
+
+  if (stream_type != content::MEDIA_DEVICE_VIDEO_CAPTURE) {
+    // This launcher only supports MEDIA_DEVICE_VIDEO_CAPTURE.
+    NOTREACHED();
+    return;
+  }
+
+  if (!device_factory_->is_bound()) {
+    // This can happen when the ServiceVideoCaptureProvider owning
+    // |device_factory_| loses connection to the service process and resets
+    // |device_factory_|.
+    OnDeviceCreationFailed(callbacks, std::move(done_cb));
+    return;
+  }
+  video_capture::mojom::DevicePtr device;
+  (*device_factory_)
+      ->CreateDevice(
+          device_id, mojo::MakeRequest(&device),
+          base::Bind(
+              // Use of Unretained |this| is safe, because |done_cb| guarantees
+              // that |this| stays alive.
+              &ServiceVideoCaptureDeviceLauncher::OnCreateDeviceCallback,
+              base::Unretained(this), params, base::Passed(&device),
+              std::move(receiver), callbacks, base::Passed(&done_cb)));
+  state_ = State::DEVICE_START_IN_PROGRESS;
 }
 
 void ServiceVideoCaptureDeviceLauncher::AbortLaunch() {
-  NOTIMPLEMENTED();
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (state_ == State::DEVICE_START_IN_PROGRESS)
+    state_ = State::DEVICE_START_ABORTING;
+}
+
+void ServiceVideoCaptureDeviceLauncher::OnCreateDeviceCallback(
+    const media::VideoCaptureParams& params,
+    video_capture::mojom::DevicePtr device,
+    base::WeakPtr<media::VideoFrameReceiver> receiver,
+    Callbacks* callbacks,
+    base::OnceClosure done_cb,
+    video_capture::mojom::DeviceAccessResultCode result_code) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  switch (result_code) {
+    case video_capture::mojom::DeviceAccessResultCode::SUCCESS:
+      OnDeviceCreatedSuccessfully(params, std::move(device),
+                                  std::move(receiver), callbacks,
+                                  std::move(done_cb));
+      return;
+    case video_capture::mojom::DeviceAccessResultCode::ERROR_DEVICE_NOT_FOUND:
+    case video_capture::mojom::DeviceAccessResultCode::NOT_INITIALIZED:
+      OnDeviceCreationFailed(callbacks, std::move(done_cb));
+      return;
+  }
+}
+
+void ServiceVideoCaptureDeviceLauncher::OnDeviceCreatedSuccessfully(
+    const media::VideoCaptureParams& params,
+    video_capture::mojom::DevicePtr device,
+    base::WeakPtr<media::VideoFrameReceiver> receiver,
+    Callbacks* callbacks,
+    base::OnceClosure done_cb) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  State state_copy = state_;

[miu, 2017/05/08 20:21:11]

naming: How about |prior_state|?

Actually, you don't even need this. A slight refactoring of your code:

  ... {
    DCHECK(thread_checker_.CalledOnValidThread());
  
    if (state_ == State::DEVICE_START_ABORTING) {
      device.reset();
      callbacks->OnDeviceLaunchAborted();
      base::ResetAndReturn(&done_cb).Run();
      return;
    }
  
    state_ = State::READY_TO_LAUNCH;
  
    ...

This assumes it's okay to leave |state_| set to DEVICE_START_ABORTING. If not,
maybe add a separated DEVICE_START_ABORTED state, since READY_TO_LAUNCH doesn't
seem accurate?

[chfremer, 2017/05/08 20:51:46]

Agreed that this is probably better than |state_copy|, even though the "prior"
might be confusing as to exactly when we are referring to. How do you feel about
|coming_from_state|?
 
The idea here was to make it legal for callbacks such as
|callbacks->OnDeviceLaunched(...)| to synchronously invoke
|ServiceVideoCaptureDeviceLauncher::LaunchDeviceAsync(...)| again in order to
retry or start a next device. And the device launch queue in VideoCaptureManager
[1], does exactly that. The somewhat ugly local copy of |state_| is only needed
because of that.

In that design, a state DEVICE_START_ABORTED would have the exact same meaning
as READY_TO_LAUNCH, so I don't think it would add value to introduce it as a
separate state.

[1]
https://cs.chromium.org/chromium/src/content/browser/renderer_host/media/vide...

+  state_ = State::READY_TO_LAUNCH;
+
+  if (state_copy == State::DEVICE_START_ABORTING) {
+    device.reset();
+    callbacks->OnDeviceLaunchAborted();
+    base::ResetAndReturn(&done_cb).Run();
+    return;
+  }
+
+  auto receiver_adapter =
+      base::MakeUnique<video_capture::ReceiverMediaToMojoAdapter>(
+          base::MakeUnique<media::VideoFrameReceiverOnTaskRunner>(
+              std::move(receiver),
+              BrowserThread::GetTaskRunnerForThread(BrowserThread::IO)));
+  video_capture::mojom::ReceiverPtr receiver_proxy;
+  mojo::MakeStrongBinding<video_capture::mojom::Receiver>(
+      std::move(receiver_adapter), mojo::MakeRequest(&receiver_proxy));
+  device->Start(params, std::move(receiver_proxy));
+  callbacks->OnDeviceLaunched(
+      base::MakeUnique<ServiceLaunchedVideoCaptureDevice>(std::move(device)));
+  base::ResetAndReturn(&done_cb).Run();
+}
+
+void ServiceVideoCaptureDeviceLauncher::OnDeviceCreationFailed(
+    Callbacks* callbacks,
+    base::OnceClosure done_cb) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  State state_copy = state_;

[miu, 2017/05/08 20:21:11]

ditto: I think you could simplify by not mutating |state_|, similar to my last
comment.

[chfremer, 2017/05/08 20:51:46]

see other reply

+  state_ = State::READY_TO_LAUNCH;
+  if (state_copy == State::DEVICE_START_ABORTING)
+    callbacks->OnDeviceLaunchAborted();
+  else
+    callbacks->OnDeviceLaunchFailed();
+  base::ResetAndReturn(&done_cb).Run();
 }
 
 }  // namespace content