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

content/browser/media/capture/image_capture_impl.cc

 // Copyright 2016 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/media/capture/image_capture_impl.h"
 
 #include <utility>
 
 #include "base/bind_helpers.h"
 #include "content/browser/browser_main_loop.h"
@@ skipping 33 matching lines @@
 }
 
 void RunFailedSetOptionsCallback(
     const ImageCaptureImpl::SetOptionsCallback& cb) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   cb.Run(false);
 }
 
 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) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   const int session_id =
       media_stream_manager->VideoDeviceIdToSessionId(source_id);
 
   if (session_id == StreamDeviceInfo::kNoId)
     return;
   media_stream_manager->video_capture_manager()->GetPhotoCapabilities(
       session_id, std::move(callback));
 }
 
 void SetOptionsOnIOThread(
-    const mojo::String& source_id,
+    const std::string& source_id,
     MediaStreamManager* media_stream_manager,
     media::mojom::PhotoSettingsPtr settings,
     media::ScopedResultCallback<ImageCaptureImpl::SetOptionsCallback>
         callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   const int session_id =
       media_stream_manager->VideoDeviceIdToSessionId(source_id);
 
   if (session_id == StreamDeviceInfo::kNoId)
     return;
   media_stream_manager->video_capture_manager()->SetPhotoOptions(
       session_id, std::move(settings), std::move(callback));
 }
 
 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);
 
   const int session_id =
       media_stream_manager->VideoDeviceIdToSessionId(source_id);
 
   if (session_id == StreamDeviceInfo::kNoId)
     return;
   media_stream_manager->video_capture_manager()->TakePhoto(session_id,
                                                            std::move(callback));
 }
 
 }  // anonymous namespace
 
 // static
 void ImageCaptureImpl::Create(
     mojo::InterfaceRequest<media::mojom::ImageCapture> request) {
   // |binding_| will take ownership of ImageCaptureImpl.
   new ImageCaptureImpl(std::move(request));
 }
 
 ImageCaptureImpl::~ImageCaptureImpl() {}
 
 void ImageCaptureImpl::GetCapabilities(
-    const mojo::String& source_id,
+    const std::string& source_id,
     const GetCapabilitiesCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   media::ScopedResultCallback<GetCapabilitiesCallback> scoped_callback(
       base::Bind(&RunGetCapabilitiesCallbackOnUIThread, callback),
       media::BindToCurrentLoop(base::Bind(&RunFailedGetCapabilitiesCallback)));
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&GetCapabilitiesOnIOThread, source_id,
                  BrowserMainLoop::GetInstance()->media_stream_manager(),
                  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);
 
   media::ScopedResultCallback<SetOptionsCallback> scoped_callback(
       base::Bind(&RunSetOptionsCallbackOnUIThread, callback),
       media::BindToCurrentLoop(base::Bind(&RunFailedSetOptionsCallback)));
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&SetOptionsOnIOThread, source_id,
                  BrowserMainLoop::GetInstance()->media_stream_manager(),
                  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);
 
   media::ScopedResultCallback<TakePhotoCallback> scoped_callback(
       base::Bind(&RunTakePhotoCallbackOnUIThread, callback),
       media::BindToCurrentLoop(base::Bind(&RunFailedTakePhotoCallback)));
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&TakePhotoOnIOThread, source_id,
                  BrowserMainLoop::GetInstance()->media_stream_manager(),
                  base::Passed(&scoped_callback)));
 }
 
 ImageCaptureImpl::ImageCaptureImpl(
     mojo::InterfaceRequest<media::mojom::ImageCapture> request)
     : binding_(this, std::move(request)) {}
 
 }  // namespace content