[Mojo Video Capture] Add support to BuildableVideoCaptureDevice for aborting the device start.

content/browser/renderer_host/media/in_process_buildable_video_capture_device.cc

Index: content/browser/renderer_host/media/in_process_buildable_video_capture_device.cc
diff --git a/content/browser/renderer_host/media/in_process_buildable_video_capture_device.cc b/content/browser/renderer_host/media/in_process_buildable_video_capture_device.cc
index 374dddaf14e654d86ee8c7f945ed61a0d8daae33..1c72dfbefd69dd7eeb2fd748c687d156acc3d6f3 100644
--- a/content/browser/renderer_host/media/in_process_buildable_video_capture_device.cc
+++ b/content/browser/renderer_host/media/in_process_buildable_video_capture_device.cc
@@ -8,6 +8,7 @@
 #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"
+#include "content/browser/renderer_host/media/ownership.h"
 #include "content/browser/renderer_host/media/video_capture_controller.h"
 #include "content/browser/renderer_host/media/video_capture_gpu_jpeg_decoder.h"
 #include "content/browser/renderer_host/media/video_frame_receiver_on_io_thread.h"
@@ -99,6 +100,7 @@ void InProcessBuildableVideoCaptureDevice::CreateAndStartDeviceAsync(
     BuildableDeviceCallbacks* callbacks,
     std::unique_ptr<Ownership> context_reference) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(state_ == State::NO_DEVICE);
 
   const int max_buffers = (controller->stream_type() == MEDIA_TAB_VIDEO_CAPTURE
                                ? kMaxNumberOfBuffersForTabCapture
@@ -165,6 +167,7 @@ void InProcessBuildableVideoCaptureDevice::CreateAndStartDeviceAsync(
   }
 
   device_task_runner_->PostTask(FROM_HERE, start_capture_closure);
+  state_ = State::DEVICE_START_IN_PROGRESS;
 }
 
 void InProcessBuildableVideoCaptureDevice::ReleaseDeviceAsync(
@@ -172,19 +175,28 @@ void InProcessBuildableVideoCaptureDevice::ReleaseDeviceAsync(
     std::unique_ptr<Ownership>) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   controller->SetConsumerFeedbackObserver(nullptr);
-  if (!device_)
-    return;
-  media::VideoCaptureDevice* device_ptr = device_.release();
-  auto device_task_runner_context_reference =
-      MakeScopedRefptrOwnership(device_task_runner_);
-  bool posting_task_succeeded = device_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&StopAndReleaseDeviceOnDeviceThread, device_ptr,
-                 base::Passed(&device_task_runner_context_reference)));
-  if (posting_task_succeeded == false) {
-    // Since posting to the task runner has failed, we attempt doing it on
-    // the calling thread instead.
-    StopAndReleaseDeviceOnDeviceThread(device_ptr, nullptr);
+  switch (state_) {
+    case State::DEVICE_START_IN_PROGRESS:
+      state_ = State::DEVICE_START_ABORTING;
+      return;
+    case State::NO_DEVICE:
+    case State::DEVICE_START_ABORTING:
+      return;
+    case State::DEVICE_STARTED:
+      media::VideoCaptureDevice* device_ptr = device_.release();
+      auto device_task_runner_context_reference =
+          MakeScopedRefptrOwnership(device_task_runner_);
+      bool posting_task_succeeded = device_task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(&StopAndReleaseDeviceOnDeviceThread, device_ptr,
+                     base::Passed(&device_task_runner_context_reference)));
+      if (posting_task_succeeded == false) {
+        // Since posting to the task runner has failed, we attempt doing it on
+        // the calling thread instead.
+        StopAndReleaseDeviceOnDeviceThread(device_ptr, nullptr);
+      }
+      state_ = State::NO_DEVICE;
+      return;
   }
 }
 
@@ -280,13 +292,14 @@ InProcessBuildableVideoCaptureDevice::CreateDeviceClient(
     base::WeakPtr<media::VideoFrameReceiver> receiver) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  scoped_refptr<media::VideoCaptureBufferPool> buffer_pool_ =
+  scoped_refptr<media::VideoCaptureBufferPool> buffer_pool =
       new media::VideoCaptureBufferPoolImpl(
           base::MakeUnique<media::VideoCaptureBufferTrackerFactoryImpl>(),
           buffer_pool_max_buffer_count);
 
   return base::MakeUnique<media::VideoCaptureDeviceClient>(
-      base::MakeUnique<VideoFrameReceiverOnIOThread>(receiver), buffer_pool_,
+      base::MakeUnique<VideoFrameReceiverOnIOThread>(receiver),
+      std::move(buffer_pool),

[miu, 2017/03/17 21:48:02]

Note: Mistakes like this can be avoided by biasing towards a more functional
style (i.e., less imperative style):

  return base::MakeUnique<media::VideoCaptureDeviceClient>(
      base::MakeUnique<VideoFrameReceiverOnIOThread>(receiver),
      new media::VideoCaptureBufferPoolImpl(
          base::MakeUnique<media::VideoCaptureBufferTrackerFactoryImpl>(),
          buffer_pool_max_buffer_count),
      base::Bind(&CreateGpuJpegDecoder,
                 base::Bind(&media::VideoFrameReceiver::OnFrameReadyInBuffer,
                            receiver)));

[chfremer, 2017/03/21 22:37:23]

Done.

       base::Bind(&CreateGpuJpegDecoder,
                  base::Bind(&media::VideoFrameReceiver::OnFrameReadyInBuffer,
                             receiver)));
@@ -298,18 +311,42 @@ void InProcessBuildableVideoCaptureDevice::OnDeviceStarted(
     std::unique_ptr<Ownership> context_reference,
     std::unique_ptr<media::VideoCaptureDevice> device) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (!device) {
-    callbacks->OnDeviceStartFailed(controller);
-    return;
+  switch (state_) {
+    case State::DEVICE_START_IN_PROGRESS:
+      if (!device) {
+        state_ = State::NO_DEVICE;
+        callbacks->OnDeviceStartFailed(controller);
+        return;
+      }
+      // Passing raw pointer |device.get()| to the controller is safe,
+      // because we take ownership of |device| and we call
+      // controller->SetConsumerFeedbackObserver(nullptr) before releasing
+      // |device|.
+      controller->SetConsumerFeedbackObserver(
+          base::MakeUnique<VideoFrameConsumerFeedbackObserverOnTaskRunner>(
+              device.get(), device_task_runner_));
+      device_ = std::move(device);
+      state_ = State::DEVICE_STARTED;
+      callbacks->DidStartDevice(controller);
+      return;
+    case State::DEVICE_START_ABORTING:
+      if (device) {
+        device_ = std::move(device);
+        state_ = State::DEVICE_STARTED;
+        // We do not move our |context_reference| to this invocation, because
+        // we still need it for the remainder of this method execution.
+        // Our implementation of ReleaseDeviceAsync() does not actually need the
+        // context while releasing the device.
+        ReleaseDeviceAsync(controller, nullptr);
+      }
+      state_ = State::NO_DEVICE;
+      callbacks->OnDeviceStartAborted();
+      return;
+    case State::NO_DEVICE:
+    case State::DEVICE_STARTED:
+      NOTREACHED();
+      return;
   }
-  // Passing raw pointer |device.get()| to the controller is safe,
-  // because we take ownership of |device| and we call
-  // controller->SetConsumerFeedbackObserver(nullptr) before releasing |device|.
-  controller->SetConsumerFeedbackObserver(
-      base::MakeUnique<VideoFrameConsumerFeedbackObserverOnTaskRunner>(
-          device.get(), device_task_runner_));
-  device_ = std::move(device);
-  callbacks->DidStartDevice(controller);
 }
 
 void InProcessBuildableVideoCaptureDevice::DoStartDeviceCaptureOnDeviceThread(