Mojo: (Theoretically) implement the read side of platform handle passing.

Mojo: (Theoretically) implement the read side of platform handle passing.

Next, I need to write tests.

R=yzshen@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269928

[yzshen1, 2014-05-11 07:16:59]

https://codereview.chromium.org/277083003/diff/1/mojo/embedder/platform_handl...
File mojo/embedder/platform_handle_vector.h (right):

https://codereview.chromium.org/277083003/diff/1/mojo/embedder/platform_handl...
mojo/embedder/platform_handle_vector.h:15: typedef std::vector<PlatformHandle>
PlatformHandleVector;
Does it make sense to merge this file into platform_handle.h?

https://codereview.chromium.org/277083003/diff/1/mojo/system/raw_channel.h
File mojo/system/raw_channel.h (right):

https://codereview.chromium.org/277083003/diff/1/mojo/system/raw_channel.h#ne...
mojo/system/raw_channel.h:103: virtual size_t GetSerializedPlatformHandleSize()
const = 0;
I have the feeling that this method belongs to the platform handle
serialization/deserialization code. But I have no idea how that code interacts
with raw channel. (Please also see my comment in transport_data.h)

https://codereview.chromium.org/277083003/diff/1/mojo/system/raw_channel_posi...
File mojo/system/raw_channel_posix.cc (right):

https://codereview.chromium.org/277083003/diff/1/mojo/system/raw_channel_posi...
mojo/system/raw_channel_posix.cc:141: read_platform_handles_.reset();
The handles need to be closed? (here and line 176)

https://codereview.chromium.org/277083003/diff/1/mojo/system/transport_data.h
File mojo/system/transport_data.h (right):

https://codereview.chromium.org/277083003/diff/1/mojo/system/transport_data.h...
mojo/system/transport_data.h:117: static const char* ValidateBuffer(size_t
serialized_platform_handle_size,
I don't think I have understood the whole design of handle passing (I only read
the comments above this class declaration).

I think the serialized platform handle size is raw-channel-specific: the same
for all messages passed over a raw channel, instead of platform-specific or
variable-length-for-each-message. Right? Otherwise, we would either use a const
platform-specific value, or encode the size information in the message.

Would you please tell me more about the design? Have we already got the platform
handle de/serialization code? If yes, please point me to it. Thanks!

[viettrungluu, 2014-05-12 17:33:28]

thanks, ptal

https://codereview.chromium.org/277083003/diff/1/mojo/embedder/platform_handl...
File mojo/embedder/platform_handle_vector.h (right):

https://codereview.chromium.org/277083003/diff/1/mojo/embedder/platform_handl...
mojo/embedder/platform_handle_vector.h:15: typedef std::vector<PlatformHandle>
PlatformHandleVector;
On 2014/05/11 07:16:59, yzshen1 wrote:
> Does it make sense to merge this file into platform_handle.h? 

Then platform_handle.h would inflict the inclusion of <vector> on everyone else.

Also, I added a function to it.

https://codereview.chromium.org/277083003/diff/1/mojo/system/raw_channel.h
File mojo/system/raw_channel.h (right):

https://codereview.chromium.org/277083003/diff/1/mojo/system/raw_channel.h#ne...
mojo/system/raw_channel.h:103: virtual size_t GetSerializedPlatformHandleSize()
const = 0;
On 2014/05/11 07:16:59, yzshen1 wrote:
> I have the feeling that this method belongs to the platform handle
> serialization/deserialization code. But I have no idea how that code interacts
> with raw channel. (Please also see my comment in transport_data.h)

It doesn't. Because there may be multiple ways of serializing a platform handle
(e.g., on Windows).

https://codereview.chromium.org/277083003/diff/1/mojo/system/raw_channel_posi...
File mojo/system/raw_channel_posix.cc (right):

https://codereview.chromium.org/277083003/diff/1/mojo/system/raw_channel_posi...
mojo/system/raw_channel_posix.cc:141: read_platform_handles_.reset();
On 2014/05/11 07:16:59, yzshen1 wrote:
> The handles need to be closed? (here and line 176)

Done.

https://codereview.chromium.org/277083003/diff/1/mojo/system/transport_data.h
File mojo/system/transport_data.h (right):

https://codereview.chromium.org/277083003/diff/1/mojo/system/transport_data.h...
mojo/system/transport_data.h:117: static const char* ValidateBuffer(size_t
serialized_platform_handle_size,
On 2014/05/11 07:16:59, yzshen1 wrote:
> I don't think I have understood the whole design of handle passing (I only
read
> the comments above this class declaration).
> 
> I think the serialized platform handle size is raw-channel-specific: the same
> for all messages passed over a raw channel, instead of platform-specific or
> variable-length-for-each-message. Right? Otherwise, we would either use a
const
> platform-specific value, or encode the size information in the message.

It is so RawChannel-specific, that it may vary between instances, even on the
same platform.

> 
> Would you please tell me more about the design? Have we already got the
platform
> handle de/serialization code? If yes, please point me to it. Thanks!

We have it on Linux, but not on Windows. On Windows, depending on the situation,
you may simply be able to DuplicateHandle() into the target process (in which
case the information is just a HANDLE[*]), or you may have to talk to a broker
(in which case the information is whatever information is needed retrieve the
handle from the broker).

[*] Possibly plus an enum value indicating what situation we're in.

[yzshen1, 2014-05-12 20:17:00]

lgtm

[viettrungluu, 2014-05-12 20:40:06]

The CQ bit was checked by viettrungluu@chromium.org

[commit-bot: I haz the power, 2014-05-12 20:41:52]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/viettrungluu@chromium.org/277083003/40001

[viettrungluu, 2014-05-13 00:03:54]

Committed patchset #3 manually as r269928 (presubmit successful).