Media Stream Image Capture (4): wire takePhoto and implement in FakeVCDevice

content/browser/media/capture/image_capture_impl.cc

Index: content/browser/media/capture/image_capture_impl.cc
diff --git a/content/browser/media/capture/image_capture_impl.cc b/content/browser/media/capture/image_capture_impl.cc
index bfc8b0d0aec9be4af5c1e59d29aeb5512a03753b..c2b050ff1d1716f12eb1a3bfbea8999601f502a4 100644
--- a/content/browser/media/capture/image_capture_impl.cc
+++ b/content/browser/media/capture/image_capture_impl.cc
@@ -5,10 +5,53 @@
 #include "content/browser/media/capture/image_capture_impl.h"
 
 #include "base/bind_helpers.h"
+#include "content/browser/browser_main_loop.h"
+#include "content/browser/renderer_host/media/media_stream_manager.h"
+#include "content/browser/renderer_host/media/video_capture_manager.h"
+#include "content/common/media/media_stream_options.h"
 #include "content/public/browser/browser_thread.h"
+#include "media/capture/video/video_capture_device.h"
 
 namespace content {
 
+namespace {
+
+void RunTakePhotoCallback(const ImageCaptureImpl::TakePhotoCallback& callback,
+                          const std::string& mime_type,
+                          std::unique_ptr<std::vector<uint8_t>> data) {
+  DCHECK(data.get());
+  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {

[miu, 2016/05/05 22:04:24]

FWICT, the trampoline to run the callback on the UI thread will always be
needed. Thus, can this whole RunTakePhotoCallback() method just be simply:

  DCHECK(data.get());
  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
                          base::Bind(callback, mime_type,
                                     mojo::Array<uint8_t>::From(*data)));

(And, even if there is a way RunTakePhotoCallback() is already running on the UI
thread, it wouldn't hurt to PostTask() anyway.)

[mcasas, 2016/05/05 23:05:50]

Done.

+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                            base::Bind(&RunTakePhotoCallback, callback,
+                                       mime_type, base::Passed(&data)));
+    return;
+  }
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  callback.Run(mime_type, mojo::Array<uint8_t>::From(*data));
+}
+
+void TakePhotoOnIOThread(const mojo::String& source_id,
+                         const ImageCaptureImpl::TakePhotoCallback& callback,
+                         MediaStreamManager* media_stream_manager) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  const int session_id =
+      media_stream_manager->VideoDeviceIdToSessionId(source_id);
+
+  if (session_id == StreamDeviceInfo::kNoId) {
+    std::unique_ptr<std::vector<uint8_t>> empty_vector(
+        new std::vector<uint8_t>());
+    RunTakePhotoCallback(callback, "", std::move(empty_vector));
+    return;
+  }
+
+  const bool result = media_stream_manager->video_capture_manager()->TakePhoto(
+      session_id, base::Bind(&RunTakePhotoCallback, callback));
+  DCHECK(result);

[miu, 2016/05/05 22:04:24]

This feels brittle. It's possible for the TakePhoto() call on the UI thread to
be made on a device that is shutdown by the time TakePhotoOnIOThread() runs
later on the IO thread.

To keep things sane, I would suggest you guarantee the
ImageCaptureImpl::TakePhotoCallback will be run regardless of success/failure at
any point in the process. The if-statement above could be:

  if (session_id == StreamDeviceInfo::kNoId ||
      !media_stream_manager->video_capture_manager()->TakePhoto(...)) {
    // run callback w/ empty vector
  }

[mcasas, 2016/05/05 23:05:50]

Done.

+}
+
+}  // anonymous namespace
+
 // static
 void ImageCaptureImpl::Create(
     mojo::InterfaceRequest<blink::mojom::ImageCapture> request) {
@@ -18,13 +61,14 @@ void ImageCaptureImpl::Create(
 
 ImageCaptureImpl::~ImageCaptureImpl() {}
 
-void ImageCaptureImpl::TakePhoto(const mojo::String& /* source_id */,
+void ImageCaptureImpl::TakePhoto(const mojo::String& source_id,
                                  const TakePhotoCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
-  // TODO(mcasas): Implement this feature., https://crbug.com/518807.
-  mojo::Array<uint8_t> data(1);
-  callback.Run("text/plain", std::move(data));
+  // media_stream_manager() can only be called on UI thread.
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&TakePhotoOnIOThread, source_id, callback,
+                 BrowserMainLoop::GetInstance()->media_stream_manager()));
 }
 
 ImageCaptureImpl::ImageCaptureImpl(