Add multiplexing of message pipes in the new EDK.

Add multiplexing of message pipes in the new EDK.

This dramatically cuts down on the number of OS pipes used. The key is that the vast majority (all for now) of message pipes are only read or written in one process. If they're passed to another process, that is before they're interacted with. So by default, create message pipes such that they're not transferable after they're read or written. A non-transferable pipe is then just a unique ID. When two process "bind" their end by reading/writing to that ID, the parent process ensures that the two processes have a channel between them and tells them to connect to each other using it.

"Transferable" message pipes can still be created and these can be sent after they're read or written to, by specifying MOJO_CREATE_MESSAGE_PIPE_OPTIONS_FLAG_TRANSFERABLE.

BUG=556259
R=tsepez@chromium.org, yzshen@chromium.org

Committed: https://chromium.googlesource.com/chromium/src/+/44929cdb67189f2715c90b1a17bc29ccc2276991

[jam, 2015-12-02 05:51:15]

Patchset #3 (id:40001) has been deleted

[jam, 2015-12-02 21:47:22]

Description was changed from

==========
multiplexing

BUG=556259
==========

to

==========
Add multiplexing of message pipes in the new EDK.

This dramatically cuts down on the number of OS pipes used. The key is that the
vast majority (all for now) of message pipes are only read or written in one
process. If they're passed to another process, that is before they're interacted
with. So by default, create message pipes such that they're not transferable
after they're read or written to. A non-transferable pipe is then just a unique
ID. When two process then "bind" their end by reading/writing to that ID, the
parent process ensures that the two processes have a channel between them and
tells them to connect to each other using it.

"Transferable" message pipes can still be created and these can be sent after
they're read or written to, by specifying
MOJO_CREATE_MESSAGE_PIPE_OPTIONS_FLAG_TRANSFERABLE.

BUG=556259
==========

[jam, 2015-12-02 21:47:23]

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

[jam, 2015-12-03 00:44:44]

Patchset #6 (id:120001) has been deleted

[jam, 2015-12-03 00:48:36]

Description was changed from

==========
Add multiplexing of message pipes in the new EDK.

This dramatically cuts down on the number of OS pipes used. The key is that the
vast majority (all for now) of message pipes are only read or written in one
process. If they're passed to another process, that is before they're interacted
with. So by default, create message pipes such that they're not transferable
after they're read or written to. A non-transferable pipe is then just a unique
ID. When two process then "bind" their end by reading/writing to that ID, the
parent process ensures that the two processes have a channel between them and
tells them to connect to each other using it.

"Transferable" message pipes can still be created and these can be sent after
they're read or written to, by specifying
MOJO_CREATE_MESSAGE_PIPE_OPTIONS_FLAG_TRANSFERABLE.

BUG=556259
==========

to

==========
Add multiplexing of message pipes in the new EDK.

This dramatically cuts down on the number of OS pipes used. The key is that the
vast majority (all for now) of message pipes are only read or written in one
process. If they're passed to another process, that is before they're interacted
with. So by default, create message pipes such that they're not transferable
after they're read or written. A non-transferable pipe is then just a unique ID.
When two process "bind" their end by reading/writing to that ID, the parent
process ensures that the two processes have a channel between them and tells
them to connect to each other using it.

"Transferable" message pipes can still be created and these can be sent after
they're read or written to, by specifying
MOJO_CREATE_MESSAGE_PIPE_OPTIONS_FLAG_TRANSFERABLE.

BUG=556259
==========

[jam, 2015-12-03 23:01:53]

Patchset #7 (id:150001) has been deleted

[jam, 2015-12-03 23:04:04]

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

[jam, 2015-12-03 23:04:05]

+tsepez as security reviewer since you looked at the last
mojo/edk/system/broker_messages.h changes.

This should be the last set of changes here. I don't expect other changes needed
to the messages file. Should I add security folks as owners?

[jam, 2015-12-03 23:22:08]

Regarding the size of the pipe_id key: I pondered whether 64 bit is enough or I
should go higher. There are two scenarios where we might get a collision:
-2 pairs of message pipes sharing a single RawChannel having the same routing id
-2 pairs of message pipes, in between the time they're created and they're
received and "bound" to a RawChannel, have the same routing id

For the former case, at the very most I can't see more than a few thousand
message pipes between two processes. The math on that works out to be 1 in 10
trillion (since the cryptographically secure IDs). For the latter case, for the
use case of how message pipes are used (through bindings), I don't see more than
a few hundred in the system at once that haven't connected yet.

[Tom Sepez, 2015-12-03 23:35:28]

Ok on the rand IDs.  I'm still plowing through the meat of this, its going to
take a while.

https://codereview.chromium.org/1488853002/diff/190001/mojo/edk/system/broker...
File mojo/edk/system/broker_messages.h (right):

https://codereview.chromium.org/1488853002/diff/190001/mojo/edk/system/broker...
mojo/edk/system/broker_messages.h:44: const int kBrokerMessageHeaderSize =
Actually, this hasn't been 100% correct since it doesn't account for possible
padding between size and id or between id and the union.  I think you actually
want offsetof(BrokerMessage, handles) here.

https://codereview.chromium.org/1488853002/diff/190001/mojo/edk/system/broker...
mojo/edk/system/broker_messages.h:52: // Messages from child to parent.
nit: indent.

[yzshen1, 2015-12-03 23:37:51]

Just some nits. Thanks!

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/broker.h
File mojo/edk/system/broker.h (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/broker...
mojo/edk/system/broker.h:52: // that it can us for sending messages.
nit: us -> use

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/broker...
File mojo/edk/system/broker_state.h (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/broker...
mojo/edk/system/broker_state.h:78: base::Lock lock_;  // Guards access to below.
If ConnectMessagePipe() and CloseMessagePipe() are only called from the IO
thread, it is possible to access (some of) the following members without
locking?

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/child_...
File mojo/edk/system/child_broker.h (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/child_...
mojo/edk/system/child_broker.h:26: // ChildBrokerHost) to convert handles to
tokens and vice versa.
Please consider updating the comment since it doesn't just do sync IPCs anymore.

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/child_...
mojo/edk/system/child_broker.h:91: // Guards access to below.
I think |lock_| guards |parent_[a]sync_channel_| access too?

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/child_...
File mojo/edk/system/child_broker_host.cc (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/child_...
mojo/edk/system/child_broker_host.cc:64: FROM_HERE,
nit: incorrect indent.

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/child_...
File mojo/edk/system/child_broker_host.h (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/child_...
mojo/edk/system/child_broker_host.h:46: // |process_id|.
nit: please update the comment to |peer_id|.

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/messag...
File mojo/edk/system/message_pipe_dispatcher.cc (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/messag...
mojo/edk/system/message_pipe_dispatcher.cc:529: // TODO(vtl): Currently, there
are no options, so we just use
I think this comment is not useful anymore.

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/messag...
File mojo/edk/system/message_pipe_dispatcher.h (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/messag...
mojo/edk/system/message_pipe_dispatcher.h:61: // |RawChannel::Delegate methods:
nit: unmatched "|" and incorrect indent at line 63.

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/messag...
mojo/edk/system/message_pipe_dispatcher.h:71: void
GotNonTransferableChannel(RawChannel* channel);
Does it make sense to take RoutedRawChannel* as input instead?
IIUC, we should never use a "standalone" RawChannel for this call. And all
callsites of this method currently use
  GotNonTransferableChannel(some_routed_raw_channel->channel());

Maybe using RoutedRawChannel* here will make it more clear and also a little
easier for the callsites?

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/messag...
mojo/edk/system/message_pipe_dispatcher.h:162: // get a RawChannel. So this will
be null after requesting a channel.
I think pipe_id_ stays valid, because it is used as the route ID for all
outgoing messages.

https://codereview.chromium.org/1488853002/diff/120002/mojo/public/c/system/m...
File mojo/public/c/system/message_pipe.h (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/public/c/system/m...
mojo/public/c/system/message_pipe.h:25: //           can be sent over another
pipe after it's been read or written. This
Is it "read, written or *waited*"?

[jam, 2015-12-04 00:43:47]

https://codereview.chromium.org/1488853002/diff/190001/mojo/edk/system/broker...
File mojo/edk/system/broker_messages.h (right):

https://codereview.chromium.org/1488853002/diff/190001/mojo/edk/system/broker...
mojo/edk/system/broker_messages.h:44: const int kBrokerMessageHeaderSize =
On 2015/12/03 23:35:28, Tom Sepez wrote:
> Actually, this hasn't been 100% correct since it doesn't account for possible
> padding between size and id or between id and the union.  I think you actually
> want offsetof(BrokerMessage, handles) here.

This is only used on Windows and there's no padding between the first two
members with VS. If this did change in future compilers, it will be very obvious
(i.e. ipc fails). (btw note this isn't changed code, I just renamed the enum).

https://codereview.chromium.org/1488853002/diff/190001/mojo/edk/system/broker...
mojo/edk/system/broker_messages.h:52: // Messages from child to parent.
On 2015/12/03 23:35:28, Tom Sepez wrote:
> nit: indent.

i had meant to put it like this to separate out both types of messages. I'll
switch it to blank lines instead.

[jam, 2015-12-04 05:06:47]

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/broker.h
File mojo/edk/system/broker.h (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/broker...
mojo/edk/system/broker.h:52: // that it can us for sending messages.
On 2015/12/03 23:37:50, yzshen1 wrote:
> nit: us -> use

Done.

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/broker...
File mojo/edk/system/broker_state.h (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/broker...
mojo/edk/system/broker_state.h:78: base::Lock lock_;  // Guards access to below.
On 2015/12/03 23:37:50, yzshen1 wrote:
> If ConnectMessagePipe() and CloseMessagePipe() are only called from the IO
> thread, it is possible to access (some of) the following members without
> locking?

The issue is that child_processes_ is accessed in ChildBrokerHostCreated outside
the IO thread. Since that is accessed in other functions, it would be
inconvenient to lock this member but not others (say specifically
child_channels_ in EnsureProcessesConnected).

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/child_...
File mojo/edk/system/child_broker.h (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/child_...
mojo/edk/system/child_broker.h:26: // ChildBrokerHost) to convert handles to
tokens and vice versa.
On 2015/12/03 23:37:50, yzshen1 wrote:
> Please consider updating the comment since it doesn't just do sync IPCs
anymore.

Done.

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/child_...
mojo/edk/system/child_broker.h:91: // Guards access to below.
On 2015/12/03 23:37:50, yzshen1 wrote:
> I think |lock_| guards |parent_[a]sync_channel_| access too?

Done.

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/child_...
File mojo/edk/system/child_broker_host.cc (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/child_...
mojo/edk/system/child_broker_host.cc:64: FROM_HERE,
On 2015/12/03 23:37:50, yzshen1 wrote:
> nit: incorrect indent.

Done.

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/child_...
File mojo/edk/system/child_broker_host.h (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/child_...
mojo/edk/system/child_broker_host.h:46: // |process_id|.
On 2015/12/03 23:37:50, yzshen1 wrote:
> nit: please update the comment to |peer_id|.

Done.

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/messag...
File mojo/edk/system/message_pipe_dispatcher.cc (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/messag...
mojo/edk/system/message_pipe_dispatcher.cc:529: // TODO(vtl): Currently, there
are no options, so we just use
On 2015/12/03 23:37:50, yzshen1 wrote:
> I think this comment is not useful anymore.

Done.

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/messag...
File mojo/edk/system/message_pipe_dispatcher.h (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/messag...
mojo/edk/system/message_pipe_dispatcher.h:61: // |RawChannel::Delegate methods:
On 2015/12/03 23:37:51, yzshen1 wrote:
> nit: unmatched "|" and incorrect indent at line 63.

Done.

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/messag...
mojo/edk/system/message_pipe_dispatcher.h:71: void
GotNonTransferableChannel(RawChannel* channel);
On 2015/12/03 23:37:51, yzshen1 wrote:
> Does it make sense to take RoutedRawChannel* as input instead?
> IIUC, we should never use a "standalone" RawChannel for this call. And all
> callsites of this method currently use
>   GotNonTransferableChannel(some_routed_raw_channel->channel());
> 
> Maybe using RoutedRawChannel* here will make it more clear and also a little
> easier for the callsites?

I was hoping to make RoutedRawChannel an implementation detail that
MessagePipeDispatcher doesn't know about.. i.e. it always uses channel_ to send.

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/messag...
mojo/edk/system/message_pipe_dispatcher.h:162: // get a RawChannel. So this will
be null after requesting a channel.
On 2015/12/03 23:37:51, yzshen1 wrote:
> I think pipe_id_ stays valid, because it is used as the route ID for all
> outgoing messages.

thanks, that was out of date

https://codereview.chromium.org/1488853002/diff/120002/mojo/public/c/system/m...
File mojo/public/c/system/message_pipe.h (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/public/c/system/m...
mojo/public/c/system/message_pipe.h:25: //           can be sent over another
pipe after it's been read or written. This
On 2015/12/03 23:37:51, yzshen1 wrote:
> Is it "read, written or *waited*"?

Done.

[yzshen1, 2015-12-04 05:57:05]

LGTM Thanks!

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/broker...
File mojo/edk/system/broker_state.h (right):

https://codereview.chromium.org/1488853002/diff/120002/mojo/edk/system/broker...
mojo/edk/system/broker_state.h:78: base::Lock lock_;  // Guards access to below.
On 2015/12/04 05:06:47, jam wrote:
> On 2015/12/03 23:37:50, yzshen1 wrote:
> > If ConnectMessagePipe() and CloseMessagePipe() are only called from the IO
> > thread, it is possible to access (some of) the following members without
> > locking?
> 
> The issue is that child_processes_ is accessed in ChildBrokerHostCreated
outside
> the IO thread. Since that is accessed in other functions, it would be
> inconvenient to lock this member but not others (say specifically
> child_channels_ in EnsureProcessesConnected).

I see. Thanks for explanation!

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/test/run_all_...
File mojo/edk/test/run_all_perftests.cc (right):

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/test/run_all_...
mojo/edk/test/run_all_perftests.cc:30: // loop, which is destructed inside
base::LaunchUnitTests.
where is base::LaunchUnitTests called?

[Tom Sepez, 2015-12-04 17:59:43]

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/broker...
File mojo/edk/system/broker_state.cc (right):

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/broker...
mojo/edk/system/broker_state.cc:119: CHECK(connected_pipes_.find(message_pipe)
!= connected_pipes_.end());
These checks are essential for safety; I'd hate for someone to change them to
dchecks down the road.  Can these occur in a priv process? If so, we might want
to handle this without aborting.

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/child_...
File mojo/edk/system/child_broker.cc (right):

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/child_...
mojo/edk/system/child_broker.cc:39: DCHECK(rv);
Do we want something stronger than a DCHECK here?

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/child_...
File mojo/edk/system/child_broker_host.cc (right):

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/child_...
mojo/edk/system/child_broker_host.cc:203: CHECK(write_data_.empty());
Can a rouge child force us to abort here?

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/messag...
File mojo/edk/system/message_pipe_dispatcher.h (right):

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/messag...
mojo/edk/system/message_pipe_dispatcher.h:33: validated_options.flags &
nit: maybe !!(validated_options.flags &
MOJO_CREATE_MESSAGE_PIPE_OPTIONS_FLAG_TRANSFERABLE)

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/routed...
File mojo/edk/system/routed_raw_channel.cc (right):

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/routed...
mojo/edk/system/routed_raw_channel.cc:113: CHECK_EQ(message_view.num_bytes(),
sizeof(char) + sizeof(uint64_t));
Again, can a rouge child cause us to abort there?

[jam, 2015-12-05 00:09:34]

Thanks Yuzhu and Tom for the thorough review. Tom: your comments are addressed
now. ptal.

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/broker...
File mojo/edk/system/broker_state.cc (right):

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/broker...
mojo/edk/system/broker_state.cc:119: CHECK(connected_pipes_.find(message_pipe)
!= connected_pipes_.end());
On 2015/12/04 17:59:43, Tom Sepez wrote:
> These checks are essential for safety; I'd hate for someone to change them to
> dchecks down the road.  Can these occur in a priv process? If so, we might
want
> to handle this without aborting.

BrokerState is global state that is in the browser process.

This specific method is only called from other code in the browser process, so
it's not controlled by unprivileged processes. The two Handle* methods below are
methods called by ChildBrokerHost in response to IPCs from the child. There was
one CHECK in HandleCancelConnectMessagePipe, I've changed it to a DCHECK and
added an if statement based on your suggestion.

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/child_...
File mojo/edk/system/child_broker.cc (right):

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/child_...
mojo/edk/system/child_broker.cc:39: DCHECK(rv);
On 2015/12/04 17:59:43, Tom Sepez wrote:
> Do we want something stronger than a DCHECK here?

sure, changed it to CHECK so that we know if this fails (which would be
catastrophic)

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/child_...
File mojo/edk/system/child_broker_host.cc (right):

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/child_...
mojo/edk/system/child_broker_host.cc:203: CHECK(write_data_.empty());
On 2015/12/04 17:59:43, Tom Sepez wrote:
> Can a rouge child force us to abort here?

yeah, added an if statement

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/messag...
File mojo/edk/system/message_pipe_dispatcher.h (right):

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/messag...
mojo/edk/system/message_pipe_dispatcher.h:33: validated_options.flags &
On 2015/12/04 17:59:43, Tom Sepez wrote:
> nit: maybe !!(validated_options.flags &
> MOJO_CREATE_MESSAGE_PIPE_OPTIONS_FLAG_TRANSFERABLE)

Done.

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/routed...
File mojo/edk/system/routed_raw_channel.cc (right):

https://codereview.chromium.org/1488853002/diff/210001/mojo/edk/system/routed...
mojo/edk/system/routed_raw_channel.cc:113: CHECK_EQ(message_view.num_bytes(),
sizeof(char) + sizeof(uint64_t));
On 2015/12/04 17:59:43, Tom Sepez wrote:
> Again, can a rouge child cause us to abort there?

yep, added if statement

[Tom Sepez, 2015-12-05 00:14:36]

LGTM

[jam, 2015-12-06 22:23:52]

The CQ bit was checked by jam@chromium.org

[jam, 2015-12-06 22:23:53]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org
Link to the patchset: https://codereview.chromium.org/1488853002/#ps250001
(title: "tsepez review comments")

[commit-bot: I haz the power, 2015-12-06 22:24:11]

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

[commit-bot: I haz the power, 2015-12-06 23:36:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-06 23:36:03]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jam, 2015-12-06 23:40:25]

The CQ bit was checked by jam@chromium.org

[commit-bot: I haz the power, 2015-12-06 23:40:48]

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

[commit-bot: I haz the power, 2015-12-06 23:51:03]

Description was changed from

==========
Add multiplexing of message pipes in the new EDK.

This dramatically cuts down on the number of OS pipes used. The key is that the
vast majority (all for now) of message pipes are only read or written in one
process. If they're passed to another process, that is before they're interacted
with. So by default, create message pipes such that they're not transferable
after they're read or written. A non-transferable pipe is then just a unique ID.
When two process "bind" their end by reading/writing to that ID, the parent
process ensures that the two processes have a channel between them and tells
them to connect to each other using it.

"Transferable" message pipes can still be created and these can be sent after
they're read or written to, by specifying
MOJO_CREATE_MESSAGE_PIPE_OPTIONS_FLAG_TRANSFERABLE.

BUG=556259
==========

to

==========
Add multiplexing of message pipes in the new EDK.

This dramatically cuts down on the number of OS pipes used. The key is that the
vast majority (all for now) of message pipes are only read or written in one
process. If they're passed to another process, that is before they're interacted
with. So by default, create message pipes such that they're not transferable
after they're read or written. A non-transferable pipe is then just a unique ID.
When two process "bind" their end by reading/writing to that ID, the parent
process ensures that the two processes have a channel between them and tells
them to connect to each other using it.

"Transferable" message pipes can still be created and these can be sent after
they're read or written to, by specifying
MOJO_CREATE_MESSAGE_PIPE_OPTIONS_FLAG_TRANSFERABLE.

BUG=556259
R=tsepez@chromium.org, yzshen@chromium.org

Committed: https://crrev.com/44929cdb67189f2715c90b1a17bc29ccc2276991
Cr-Commit-Position: refs/heads/master@{#363386}
==========

[commit-bot: I haz the power, 2015-12-06 23:51:04]

Patchset 11 (id:??) landed as
https://crrev.com/44929cdb67189f2715c90b1a17bc29ccc2276991
Cr-Commit-Position: refs/heads/master@{#363386}

[jam, 2015-12-06 23:51:40]

Description was changed from

==========
Add multiplexing of message pipes in the new EDK.

This dramatically cuts down on the number of OS pipes used. The key is that the
vast majority (all for now) of message pipes are only read or written in one
process. If they're passed to another process, that is before they're interacted
with. So by default, create message pipes such that they're not transferable
after they're read or written. A non-transferable pipe is then just a unique ID.
When two process "bind" their end by reading/writing to that ID, the parent
process ensures that the two processes have a channel between them and tells
them to connect to each other using it.

"Transferable" message pipes can still be created and these can be sent after
they're read or written to, by specifying
MOJO_CREATE_MESSAGE_PIPE_OPTIONS_FLAG_TRANSFERABLE.

BUG=556259
R=tsepez@chromium.org, yzshen@chromium.org

Committed: https://crrev.com/44929cdb67189f2715c90b1a17bc29ccc2276991
Cr-Commit-Position: refs/heads/master@{#363386}
==========

to

==========
Add multiplexing of message pipes in the new EDK.

This dramatically cuts down on the number of OS pipes used. The key is that the
vast majority (all for now) of message pipes are only read or written in one
process. If they're passed to another process, that is before they're interacted
with. So by default, create message pipes such that they're not transferable
after they're read or written. A non-transferable pipe is then just a unique ID.
When two process "bind" their end by reading/writing to that ID, the parent
process ensures that the two processes have a channel between them and tells
them to connect to each other using it.

"Transferable" message pipes can still be created and these can be sent after
they're read or written to, by specifying
MOJO_CREATE_MESSAGE_PIPE_OPTIONS_FLAG_TRANSFERABLE.

BUG=556259
R=tsepez@chromium.org, yzshen@chromium.org

Committed:
https://chromium.googlesource.com/chromium/src/+/44929cdb67189f2715c90b1a17bc...
==========

[jam, 2015-12-06 23:51:41]

Committed patchset #11 (id:250001) manually as
44929cdb67189f2715c90b1a17bc29ccc2276991 (presubmit successful).

[loyso (OOO), 2015-12-07 06:57:29]

On 2015/12/06 23:51:41, jam wrote:
> Committed patchset #11 (id:250001) manually as
> 44929cdb67189f2715c90b1a17bc29ccc2276991 (presubmit successful).

Looks like this CL breaks win build for battor_agent.
http://build.chromium.org/p/chromium/buildstatus?builder=Win&number=38134

FAILED: ninja -t msvc -e environment.x86 -- C:\b\build\goma/gomacc
"C:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64_x86\cl.exe" /nologo
/showIncludes /FC @obj\tools\battor_agent\battor_agent_lib.battor_agent.obj.rsp
/c ..\..\tools\battor_agent\battor_agent.cc
/Foobj\tools\battor_agent\battor_agent_lib.battor_agent.obj
/Fdobj\tools\battor_agent\battor_agent_lib.cc.pdb 
c:\b\build\slave\win\build\src\device\serial\buffer.h(10) : fatal error C1083:
Cannot open include file: 'device/serial/serial.mojom.h': No such file or
directory
FAILED: ninja -t msvc -e environment.x86 -- C:\b\build\goma/gomacc
"C:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64_x86\cl.exe" /nologo
/showIncludes /FC
@obj\tools\battor_agent\battor_agent_lib.battor_connection.obj.rsp /c
..\..\tools\battor_agent\battor_connection.cc
/Foobj\tools\battor_agent\battor_agent_lib.battor_connection.obj
/Fdobj\tools\battor_agent\battor_agent_lib.cc.pdb 
c:\b\build\slave\win\build\src\device\serial\buffer.h(10) : fatal error C1083:
Cannot open include file: 'device/serial/serial.mojom.h': No such file or
directory

[jam, 2015-12-07 15:38:04]

On 2015/12/07 06:57:29, loyso wrote:
> On 2015/12/06 23:51:41, jam wrote:
> > Committed patchset #11 (id:250001) manually as
> > 44929cdb67189f2715c90b1a17bc29ccc2276991 (presubmit successful).
> 
> Looks like this CL breaks win build for battor_agent.
> http://build.chromium.org/p/chromium/buildstatus?builder=Win&number=38134
> 
> FAILED: ninja -t msvc -e environment.x86 -- C:\b\build\goma/gomacc
> "C:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64_x86\cl.exe" /nologo
> /showIncludes /FC
@obj\tools\battor_agent\battor_agent_lib.battor_agent.obj.rsp
> /c ..\..\tools\battor_agent\battor_agent.cc
> /Foobj\tools\battor_agent\battor_agent_lib.battor_agent.obj
> /Fdobj\tools\battor_agent\battor_agent_lib.cc.pdb 
> c:\b\build\slave\win\build\src\device\serial\buffer.h(10) : fatal error C1083:
> Cannot open include file: 'device/serial/serial.mojom.h': No such file or
> directory
> FAILED: ninja -t msvc -e environment.x86 -- C:\b\build\goma/gomacc
> "C:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64_x86\cl.exe" /nologo
> /showIncludes /FC
> @obj\tools\battor_agent\battor_agent_lib.battor_connection.obj.rsp /c
> ..\..\tools\battor_agent\battor_connection.cc
> /Foobj\tools\battor_agent\battor_agent_lib.battor_connection.obj
> /Fdobj\tools\battor_agent\battor_agent_lib.cc.pdb 
> c:\b\build\slave\win\build\src\device\serial\buffer.h(10) : fatal error C1083:
> Cannot open include file: 'device/serial/serial.mojom.h': No such file or
> directory

This is unrelated; it looks like there's a missing dependency for that and
that's why the build intermittently fails (even before this cl). I'll file a
bug.