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..16d8083c68b860998926b5d6bfe294c77ab84e75 100644
--- a/content/browser/media/capture/image_capture_impl.cc
+++ b/content/browser/media/capture/image_capture_impl.cc
@@ -5,10 +5,33 @@
 #include "content/browser/media/capture/image_capture_impl.h"
 
 #include "base/bind_helpers.h"
+#include "content/browser/browser_main_loop.h"

[miu, 2016/05/04 23:45:50]

Doesn't look like this include is needed.

[mcasas, 2016/05/05 00:57:12]

Is needed for BrowserMainLoop::GetInstance()
in l.52.

+#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::string> data) {

[miu, 2016/05/04 23:45:50]

Any particular reason you're mixing use of std::string and std::vector<uint8_t>?
Seems that copying could be avoided if you just used the vector throughout.

[mcasas, 2016/05/05 00:57:12]

Good point, yeah. The previous CL shifted between std::string
and std::vector<uint8_t> and this is a remnant. Seeing the
next CL in line, std::vector<uint8_t> looks like a better type
throughout, so moving in that direction.

+  DCHECK(data.get());
+  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    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, std::vector<uint8_t>(data->begin(), data->end()));
+}
+
+}  // anonymous namespace
+
 // static
 void ImageCaptureImpl::Create(
     mojo::InterfaceRequest<blink::mojom::ImageCapture> request) {
@@ -18,17 +41,40 @@ 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(&ImageCaptureImpl::TakePhotoOnIOThread, base::Unretained(this),

[miu, 2016/05/04 23:45:50]

base::Unretained(this) is safe, right?  (I'm not that familiar with Mojo's
lifecycle management.)

[mcasas, 2016/05/05 00:57:12]

Phew, it might be hard to reason as to if it's really 
safe or not, instead I made TakePhotoOnIOThread()
file-static, so there's no confusion.

+                 source_id, callback,
+                 BrowserMainLoop::GetInstance()->media_stream_manager()));
 }
 
 ImageCaptureImpl::ImageCaptureImpl(
     mojo::InterfaceRequest<blink::mojom::ImageCapture> request)
     : binding_(this, std::move(request)) {}
 
+void ImageCaptureImpl::TakePhotoOnIOThread(
+    const mojo::String& source_id,
+    const 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::string> empty_string(new std::string(""));
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,

[miu, 2016/05/04 23:45:50]

IMHO, just call RunTakePhotoCallback() directly (i.e., without the PostTask())
here.

[mcasas, 2016/05/05 00:57:12]

Done.

+                            base::Bind(&RunTakePhotoCallback, callback, "",
+                                       base::Passed(&empty_string)));
+    return;
+  }
+
+  const bool result = media_stream_manager->video_capture_manager()->TakePhoto(
+      session_id, base::Bind(&RunTakePhotoCallback, callback));
+  DCHECK(result);
+}
+
 }  // namespace content