ImageCapture: replace Mojo String/Array with stl/wtf string/vector

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 23da723eae8c78bb4712bab3a4267c42f9aad6f5..402f933ca57137222cbc9874f284c73529991c46 100644
--- a/content/browser/media/capture/image_capture_impl.cc
+++ b/content/browser/media/capture/image_capture_impl.cc
@@ -51,20 +51,19 @@ void RunFailedSetOptionsCallback(
 
 void RunTakePhotoCallbackOnUIThread(
     const ImageCaptureImpl::TakePhotoCallback& callback,
-    mojo::String mime_type,
-    mojo::Array<uint8_t> data) {
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(callback, mime_type, base::Passed(std::move(data))));
+    const std::string& mime_type,
+    const std::vector<uint8_t>& data) {
+  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+                          base::Bind(callback, mime_type, data));

[Wez, 2016/07/20 18:52:07]

|data| was previously move()'d, which was presumably just a pointer-swap, but
must now be copied - can we arrange to keep the move semantics?

[mcasas, 2016/07/20 22:57:01]

It's complicated due to the automatic generation
of the TakePhotoCallback, that includes const& for
|data|, but not impossible, so, done.

An alternative is to have a temporary
std::vector<uint8_t> data_copy = std::move(data);
but even then, due to |data| being const&, the
copy constructor would be used instead of the
move constructor, so one way or another, 
const_cast is the only way I saw.

[yzshen1, 2016/07/20 23:05:13]

I don't know whether it is necessary, but if you want here is one way to do it:

(1) define a mojom struct
// in mojom:
struct SomeData {
  array<uint8> data;
};

(2) create a custom typemapping between SomeData and
std::unique_ptr<std::vector<uint8_t>>, in the typemap config annotate that this
type is move-only:

type_mappings = [ "SomeData=std::unique_ptr<std::vector<uint8_t>>[move_only]" ]

[Wez, 2016/07/20 23:19:54]

I mentioned this because |data| seems like it's some sort of bulky image data,
and the old mojo::Array form was move-only, with [potentially] no copying
end-to-end - I'd be wary of introducing an extra copy for any form of "bulk"
data.

[yzshen1, 2016/07/20 23:27:36]

Agreed. Copy seems sad here. mcasas@ mentioned that he could use the const_cast
as a temp solution and add a TODO to address using typemapping separately. I am
personally fine with that.

 }
 
 void RunFailedTakePhotoCallback(const ImageCaptureImpl::TakePhotoCallback& cb) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  cb.Run("", mojo::Array<uint8_t>());
+  cb.Run("", std::vector<uint8_t>());
 }
 
 void GetCapabilitiesOnIOThread(
-    const mojo::String& source_id,
+    const std::string& source_id,
     MediaStreamManager* media_stream_manager,
     media::ScopedResultCallback<ImageCaptureImpl::GetCapabilitiesCallback>
         callback) {
@@ -80,7 +79,7 @@ void GetCapabilitiesOnIOThread(
 }
 
 void SetOptionsOnIOThread(
-    const mojo::String& source_id,
+    const std::string& source_id,
     MediaStreamManager* media_stream_manager,
     media::mojom::PhotoSettingsPtr settings,
     media::ScopedResultCallback<ImageCaptureImpl::SetOptionsCallback>
@@ -97,7 +96,7 @@ void SetOptionsOnIOThread(
 }
 
 void TakePhotoOnIOThread(
-    const mojo::String& source_id,
+    const std::string& source_id,
     MediaStreamManager* media_stream_manager,
     media::ScopedResultCallback<ImageCaptureImpl::TakePhotoCallback> callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -123,7 +122,7 @@ void ImageCaptureImpl::Create(
 ImageCaptureImpl::~ImageCaptureImpl() {}
 
 void ImageCaptureImpl::GetCapabilities(
-    const mojo::String& source_id,
+    const std::string& source_id,
     const GetCapabilitiesCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
@@ -138,7 +137,7 @@ void ImageCaptureImpl::GetCapabilities(
                  base::Passed(&scoped_callback)));
 }
 
-void ImageCaptureImpl::SetOptions(const mojo::String& source_id,
+void ImageCaptureImpl::SetOptions(const std::string& source_id,
                                   media::mojom::PhotoSettingsPtr settings,
                                   const SetOptionsCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -154,7 +153,7 @@ void ImageCaptureImpl::SetOptions(const mojo::String& source_id,
                  base::Passed(&settings), base::Passed(&scoped_callback)));
 }
 
-void ImageCaptureImpl::TakePhoto(const mojo::String& source_id,
+void ImageCaptureImpl::TakePhoto(const std::string& source_id,
                                  const TakePhotoCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);