[Mojo] Add [Native] type traits for base::Values

[Mojo] Add [Native] type traits for base::Values

Mojo struct traits aren't quite ready for something like base::Value,
but adding [native] traits will allow base::DictionaryValue and
base::ListValue to be passed in mojo messages.

BUG=614410
Committed: https://crrev.com/a6458a0c45099522f647b63337f94ea4e98c9f94
Cr-Commit-Position: refs/heads/master@{#395701}

[Devlin, 2016-05-24 17:05:48]

Description was changed from

==========
[Mojo] Add [Native] type traits for base::Values

Mojo struct traits aren't quite ready for something like base::Value,
but adding [native] traits will allow base::DictionaryValue and
base::ListValue to be passed in IPCs.

BUG=614410
==========

to

==========
[Mojo] Add [Native] type traits for base::Values

Mojo struct traits aren't quite ready for something like base::Value,
but adding [native] traits will allow base::DictionaryValue and
base::ListValue to be passed in mojo messages.

BUG=614410
==========

[Devlin, 2016-05-24 17:28:13]

Patchset #1 (id:1) has been deleted

[Devlin, 2016-05-24 17:31:48]

rdevlin.cronin@chromium.org changed reviewers:
+ rockot@chromium.org, yzshen@chromium.org

[Devlin, 2016-05-24 17:31:49]

Per Yuzhu's suggestion, starting with a [native] type traits version leveraging
the existing IPC pickling.  I've forked the CL so that the other stays around
for reference and in case we want to go back to it a little later.

[Ken Rockot(use gerrit already), 2016-05-24 17:39:02]

Cool, lgtm

[yzshen1, 2016-05-24 17:54:41]

https://codereview.chromium.org/2013433002/diff/20001/mojo/common/common_cust...
File mojo/common/common_custom_types.typemap (right):

https://codereview.chromium.org/2013433002/diff/20001/mojo/common/common_cust...
mojo/common/common_custom_types.typemap:6: public_headers = [
"//base/files/file_path.h", "//base/values.h" ]
Please put each path on a separate line, just like what you did for
"type_mappings".

https://codereview.chromium.org/2013433002/diff/20001/mojo/common/common_cust...
File mojo/common/common_custom_types_unittest.cc (right):

https://codereview.chromium.org/2013433002/diff/20001/mojo/common/common_cust...
mojo/common/common_custom_types_unittest.cc:102: base::MessageLoop message_loop;
Please consider moving this into a test fixture class, now that every test needs
it.

https://codereview.chromium.org/2013433002/diff/20001/mojo/common/common_cust...
mojo/common/common_custom_types_unittest.cc:108: auto TestDictEqual = [&ptr,
&error](const base::DictionaryValue& value) {
I feel that this testing code is fairly hard to read. Comparing with simply:

- Construct the value.
- Directly call Bounce*Value() with the value.
- Check equity.

But that may be just my personal preference. So please consider this optional.

[Devlin, 2016-05-24 18:04:31]

Patchset #2 (id:40001) has been deleted

[Devlin, 2016-05-24 18:05:57]

https://codereview.chromium.org/2013433002/diff/20001/mojo/common/common_cust...
File mojo/common/common_custom_types.typemap (right):

https://codereview.chromium.org/2013433002/diff/20001/mojo/common/common_cust...
mojo/common/common_custom_types.typemap:6: public_headers = [
"//base/files/file_path.h", "//base/values.h" ]
On 2016/05/24 17:54:41, yzshen1 wrote:
> Please put each path on a separate line, just like what you did for
> "type_mappings".

Done.

https://codereview.chromium.org/2013433002/diff/20001/mojo/common/common_cust...
File mojo/common/common_custom_types_unittest.cc (right):

https://codereview.chromium.org/2013433002/diff/20001/mojo/common/common_cust...
mojo/common/common_custom_types_unittest.cc:102: base::MessageLoop message_loop;
On 2016/05/24 17:54:41, yzshen1 wrote:
> Please consider moving this into a test fixture class, now that every test
needs
> it.

Done.

https://codereview.chromium.org/2013433002/diff/20001/mojo/common/common_cust...
mojo/common/common_custom_types_unittest.cc:108: auto TestDictEqual = [&ptr,
&error](const base::DictionaryValue& value) {
On 2016/05/24 17:54:41, yzshen1 wrote:
> I feel that this testing code is fairly hard to read. Comparing with simply:
> 
> - Construct the value.
> - Directly call Bounce*Value() with the value.
> - Check equity.
> 
> But that may be just my personal preference. So please consider this optional.

Agreed.  This had a lot more value when we were testing 7 types of values
instead of 2.

[yzshen1, 2016-05-24 18:09:03]

lgtm

[Devlin, 2016-05-24 18:09:35]

The CQ bit was checked by rdevlin.cronin@chromium.org

[Devlin, 2016-05-24 18:09:36]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org
Link to the patchset: https://codereview.chromium.org/2013433002/#ps60001
(title: "Yuzhu's")

[commit-bot: I haz the power, 2016-05-24 18:10:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2013433002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2013433002/60001

[commit-bot: I haz the power, 2016-05-24 21:34:09]

Description was changed from

==========
[Mojo] Add [Native] type traits for base::Values

Mojo struct traits aren't quite ready for something like base::Value,
but adding [native] traits will allow base::DictionaryValue and
base::ListValue to be passed in mojo messages.

BUG=614410
==========

to

==========
[Mojo] Add [Native] type traits for base::Values

Mojo struct traits aren't quite ready for something like base::Value,
but adding [native] traits will allow base::DictionaryValue and
base::ListValue to be passed in mojo messages.

BUG=614410
==========

[commit-bot: I haz the power, 2016-05-24 21:34:10]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-05-24 21:35:37]

Description was changed from

==========
[Mojo] Add [Native] type traits for base::Values

Mojo struct traits aren't quite ready for something like base::Value,
but adding [native] traits will allow base::DictionaryValue and
base::ListValue to be passed in mojo messages.

BUG=614410
==========

to

==========
[Mojo] Add [Native] type traits for base::Values

Mojo struct traits aren't quite ready for something like base::Value,
but adding [native] traits will allow base::DictionaryValue and
base::ListValue to be passed in mojo messages.

BUG=614410

Committed: https://crrev.com/a6458a0c45099522f647b63337f94ea4e98c9f94
Cr-Commit-Position: refs/heads/master@{#395701}
==========

[commit-bot: I haz the power, 2016-05-24 21:35:38]

Patchset 2 (id:??) landed as
https://crrev.com/a6458a0c45099522f647b63337f94ea4e98c9f94
Cr-Commit-Position: refs/heads/master@{#395701}