Implement IPC::ParamTraits<mojo::MessagePipeHandle>

Implement IPC::ParamTraits<mojo::MessagePipeHandle>

This allows MessagePipeHandle to be an IPC message member.
Note that this assumes underlying channel being a ChannelMojo
and can be used in content/  only after ChannelMojo settles.

BUG=448190
TEST=IPCChannelMojoTest.ParamTrait*
R=viettrungluu@chromium.org

Committed: https://crrev.com/5656ae68841a3e3ddaac75cf2983d9052d71d0d5
Cr-Commit-Position: refs/heads/master@{#323265}

Committed: https://crrev.com/2b85d21119ce534940ccd2ff57727cd4b428081c
Cr-Commit-Position: refs/heads/master@{#323370}

Committed: https://crrev.com/438a2eedbb124eb3fc41ea17a4701eb7d3869af3
Cr-Commit-Position: refs/heads/master@{#323629}

[viettrungluu, 2015-03-31 22:35:52]

Is there any reasonable way to make this deal with ScopedMessagePipeHandles
instead? (Using just MessagePipeHandle is likely to lead to leaks[*], at least
under some circumstances -- e.g., what happens when an IPC channel is broken
with enqueued messages containing message pipe handles?)

[*] And maybe also use-after-close/transfer, though that's less likely.

https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
File ipc/mojo/ipc_mojo_param_traits.cc (right):

https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
ipc/mojo/ipc_mojo_param_traits.cc:1: // Copyright (c) 2015 The Chromium Authors.
All rights reserved.
"

https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
ipc/mojo/ipc_mojo_param_traits.cc:37: l->append("MojoMessageHelper()");
Shouldn't you log something like mojo::MessagePipeHandle instead? (You could
even log its value....)

https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
File ipc/mojo/ipc_mojo_param_traits.h (right):

https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
ipc/mojo/ipc_mojo_param_traits.h:1: // Copyright (c) 2015 The Chromium Authors.
All rights reserved.
nit: no "(c)" in the current copyright statement

[Hajime Morrita, 2015-03-31 22:45:07]

> Is there any reasonable way to make this deal with  
> ScopedMessagePipeHandles
> instead?

No, unfortunately :-(
All things have to be copyable around IPC::Message.
I think that this is one of the worst part of the design
of IPC module.

https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
File ipc/mojo/ipc_mojo_param_traits.cc (right):

https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
ipc/mojo/ipc_mojo_param_traits.cc:1: // Copyright (c) 2015 The Chromium Authors.
All rights reserved.
On 2015/03/31 22:35:52, viettrungluu wrote:
> "

Done.

https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
ipc/mojo/ipc_mojo_param_traits.cc:37: l->append("MojoMessageHelper()");
On 2015/03/31 22:35:52, viettrungluu wrote:
> Shouldn't you log something like mojo::MessagePipeHandle instead? (You could
> even log its value....)

Oops...

https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
File ipc/mojo/ipc_mojo_param_traits.h (right):

https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
ipc/mojo/ipc_mojo_param_traits.h:1: // Copyright (c) 2015 The Chromium Authors.
All rights reserved.
On 2015/03/31 22:35:52, viettrungluu wrote:
> nit: no "(c)" in the current copyright statement

Done.

[viettrungluu, 2015-04-01 16:11:32]

On 2015/03/31 22:45:07, Hajime Morrita wrote:
> > Is there any reasonable way to make this deal with  
> > ScopedMessagePipeHandles
> > instead?
> 
> No, unfortunately :-(
> All things have to be copyable around IPC::Message.
> I think that this is one of the worst part of the design
> of IPC module.

I guess we also have this problem with file descriptors? :(

LGTM then, I guess....

> 
>
https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
> File ipc/mojo/ipc_mojo_param_traits.cc (right):
> 
>
https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
> ipc/mojo/ipc_mojo_param_traits.cc:1: // Copyright (c) 2015 The Chromium
Authors.
> All rights reserved.
> On 2015/03/31 22:35:52, viettrungluu wrote:
> > "
> 
> Done.
> 
>
https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
> ipc/mojo/ipc_mojo_param_traits.cc:37: l->append("MojoMessageHelper()");
> On 2015/03/31 22:35:52, viettrungluu wrote:
> > Shouldn't you log something like mojo::MessagePipeHandle instead? (You could
> > even log its value....)
> 
> Oops...
> 
>
https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
> File ipc/mojo/ipc_mojo_param_traits.h (right):
> 
>
https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
> ipc/mojo/ipc_mojo_param_traits.h:1: // Copyright (c) 2015 The Chromium
Authors.
> All rights reserved.
> On 2015/03/31 22:35:52, viettrungluu wrote:
> > nit: no "(c)" in the current copyright statement
> 
> Done.

[Hajime Morrita, 2015-04-01 17:02:56]

On 2015/04/01 16:11:32, viettrungluu wrote:
> On 2015/03/31 22:45:07, Hajime Morrita wrote:
> > > Is there any reasonable way to make this deal with  
> > > ScopedMessagePipeHandles
> > > instead?
> > 
> > No, unfortunately :-(
> > All things have to be copyable around IPC::Message.
> > I think that this is one of the worst part of the design
> > of IPC module.
> 
> I guess we also have this problem with file descriptors? :(
Yeah, this is exactly why we cannot get rid of awkward classes like this ...
https://code.google.com/p/chromium/codesearch#chromium/src/base/file_descript...

> 
> LGTM then, I guess....
> 


> > 
> >
>
https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
> > File ipc/mojo/ipc_mojo_param_traits.cc (right):
> > 
> >
>
https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
> > ipc/mojo/ipc_mojo_param_traits.cc:1: // Copyright (c) 2015 The Chromium
> Authors.
> > All rights reserved.
> > On 2015/03/31 22:35:52, viettrungluu wrote:
> > > "
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
> > ipc/mojo/ipc_mojo_param_traits.cc:37: l->append("MojoMessageHelper()");
> > On 2015/03/31 22:35:52, viettrungluu wrote:
> > > Shouldn't you log something like mojo::MessagePipeHandle instead? (You
could
> > > even log its value....)
> > 
> > Oops...
> > 
> >
>
https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
> > File ipc/mojo/ipc_mojo_param_traits.h (right):
> > 
> >
>
https://codereview.chromium.org/1051443003/diff/20001/ipc/mojo/ipc_mojo_param...
> > ipc/mojo/ipc_mojo_param_traits.h:1: // Copyright (c) 2015 The Chromium
> Authors.
> > All rights reserved.
> > On 2015/03/31 22:35:52, viettrungluu wrote:
> > > nit: no "(c)" in the current copyright statement
> > 
> > Done.

[Hajime Morrita, 2015-04-01 17:03:01]

The CQ bit was checked by morrita@chromium.org

[commit-bot: I haz the power, 2015-04-01 17:03:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1051443003/40001

[commit-bot: I haz the power, 2015-04-01 17:07:42]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-04-01 17:08:42]

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

[mfomitchev1, 2015-04-01 20:36:05]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1054723002/ by mfomitchev@google.com.

The reason for reverting is: Broke XP Tests (1):

https://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%281%29/build...

https://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%281%29/build....

[Hajime Morrita, 2015-04-01 21:07:14]

The CQ bit was checked by morrita@chromium.org

[Hajime Morrita, 2015-04-01 21:07:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
viettrungluu@chromium.org
Link to the patchset: https://codereview.chromium.org/1051443003/#ps60001
(title: "Fixing errors on Windows")

[commit-bot: I haz the power, 2015-04-01 21:08:06]

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

[commit-bot: I haz the power, 2015-04-01 23:37:35]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-01 23:38:30]

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

[mfomitchev, 2015-04-02 14:49:42]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1054983003/ by mfomitchev@chromium.org.

The reason for reverting is: This broke the tests again, looks like
ParamTraitInvalidMesssagePipe was fixed, but ParamTraitValidMessagePipe wasn't.

https://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%281%29/build...

https://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%281%29/build....

[Hajime Morrita, 2015-04-02 18:38:14]

Made tests work, actually, on Windows.

[Hajime Morrita, 2015-04-02 18:39:50]

The CQ bit was checked by morrita@chromium.org

[Hajime Morrita, 2015-04-02 18:39:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
viettrungluu@chromium.org
Link to the patchset: https://codereview.chromium.org/1051443003/#ps80001
(title: "Made tests work, actually, on Windows.")

[commit-bot: I haz the power, 2015-04-02 18:41:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1051443003/80001

[commit-bot: I haz the power, 2015-04-03 05:28:31]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-04-03 20:32:30]

Patchset 5 (id:??) landed as
https://crrev.com/438a2eedbb124eb3fc41ea17a4701eb7d3869af3
Cr-Commit-Position: refs/heads/master@{#323629}