[ABANDONED] Make it possible to send unique_ptr<Value> over IPC/Mojo.

Make it possible to send unique_ptr<Value> over IPC/Mojo.

I will need this for Reporting, which needs to accept "any valid JSON" 
as a parameter to a method exported over Mojo.

BUG=

[Julia Tuttle, 2016-11-03 20:29:48]

Description was changed from

==========
Make it possible to send unique_ptr<Value> over IPC/Mojo.

BUG=
==========

to

==========
Make it possible to send unique_ptr<Value> over IPC/Mojo.

I will need this for Reporting, which needs to accept "any valid JSON" 
as a parameter to a method exported over Mojo.

BUG=
==========

[Julia Tuttle, 2016-11-03 20:29:48]

juliatuttle@chromium.org changed reviewers:
+ tsepez@chromium.org

[Julia Tuttle, 2016-11-03 20:29:57]

PTAL, tsepez.

[Tom Sepez, 2016-11-03 20:50:37]

On 2016/11/03 20:29:57, Julia Tuttle wrote:
> PTAL, tsepez.

We've shied away from this in the past; is there a reason you don't pass
ptr.release() ?

[Tom Sepez, 2016-11-03 20:54:51]

I take it back, looks like someone has already done this however

https://codereview.chromium.org/2474783004/diff/1/ipc/ipc_message_utils.h
File ipc/ipc_message_utils.h (right):

https://codereview.chromium.org/2474783004/diff/1/ipc/ipc_message_utils.h#new...
ipc/ipc_message_utils.h:969: template <class P>
Wondering why this partial specialization doesn't work for you.

[Tom Sepez, 2016-11-03 20:59:58]

tsepez@chromium.org changed reviewers:
+ yzshen@google.com

[Tom Sepez, 2016-11-03 20:59:59]

+yzshen - surprised we don't already have a base::Value in some other .mojom.

[Julia Tuttle, 2016-11-03 21:01:38]

On 2016/11/03 20:54:51, Tom Sepez wrote:
> I take it back, looks like someone has already done this however
> 
> https://codereview.chromium.org/2474783004/diff/1/ipc/ipc_message_utils.h
> File ipc/ipc_message_utils.h (right):
> 
>
https://codereview.chromium.org/2474783004/diff/1/ipc/ipc_message_utils.h#new...
> ipc/ipc_message_utils.h:969: template <class P>
> Wondering why this partial specialization doesn't work for you.

Value is polymorphic, and the Read method in that specialization creates P
itself (in this case, Value), which means that even if I write a ReadParam for
Value, it can't actually return a ListValue or DictionaryValue or such.

[Julia Tuttle, 2016-11-03 21:03:24]

On 2016/11/03 20:50:37, Tom Sepez wrote:
> On 2016/11/03 20:29:57, Julia Tuttle wrote:
> > PTAL, tsepez.
> 
> We've shied away from this in the past; is there a reason you don't pass
> ptr.release() ?

Without any new code, I think this has the same problem as the unique_ptr<P>
specialization. I could rewrite mine to be a specialization for Value*, instead
of a specialization for unique_ptr<Value>, if you'd prefer.

[Tom Sepez, 2016-11-03 21:53:21]

On 2016/11/03 21:03:24, Julia Tuttle wrote:
> On 2016/11/03 20:50:37, Tom Sepez wrote:
> > On 2016/11/03 20:29:57, Julia Tuttle wrote:
> > > PTAL, tsepez.
> > 
> > We've shied away from this in the past; is there a reason you don't pass
> > ptr.release() ?
> 
> Without any new code, I think this has the same problem as the unique_ptr<P>
> specialization. I could rewrite mine to be a specialization for Value*,
instead
> of a specialization for unique_ptr<Value>, if you'd prefer.

Ah, in the past folks have worked around this by wrapping the actual data in a
ListValue so that you're always passing a list of one element, and then
unwrapping it on the other side. Does that work here?

[yzshen1, 2016-11-03 21:59:23]

yzshen@chromium.org changed reviewers:
+ yzshen@chromium.org

[yzshen1, 2016-11-03 21:59:24]

As Tom said, usually we use a ListValue as container and pass that.

The code itself works correctly so LGTM, please wait for Tom's opinion.

https://codereview.chromium.org/2474783004/diff/1/ipc/ipc_message_utils.cc
File ipc/ipc_message_utils.cc (right):

https://codereview.chromium.org/2474783004/diff/1/ipc/ipc_message_utils.cc#ne...
ipc/ipc_message_utils.cc:679: std::string json = "nullptr";
nit: maybe default to empty and only set "nullptr" when p is null (which should
usually be a minor case)?

[Tom Sepez, 2016-11-03 22:04:05]

On 2016/11/03 21:59:24, yzshen1 wrote:
> As Tom said, usually we use a ListValue as container and pass that.
> 
> The code itself works correctly so LGTM, please wait for Tom's opinion.
> 
> https://codereview.chromium.org/2474783004/diff/1/ipc/ipc_message_utils.cc
> File ipc/ipc_message_utils.cc (right):
> 
>
https://codereview.chromium.org/2474783004/diff/1/ipc/ipc_message_utils.cc#ne...
> ipc/ipc_message_utils.cc:679: std::string json = "nullptr";
> nit: maybe default to empty and only set "nullptr" when p is null (which
should
> usually be a minor case)?

Well, we're going to need tests if we're going to introduce this.  I'd prefer
going
with the wrapper, even json parser does this:
https://cs.chromium.org/chromium/src/components/safe_json/utility/safe_json_p...

[Julia Tuttle, 2016-11-03 22:42:37]

Alright, sounds like the wrapper trick is simpler. I'll do that instead.