Allow data pipe producer/consumer handles to be re-sent.

Allow data pipe producer/consumer handles to be re-sent.

I.e., implement the appropriate serialization methods in
Remote{Producer,Consumer}DataPipeImpl.

There's a bit of refactoring to share some code. There's also some
clean-up/minor bug-fixing.

TODO: We'll need to add more tests:
* Sending data pipe handles when the data pipe already has data.
* Sending a data pipe handle when someone's waiting on it (should result
  in "cancelled").
* Sending other types of handles when someone's waiting (ditto); I'm
  almost certain we have bugs here.

R=morrita@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/006e0c80dc0595175861d3f840fcafdca328a8e0

[Hajime Morrita, 2015-03-03 03:00:39]

lgtm.

https://codereview.chromium.org/973513004/diff/1/mojo/edk/system/data_pipe_im...
File mojo/edk/system/data_pipe_impl_unittest.cc (right):

https://codereview.chromium.org/973513004/diff/1/mojo/edk/system/data_pipe_im...
mojo/edk/system/data_pipe_impl_unittest.cc:386:
RemoteProducerDataPipeImplTestHelper::DoTransfer();
It might be clearer just to call SendDispatcher() twice, instead of 
calling super DoTransfer() of the super class.

https://codereview.chromium.org/973513004/diff/1/mojo/edk/system/data_pipe_im...
mojo/edk/system/data_pipe_impl_unittest.cc:423:
RemoteConsumerDataPipeImplTestHelper::DoTransfer();
Ditto.

https://codereview.chromium.org/973513004/diff/1/mojo/edk/system/remote_consu...
File mojo/edk/system/remote_consumer_data_pipe_impl.cc (right):

https://codereview.chromium.org/973513004/diff/1/mojo/edk/system/remote_consu...
mojo/edk/system/remote_consumer_data_pipe_impl.cc:291:
owner()->SetConsumerClosedNoLock();
This isn't directly related to this CL, but it'd be nice if the naming indicates
that consumer_open() and SetConsumeClosedNoLock() refers the same flag, or even
there is no flag and it just sees channel_endpoint_ to decide open-ness. The
flag seems a bit too delicate.

[viettrungluu, 2015-03-03 05:17:45]

Thanks.

https://codereview.chromium.org/973513004/diff/1/mojo/edk/system/data_pipe_im...
File mojo/edk/system/data_pipe_impl_unittest.cc (right):

https://codereview.chromium.org/973513004/diff/1/mojo/edk/system/data_pipe_im...
mojo/edk/system/data_pipe_impl_unittest.cc:386:
RemoteProducerDataPipeImplTestHelper::DoTransfer();
On 2015/03/03 03:00:38, morrita wrote:
> It might be clearer just to call SendDispatcher() twice, instead of 
> calling super DoTransfer() of the super class.

Yeah, I went back and forth on that (since the super's DoTransfer() is slightly
non-trivial).

Done.

https://codereview.chromium.org/973513004/diff/1/mojo/edk/system/data_pipe_im...
mojo/edk/system/data_pipe_impl_unittest.cc:423:
RemoteConsumerDataPipeImplTestHelper::DoTransfer();
On 2015/03/03 03:00:38, morrita wrote:
> Ditto.

Done.

https://codereview.chromium.org/973513004/diff/1/mojo/edk/system/remote_consu...
File mojo/edk/system/remote_consumer_data_pipe_impl.cc (right):

https://codereview.chromium.org/973513004/diff/1/mojo/edk/system/remote_consu...
mojo/edk/system/remote_consumer_data_pipe_impl.cc:291:
owner()->SetConsumerClosedNoLock();
On 2015/03/03 03:00:38, morrita wrote:
> This isn't directly related to this CL, but it'd be nice if the naming
indicates
> that consumer_open() and SetConsumeClosedNoLock() refers the same flag, or
even
> there is no flag and it just sees channel_endpoint_ to decide open-ness. The
> flag seems a bit too delicate.

Yeah, things aren't factored quite right (partly a consequence of my moving the
implementation of DataPipe into the ...DataPipeImpl subclasses). :-/

I kind of want to just make the the impls are in charge of this information
(rather than having flags in DataPipe) ... but I'd have to do this carefully
since impls can be swapped (and sometimes there may not be an impl, even if
momentarily, which would complicate various DCHECKs and such).

[viettrungluu, 2015-03-03 05:18:04]

Committed patchset #2 (id:20001) manually as
006e0c80dc0595175861d3f840fcafdca328a8e0 (presubmit successful).