Mojo C++ binding: make device/usb mojom targets use STD string/vector types

Mojo C++ binding: make device/usb mojom targets use STD string/vector types

BUG=624136
Committed: https://crrev.com/1d90ae44053b41fae6c202e9183b7c3ee795bc6c
Cr-Commit-Position: refs/heads/master@{#412092}

[Reilly Grant (use Gerrit), 2016-08-09 21:00:22]

The CQ bit was checked by reillyg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-09 21:01:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-09 21:29:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-09 21:29:08]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[Reilly Grant (use Gerrit), 2016-08-09 23:46:02]

The CQ bit was checked by reillyg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-09 23:46:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Reilly Grant (use Gerrit), 2016-08-10 00:59:03]

reillyg@chromium.org changed reviewers:
+ rockot@chromium.org

[Reilly Grant (use Gerrit), 2016-08-10 00:59:04]

Please take a look.

[commit-bot: I haz the power, 2016-08-10 01:27:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-10 01:27:43]

Dry run: This issue passed the CQ dry run.

[Ken Rockot(use gerrit already), 2016-08-10 01:38:41]

lgtm

https://codereview.chromium.org/2234443002/diff/20001/device/usb/mojo/device_...
File device/usb/mojo/device_impl.cc (right):

https://codereview.chromium.org/2234443002/diff/20001/device/usb/mojo/device_...
device/usb/mojo/device_impl.cc:35: void
OnTransferIn(std::unique_ptr<Device::GenericTransferInCallback> callback,
Optional nit(s): As discussed, I really don't think wrapping the callbacks in
unique_ptrs buys you anything useful anymore.

[Reilly Grant (use Gerrit), 2016-08-10 06:45:36]

The CQ bit was checked by reillyg@chromium.org

[commit-bot: I haz the power, 2016-08-10 06:45:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 06:52:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-10 06:52:49]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Reilly Grant (use Gerrit), 2016-08-10 18:02:30]

reillyg@chromium.org changed reviewers:
+ bauerb@chromium.org

[Reilly Grant (use Gerrit), 2016-08-10 18:02:31]

bauerb@, please take a look at //chrome/browser/android and
//chrome/browser/ui/android.
dcheng@, please take a look //mojo/common/common_type_converters.h.

[Reilly Grant (use Gerrit), 2016-08-10 20:24:47]

reillyg@chromium.org changed reviewers:
+ dcheng@chromium.org

[Reilly Grant (use Gerrit), 2016-08-10 20:24:49]

Adding dcheng@ for real this time.

[Bernhard Bauer, 2016-08-11 11:01:29]

Android LGTM.

[dcheng, 2016-08-15 18:18:16]

https://codereview.chromium.org/2234443002/diff/20001/device/usb/mojo/device_...
File device/usb/mojo/device_manager_impl_unittest.cc (right):

https://codereview.chromium.org/2234443002/diff/20001/device/usb/mojo/device_...
device/usb/mojo/device_manager_impl_unittest.cc:127: options->filters.emplace();
?

Isn't option->filters a vector here? How does a call to emplace() with no
arguments work?

https://codereview.chromium.org/2234443002/diff/20001/mojo/common/common_type...
File mojo/common/common_type_converters.h (right):

https://codereview.chromium.org/2234443002/diff/20001/mojo/common/common_type...
mojo/common/common_type_converters.h:74: };
Actually, let's not add this type converter please. We should be trying to
reduce usage of this, and adding this will make it easier to add new
dependencies.

That means that this CL will have to use type mapping for device USB types too
(or that should be done in a prerequisite CL)

See https://www.chromium.org/developers/design-documents/mojo/type-mapping for
more information about this.

[Reilly Grant (use Gerrit), 2016-08-15 21:39:02]

https://codereview.chromium.org/2234443002/diff/20001/device/usb/mojo/device_...
File device/usb/mojo/device_manager_impl_unittest.cc (right):

https://codereview.chromium.org/2234443002/diff/20001/device/usb/mojo/device_...
device/usb/mojo/device_manager_impl_unittest.cc:127: options->filters.emplace();
On 2016/08/15 at 18:18:15, dcheng wrote:
> ?
> 
> Isn't option->filters a vector here? How does a call to emplace() with no
arguments work?

option->filters is a base::Optional<std::vector<>>. Calling emplace() runs the
std::vector<> constructor so that the ->push_back() call on the next line is
valid.

https://codereview.chromium.org/2234443002/diff/20001/mojo/common/common_type...
File mojo/common/common_type_converters.h (right):

https://codereview.chromium.org/2234443002/diff/20001/mojo/common/common_type...
mojo/common/common_type_converters.h:74: };
On 2016/08/15 at 18:18:15, dcheng wrote:
> Actually, let's not add this type converter please. We should be trying to
reduce usage of this, and adding this will make it easier to add new
dependencies.
> 
> That means that this CL will have to use type mapping for device USB types too
(or that should be done in a prerequisite CL)
> 
> See https://www.chromium.org/developers/design-documents/mojo/type-mapping for
more information about this.

As discussed OOB I've moved this to a more private header. Converting device/usb
to use typemaps is a larger project than I want to tackle in this CL.

[Reilly Grant (use Gerrit), 2016-08-15 21:39:11]

The CQ bit was checked by reillyg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-15 21:39:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dcheng, 2016-08-15 21:41:28]

LGTM but I hope we can fix the typemapping issue sooner rather than later, to
prevent potential ODR issues down the road.

https://codereview.chromium.org/2234443002/diff/40001/device/usb/mojo/type_co...
File device/usb/mojo/type_converters.h (right):

https://codereview.chromium.org/2234443002/diff/40001/device/usb/mojo/type_co...
device/usb/mojo/type_converters.h:109: template <typename A, typename B>
The main thing I'm worried about is this leading to ODR violations. I guess
we'll just have to hope...

[commit-bot: I haz the power, 2016-08-15 23:14:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-15 23:14:44]

Dry run: This issue passed the CQ dry run.

[Reilly Grant (use Gerrit), 2016-08-15 23:20:57]

The CQ bit was checked by reillyg@chromium.org

[Reilly Grant (use Gerrit), 2016-08-15 23:20:58]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
rockot@chromium.org
Link to the patchset: https://codereview.chromium.org/2234443002/#ps40001
(title: "Move the vector converter into a private header.")

[commit-bot: I haz the power, 2016-08-15 23:21:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-15 23:27:07]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-15 23:29:07]

Description was changed from

==========
Mojo C++ binding: make device/usb mojom targets use STD string/vector types

BUG=624136
==========

to

==========
Mojo C++ binding: make device/usb mojom targets use STD string/vector types

BUG=624136

Committed: https://crrev.com/1d90ae44053b41fae6c202e9183b7c3ee795bc6c
Cr-Commit-Position: refs/heads/master@{#412092}
==========

[commit-bot: I haz the power, 2016-08-15 23:29:09]

Patchset 3 (id:??) landed as
https://crrev.com/1d90ae44053b41fae6c202e9183b7c3ee795bc6c
Cr-Commit-Position: refs/heads/master@{#412092}