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

content/browser/renderer_host/media/in_process_buildable_video_capture_device.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_buildable_video_capture_device.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 47 matching lines @@
     const media::VideoCaptureJpegDecoder::DecodeDoneCB& decode_done_cb) {
   return base::MakeUnique<content::VideoCaptureGpuJpegDecoder>(decode_done_cb);
 }
 
 void StopAndReleaseDeviceOnDeviceThread(media::VideoCaptureDevice* device,
                                         base::OnceClosure done_cb) {
   SCOPED_UMA_HISTOGRAM_TIMER("Media.VideoCaptureManager.StopDeviceTime");
   device->StopAndDeAllocate();
   DVLOG(3) << "StopAndReleaseDeviceOnDeviceThread";
   delete device;
   std::move(done_cb).Run();

[miu, 2017/03/22 21:33:28]

nit: Is the std::move() necessary (and everywhere else you do this at the end of
a method)?

[chfremer, 2017/03/22 22:37:52]

Yes. I believe this is the only way to invoke Run() on a OnceCallback. It is
requiring a non-const R-value and otherwise hits a static_assert.

 }
 
 // The maximum number of video frame buffers in-flight at any one time. This
 // value should be based on the logical capacity of the capture pipeline, and
 // not on hardware performance.  For example, tab capture requires more buffers
 // than webcam capture because the pipeline is longer (it includes read-backs
 // pending in the GPU pipeline).
 const int kMaxNumberOfBuffers = 3;
 // TODO(miu): The value for tab capture should be determined programmatically.
 // http://crbug.com/460318
@@ skipping 13 matching lines @@
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(!device_);
 }
 
 void InProcessBuildableVideoCaptureDevice::CreateAndStartDeviceAsync(
     VideoCaptureController* controller,
     const media::VideoCaptureParams& params,
     BuildableDeviceCallbacks* callbacks,
     base::OnceClosure done_cb) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(state_ == State::NO_DEVICE);

[mcasas, 2017/03/24 21:54:36]

DCHECK_EQ(State::NO_DEVICE, state_) ?
Not sure if enum classes have operator<<,
if not, then this construction won't work and
you should disregard this comment :-)

[chfremer, 2017/03/31 17:25:37]

Done.

 
   const int max_buffers = (controller->stream_type() == MEDIA_TAB_VIDEO_CAPTURE
                                ? kMaxNumberOfBuffersForTabCapture
                                : kMaxNumberOfBuffers);
 
   auto device_client =
       CreateDeviceClient(max_buffers, controller->GetWeakPtrForIOThread());
 
   base::Closure start_capture_closure;
   switch (controller->stream_type()) {
@@ skipping 61 matching lines @@
                      std::move(after_start_capture_callback));
       break;
     }
     default: {
       NOTIMPLEMENTED();
       return;
     }
   }
 
   device_task_runner_->PostTask(FROM_HERE, start_capture_closure);
+  state_ = State::DEVICE_START_IN_PROGRESS;
 }
 
 void InProcessBuildableVideoCaptureDevice::ReleaseDeviceAsync(
     VideoCaptureController* controller,
     base::OnceClosure done_cb) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   controller->SetConsumerFeedbackObserver(nullptr);
-  if (!device_)
-    return;
-  media::VideoCaptureDevice* device_ptr = device_.release();
-
-  bool posting_task_succeeded = device_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&StopAndReleaseDeviceOnDeviceThread, device_ptr,
-                 base::Bind([](scoped_refptr<base::SingleThreadTaskRunner>) {},
-                            device_task_runner_)));
-  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, base::Bind([]() {}));
+  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();
+      bool posting_task_succeeded = device_task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(
+              &StopAndReleaseDeviceOnDeviceThread, device_ptr,
+              base::Bind([](scoped_refptr<base::SingleThreadTaskRunner>) {},
+                         device_task_runner_)));
+      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, base::Bind([]() {}));
+      }
+      state_ = State::NO_DEVICE;
+      return;
   }
   std::move(done_cb).Run();
 }
 
 bool InProcessBuildableVideoCaptureDevice::IsDeviceAlive() const {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   return device_ != nullptr;
 }
 
 void InProcessBuildableVideoCaptureDevice::GetPhotoCapabilities(
@@ skipping 76 matching lines @@
                             base::Unretained(this), device_.get(), window_id,
                             base::Passed(&done_cb)));
 }
 
 std::unique_ptr<media::VideoCaptureDeviceClient>
 InProcessBuildableVideoCaptureDevice::CreateDeviceClient(
     int buffer_pool_max_buffer_count,
     base::WeakPtr<media::VideoFrameReceiver> receiver) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  scoped_refptr<media::VideoCaptureBufferPool> buffer_pool_ =
+  return base::MakeUnique<media::VideoCaptureDeviceClient>(
+      base::MakeUnique<VideoFrameReceiverOnIOThread>(receiver),
       new media::VideoCaptureBufferPoolImpl(
           base::MakeUnique<media::VideoCaptureBufferTrackerFactoryImpl>(),
-          buffer_pool_max_buffer_count);
-
-  return base::MakeUnique<media::VideoCaptureDeviceClient>(
-      base::MakeUnique<VideoFrameReceiverOnIOThread>(receiver), buffer_pool_,
+          buffer_pool_max_buffer_count),
       base::Bind(&CreateGpuJpegDecoder,
                  base::Bind(&media::VideoFrameReceiver::OnFrameReadyInBuffer,
                             receiver)));
 }
 
 void InProcessBuildableVideoCaptureDevice::OnDeviceStarted(
     VideoCaptureController* controller,
     BuildableDeviceCallbacks* callbacks,
     base::OnceClosure done_cb,
     std::unique_ptr<media::VideoCaptureDevice> device) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (!device) {
-    callbacks->OnDeviceStartFailed(controller);
-    std::move(done_cb).Run();
-    return;
+  switch (state_) {
+    case State::DEVICE_START_IN_PROGRESS:
+      if (!device) {
+        state_ = State::NO_DEVICE;
+        callbacks->OnDeviceStartFailed(controller);
+        std::move(done_cb).Run();
+        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);
+      std::move(done_cb).Run();
+      return;
+    case State::DEVICE_START_ABORTING:
+      if (device) {
+        device_ = std::move(device);
+        state_ = State::DEVICE_STARTED;
+        // We do not move our |done_cb| to this invocation, because
+        // we still need it to stay alive for the remainder of this method
+        // execution. Our implementation of ReleaseDeviceAsync() does not
+        // actually need the context while releasing the device.
+        ReleaseDeviceAsync(controller, base::Bind([]() {}));
+      }
+      state_ = State::NO_DEVICE;
+      callbacks->OnDeviceStartAborted();
+      std::move(done_cb).Run();
+      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);
-  std::move(done_cb).Run();
 }
 
 void InProcessBuildableVideoCaptureDevice::DoStartDeviceCaptureOnDeviceThread(
     const media::VideoCaptureDeviceDescriptor& descriptor,
     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());
 
@@ skipping 91 matching lines @@
     !defined(OS_ANDROID)
   DesktopCaptureDevice* desktop_device =
       static_cast<DesktopCaptureDevice*>(device);
   desktop_device->SetNotificationWindowId(window_id);
   VLOG(2) << "Screen capture notification window passed on device thread.";
 #endif
   std::move(done_cb).Run();
 }
 
 }  // namespace content