Use Mojo pipes to signal sync IPC events

Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
support correct FIFO between ChannelProxy and Mojo Channel
associated interfaces.

Specifically:

 - Introduces a new IPC::MojoEvent type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with MojoEvent for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

BUG=612500
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/97912711c72d91e50bf6999f6842f9de1c05fab1
Committed: https://crrev.com/6d7be626fdfc8e1e85bc2faa789a12a3ae3960a1
Cr-Original-Commit-Position: refs/heads/master@{#399848}
Cr-Commit-Position: refs/heads/master@{#399963}

[Ken Rockot(use gerrit already), 2016-06-04 06:26:01]

Patchset #4 (id:60001) has been deleted

[Ken Rockot(use gerrit already), 2016-06-05 22:54:22]

Patchset #1 (id:1) has been deleted

[Ken Rockot(use gerrit already), 2016-06-05 22:54:26]

Patchset #1 (id:20001) has been deleted

[Ken Rockot(use gerrit already), 2016-06-05 22:54:30]

Patchset #1 (id:40001) has been deleted

[Ken Rockot(use gerrit already), 2016-06-05 22:56:24]

Description was changed from

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
supporting correct FIFO between ChannelProxy and Mojo Channel
associated interfaces.

BUG=612500
==========

to

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
supporting correct FIFO between ChannelProxy and Mojo Channel
associated interfaces.

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces all use of WaitableEvent::WaitMany in legacy sync IPC
   code with use of mojo::SyncHandleRegistry.

BUG=612500
==========

[Ken Rockot(use gerrit already), 2016-06-05 22:57:08]

Description was changed from

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
supporting correct FIFO between ChannelProxy and Mojo Channel
associated interfaces.

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces all use of WaitableEvent::WaitMany in legacy sync IPC
   code with use of mojo::SyncHandleRegistry.

BUG=612500
==========

to

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
supporting correct FIFO between ChannelProxy and Mojo Channel
associated interfaces.

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces all use of WaitableEvent::WaitMany in legacy sync IPC
   code with use of mojo::SyncHandleRegistry::WatchAllHandles.

BUG=612500
==========

[Ken Rockot(use gerrit already), 2016-06-05 23:00:49]

Description was changed from

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
supporting correct FIFO between ChannelProxy and Mojo Channel
associated interfaces.

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces all use of WaitableEvent::WaitMany in legacy sync IPC
   code with use of mojo::SyncHandleRegistry::WatchAllHandles.

BUG=612500
==========

to

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
supporting correct FIFO between ChannelProxy and Mojo Channel
associated interfaces.

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces all use of WaitableEvent::WaitMany in legacy sync IPC
   code with use of mojo::SyncHandleRegistry::WatchAllHandles.

The system's behavior should be effectively unchanged by this CL.

BUG=612500
==========

[Ken Rockot(use gerrit already), 2016-06-06 07:39:59]

Description was changed from

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
supporting correct FIFO between ChannelProxy and Mojo Channel
associated interfaces.

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces all use of WaitableEvent::WaitMany in legacy sync IPC
   code with use of mojo::SyncHandleRegistry::WatchAllHandles.

The system's behavior should be effectively unchanged by this CL.

BUG=612500
==========

to

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
supporting correct FIFO between ChannelProxy and Mojo Channel
associated interfaces. 

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with mojo::Event for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

BUG=612500
==========

[Ken Rockot(use gerrit already), 2016-06-06 08:52:42]

Patchset #2 (id:100001) has been deleted

[Ken Rockot(use gerrit already), 2016-06-06 08:52:49]

Patchset #1 (id:80001) has been deleted

[Ken Rockot(use gerrit already), 2016-06-06 19:27:05]

Description was changed from

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
supporting correct FIFO between ChannelProxy and Mojo Channel
associated interfaces. 

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with mojo::Event for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

BUG=612500
==========

to

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
supporting correct FIFO between ChannelProxy and Mojo Channel
associated interfaces. 

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with mojo::Event for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

BUG=612500
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Ken Rockot(use gerrit already), 2016-06-06 19:32:43]

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

[Ken Rockot(use gerrit already), 2016-06-06 19:32:44]

OK - I think this is ready. Pardon the dust, introducing a Mojo dependency in
//ipc means several //ipc dependents now need to express their own dependency on
//mojo/edk/system. We can't use public_deps here because Mus and other Mojo apps
depend on //ipc from typemaps, and Mojo apps must not depend on
//mojo/edk/system.

John could you please look at //ipc changes?

Yuzhu could you please look at //mojo/public changes? In particular I'm curious
how you feel about making mojo::Event part of the public C++ API. Alternatively
we could make it something isolated to //ipc code, but I opted against that
since there's nothing ipc-specific about it.

[Ken Rockot(use gerrit already), 2016-06-06 19:33:46]

Description was changed from

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
supporting correct FIFO between ChannelProxy and Mojo Channel
associated interfaces. 

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with mojo::Event for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

BUG=612500
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
support correct FIFO between ChannelProxy and Mojo Channel
associated interfaces. 

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with mojo::Event for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

BUG=612500
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Ken Rockot(use gerrit already), 2016-06-06 19:37:31]

Description was changed from

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
support correct FIFO between ChannelProxy and Mojo Channel
associated interfaces. 

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with mojo::Event for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

BUG=612500
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
support correct FIFO between ChannelProxy and Mojo Channel
associated interfaces. 

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with mojo::Event for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

All TBRs are for addition of required //mojo/edk/system deps
on leaf targets which have a dependency (direct or otherwise)
on //ipc.

BUG=612500
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
TBR=bbudge@chromium.org for ppapi
TBR=bradnelson@chromium.org for components/nacl/loader
TBR=gab@chromium.org for chrome/installer
TBR=sievers@chromium.org for gpu/ipc/service
==========

[yzshen1, 2016-06-06 21:23:38]

On Mon, Jun 6, 2016 at 12:32 PM, <rockot@chromium.org> wrote:

> Reviewers: jam, yzshen1
> CL: https://codereview.chromium.org/2033243003/
>
> Message:
> OK - I think this is ready. Pardon the dust, introducing a Mojo dependency
> in
> //ipc means several //ipc dependents now need to express their own
> dependency on
> //mojo/edk/system. We can't use public_deps here because Mus and other
> Mojo apps
> depend on //ipc from typemaps, and Mojo apps must not depend on
> //mojo/edk/system.
>

Is it possible to split the //ipc target? The typemaps of Mus and other
Mojo apps don't need channel implementations. Maybe it is possible to have
them depend on something like //ipc:message, and let //ipc depends on both
//ipc:message and //mojo/edk/system?

WDYT?


>
> John could you please look at //ipc changes?
>
> Yuzhu could you please look at //mojo/public changes? In particular I'm
> curious
> how you feel about making mojo::Event part of the public C++ API.
> Alternatively
> we could make it something isolated to //ipc code, but I opted against that
> since there's nothing ipc-specific about it.
>
>
> Description:
> Use Mojo pipes to signal sync IPC events
>
> This transitions legacy sync IPC to use Mojo message pipe handles
> for all of its sync event waiting. This is a necessary precursor to
> mixing sync legacy IPC with sync Mojo IPC, and is also required to
> supporting correct FIFO between ChannelProxy and Mojo Channel
> associated interfaces.
>
> Specifically:
>
> - Introduces a new mojo::Event type which is a WaitableEvent-like
> interface around a local message pipe.
> - Moves mojo::SyncHandleRegistry out of internal bindings API and
> exposes it publicly.
> - Replaces most uses of WaitableEvent with mojo::Event for sync IPC
> - Replaces all use of WaitableEvent::WaitMany for sync IPC
> with mojo::SyncHandleRegistry::WatchAllHandles.
> - Cleans up some unnecessary complexity in SyncMessage since
> pump_messages_event() was only being used with a single
> global event that's always signaled.
>
> The system's behavior should be effectively unchanged by this CL,
> but legacy sync IPC and mojo sync IPC can now be mixed freely.
>
> BUG=612500
>
>
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
>
> Base URL: https://chromium.googlesource.com/chromium/src.git@master
>
> Affected files (+337, -233 lines):
> M chrome/installer/setup/BUILD.gn
> M components/nacl/loader/BUILD.gn
> M gpu/ipc/service/BUILD.gn
> M ipc/BUILD.gn
> M ipc/DEPS
> M ipc/ipc.gyp
> M ipc/ipc_sync_channel.h
> M ipc/ipc_sync_channel.cc
> M ipc/ipc_sync_message.h
> M ipc/ipc_sync_message.cc
> M ipc/ipc_sync_message_filter.h
> M ipc/ipc_sync_message_filter.cc
> M ipc/run_all_unittests.cc
> M mojo/mojo_public.gyp
> M mojo/public/cpp/bindings/BUILD.gn
> D mojo/public/cpp/bindings/lib/sync_handle_registry.h
> M mojo/public/cpp/bindings/lib/sync_handle_registry.cc
> M mojo/public/cpp/bindings/lib/sync_handle_watcher.h
> A + mojo/public/cpp/bindings/sync_handle_registry.h
> M mojo/public/cpp/system/BUILD.gn
> A mojo/public/cpp/system/event.h
> A mojo/public/cpp/system/event.cc
> M mojo/public/cpp/system/watcher.h
> M ppapi/BUILD.gn
> M ppapi/native_client/BUILD.gn
>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Ken Rockot(use gerrit already), 2016-06-06 21:30:24]

On Mon, Jun 6, 2016 at 2:23 PM, Yuzhu Shen <yzshen@chromium.org> wrote:

>
>
> On Mon, Jun 6, 2016 at 12:32 PM, <rockot@chromium.org> wrote:
>
>> Reviewers: jam, yzshen1
>> CL: https://codereview.chromium.org/2033243003/
>>
>> Message:
>> OK - I think this is ready. Pardon the dust, introducing a Mojo
>> dependency in
>> //ipc means several //ipc dependents now need to express their own
>> dependency on
>> //mojo/edk/system. We can't use public_deps here because Mus and other
>> Mojo apps
>> depend on //ipc from typemaps, and Mojo apps must not depend on
>> //mojo/edk/system.
>>
>
> Is it possible to split the //ipc target? The typemaps of Mus and other
> Mojo apps don't need channel implementations. Maybe it is possible to have
> them depend on something like //ipc:message, and let //ipc depends on both
> //ipc:message and //mojo/edk/system?
>
> WDYT?
>

I started with that approach, but it looked like it would be difficult:
messages depend on brokerable attachments, brokerable attachments depend on
the attachment broker, and the attachment broker appeared to depend on
channels.

Given the incredible number of link failures I'm running into without even
looking at GYP yet, I think I'll revisit the attempt to split ipc. On
second glance the brokering dependency on channels appears to be
unnecessary.


>
>>
>> John could you please look at //ipc changes?
>>
>> Yuzhu could you please look at //mojo/public changes? In particular I'm
>> curious
>> how you feel about making mojo::Event part of the public C++ API.
>> Alternatively
>> we could make it something isolated to //ipc code, but I opted against
>> that
>> since there's nothing ipc-specific about it.
>>
>>
>> Description:
>> Use Mojo pipes to signal sync IPC events
>>
>> This transitions legacy sync IPC to use Mojo message pipe handles
>> for all of its sync event waiting. This is a necessary precursor to
>> mixing sync legacy IPC with sync Mojo IPC, and is also required to
>> supporting correct FIFO between ChannelProxy and Mojo Channel
>> associated interfaces.
>>
>> Specifically:
>>
>> - Introduces a new mojo::Event type which is a WaitableEvent-like
>> interface around a local message pipe.
>> - Moves mojo::SyncHandleRegistry out of internal bindings API and
>> exposes it publicly.
>> - Replaces most uses of WaitableEvent with mojo::Event for sync IPC
>> - Replaces all use of WaitableEvent::WaitMany for sync IPC
>> with mojo::SyncHandleRegistry::WatchAllHandles.
>> - Cleans up some unnecessary complexity in SyncMessage since
>> pump_messages_event() was only being used with a single
>> global event that's always signaled.
>>
>> The system's behavior should be effectively unchanged by this CL,
>> but legacy sync IPC and mojo sync IPC can now be mixed freely.
>>
>> BUG=612500
>>
>>
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
>>
>> Base URL: https://chromium.googlesource.com/chromium/src.git@master
>>
>> Affected files (+337, -233 lines):
>> M chrome/installer/setup/BUILD.gn
>> M components/nacl/loader/BUILD.gn
>> M gpu/ipc/service/BUILD.gn
>> M ipc/BUILD.gn
>> M ipc/DEPS
>> M ipc/ipc.gyp
>> M ipc/ipc_sync_channel.h
>> M ipc/ipc_sync_channel.cc
>> M ipc/ipc_sync_message.h
>> M ipc/ipc_sync_message.cc
>> M ipc/ipc_sync_message_filter.h
>> M ipc/ipc_sync_message_filter.cc
>> M ipc/run_all_unittests.cc
>> M mojo/mojo_public.gyp
>> M mojo/public/cpp/bindings/BUILD.gn
>> D mojo/public/cpp/bindings/lib/sync_handle_registry.h
>> M mojo/public/cpp/bindings/lib/sync_handle_registry.cc
>> M mojo/public/cpp/bindings/lib/sync_handle_watcher.h
>> A + mojo/public/cpp/bindings/sync_handle_registry.h
>> M mojo/public/cpp/system/BUILD.gn
>> A mojo/public/cpp/system/event.h
>> A mojo/public/cpp/system/event.cc
>> M mojo/public/cpp/system/watcher.h
>> M ppapi/BUILD.gn
>> M ppapi/native_client/BUILD.gn
>>
>>
>>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Ken Rockot(use gerrit already), 2016-06-07 04:14:08]

On 2016/06/06 at 21:30:24, Ken Rockot wrote:
> On Mon, Jun 6, 2016 at 2:23 PM, Yuzhu Shen <yzshen@chromium.org> wrote:
> 
> >
> >
> > On Mon, Jun 6, 2016 at 12:32 PM, <rockot@chromium.org> wrote:
> >
> >> Reviewers: jam, yzshen1
> >> CL: https://codereview.chromium.org/2033243003/
> >>
> >> Message:
> >> OK - I think this is ready. Pardon the dust, introducing a Mojo
> >> dependency in
> >> //ipc means several //ipc dependents now need to express their own
> >> dependency on
> >> //mojo/edk/system. We can't use public_deps here because Mus and other
> >> Mojo apps
> >> depend on //ipc from typemaps, and Mojo apps must not depend on
> >> //mojo/edk/system.
> >>
> >
> > Is it possible to split the //ipc target? The typemaps of Mus and other
> > Mojo apps don't need channel implementations. Maybe it is possible to have
> > them depend on something like //ipc:message, and let //ipc depends on both
> > //ipc:message and //mojo/edk/system?
> >
> > WDYT?
> >
> 
> I started with that approach, but it looked like it would be difficult:
> messages depend on brokerable attachments, brokerable attachments depend on
> the attachment broker, and the attachment broker appeared to depend on
> channels.
> 
> Given the incredible number of link failures I'm running into without even
> looking at GYP yet, I think I'll revisit the attempt to split ipc. On
> second glance the brokering dependency on channels appears to be
> unnecessary.

As it turns out, Mus depends on SyncChannel for GPU IPC, so there's really no
way for any part of //ipc to depend on //mojo/edk/system without breaking Mus
linkage. Going to have to stick with updating a bunch of targets to link against
mojo/edk/system.

> 
> 
> >
> >>
> >> John could you please look at //ipc changes?
> >>
> >> Yuzhu could you please look at //mojo/public changes? In particular I'm
> >> curious
> >> how you feel about making mojo::Event part of the public C++ API.
> >> Alternatively
> >> we could make it something isolated to //ipc code, but I opted against
> >> that
> >> since there's nothing ipc-specific about it.
> >>
> >>
> >> Description:
> >> Use Mojo pipes to signal sync IPC events
> >>
> >> This transitions legacy sync IPC to use Mojo message pipe handles
> >> for all of its sync event waiting. This is a necessary precursor to
> >> mixing sync legacy IPC with sync Mojo IPC, and is also required to
> >> supporting correct FIFO between ChannelProxy and Mojo Channel
> >> associated interfaces.
> >>
> >> Specifically:
> >>
> >> - Introduces a new mojo::Event type which is a WaitableEvent-like
> >> interface around a local message pipe.
> >> - Moves mojo::SyncHandleRegistry out of internal bindings API and
> >> exposes it publicly.
> >> - Replaces most uses of WaitableEvent with mojo::Event for sync IPC
> >> - Replaces all use of WaitableEvent::WaitMany for sync IPC
> >> with mojo::SyncHandleRegistry::WatchAllHandles.
> >> - Cleans up some unnecessary complexity in SyncMessage since
> >> pump_messages_event() was only being used with a single
> >> global event that's always signaled.
> >>
> >> The system's behavior should be effectively unchanged by this CL,
> >> but legacy sync IPC and mojo sync IPC can now be mixed freely.
> >>
> >> BUG=612500
> >>
> >>
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
> >>
> >> Base URL: https://chromium.googlesource.com/chromium/src.git@master
> >>
> >> Affected files (+337, -233 lines):
> >> M chrome/installer/setup/BUILD.gn
> >> M components/nacl/loader/BUILD.gn
> >> M gpu/ipc/service/BUILD.gn
> >> M ipc/BUILD.gn
> >> M ipc/DEPS
> >> M ipc/ipc.gyp
> >> M ipc/ipc_sync_channel.h
> >> M ipc/ipc_sync_channel.cc
> >> M ipc/ipc_sync_message.h
> >> M ipc/ipc_sync_message.cc
> >> M ipc/ipc_sync_message_filter.h
> >> M ipc/ipc_sync_message_filter.cc
> >> M ipc/run_all_unittests.cc
> >> M mojo/mojo_public.gyp
> >> M mojo/public/cpp/bindings/BUILD.gn
> >> D mojo/public/cpp/bindings/lib/sync_handle_registry.h
> >> M mojo/public/cpp/bindings/lib/sync_handle_registry.cc
> >> M mojo/public/cpp/bindings/lib/sync_handle_watcher.h
> >> A + mojo/public/cpp/bindings/sync_handle_registry.h
> >> M mojo/public/cpp/system/BUILD.gn
> >> A mojo/public/cpp/system/event.h
> >> A mojo/public/cpp/system/event.cc
> >> M mojo/public/cpp/system/watcher.h
> >> M ppapi/BUILD.gn
> >> M ppapi/native_client/BUILD.gn
> >>
> >>
> >>
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email to chromium-reviews+unsubscribe@chromium.org.
>

[yzshen1, 2016-06-07 17:14:51]

LGTM

https://codereview.chromium.org/2033243003/diff/180001/mojo/public/cpp/system...
File mojo/public/cpp/system/event.h (right):

https://codereview.chromium.org/2033243003/diff/180001/mojo/public/cpp/system...
mojo/public/cpp/system/event.h:19: Event();
Please comment on the initial state, whether the signal is auto-reset, etc.

[Ken Rockot(use gerrit already), 2016-06-07 17:57:56]

Description was changed from

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
support correct FIFO between ChannelProxy and Mojo Channel
associated interfaces. 

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with mojo::Event for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

All TBRs are for addition of required //mojo/edk/system deps
on leaf targets which have a dependency (direct or otherwise)
on //ipc.

BUG=612500
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
TBR=bbudge@chromium.org for ppapi
TBR=bradnelson@chromium.org for components/nacl/loader
TBR=gab@chromium.org for chrome/installer
TBR=sievers@chromium.org for gpu/ipc/service
==========

to

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
support correct FIFO between ChannelProxy and Mojo Channel
associated interfaces. 

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with mojo::Event for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

All TBRs are for addition of required //mojo/edk/system deps
on leaf targets which have a dependency (direct or otherwise)
on //ipc.

BUG=612500
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Ken Rockot(use gerrit already), 2016-06-07 18:02:25]

OK, I've moved mojo::Event to IPC::MojoEvent.

Note: I didn't put MojoEvent in an internal namespace since there are other
internal things in //ipc without such a namespace, and the asymmetry seemed
weird. I also didn't put it in //ipc/mojo to avoid either having circular
dependencies or having mojo_event.h,cc be the only part of //ipc/mojo built as
part of an //ipc target.

In order to reduce the amount of build target churn caused by this, I ended up
introducing a new ipc:channel (gyp ipc_channel) target for the small bits of ipc
which depend on mojo. This still doesn't provide a public dependency on the EDK
because of mus, but the few ipc consumers which need channel stuff have been
updated with both this new dependency and (where appropriate) a direct EDK
dependency.

https://codereview.chromium.org/2033243003/diff/180001/mojo/public/cpp/system...
File mojo/public/cpp/system/event.h (right):

https://codereview.chromium.org/2033243003/diff/180001/mojo/public/cpp/system...
mojo/public/cpp/system/event.h:19: Event();
On 2016/06/07 at 17:14:51, yzshen1 wrote:
> Please comment on the initial state, whether the signal is auto-reset, etc.

Done

[Ken Rockot(use gerrit already), 2016-06-07 18:02:57]

Description was changed from

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
support correct FIFO between ChannelProxy and Mojo Channel
associated interfaces. 

Specifically:

 - Introduces a new mojo::Event type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with mojo::Event for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

All TBRs are for addition of required //mojo/edk/system deps
on leaf targets which have a dependency (direct or otherwise)
on //ipc.

BUG=612500
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
support correct FIFO between ChannelProxy and Mojo Channel
associated interfaces. 

Specifically:

 - Introduces a new IPC::MojoEvent type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with MojoEvent for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

BUG=612500
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Ken Rockot(use gerrit already), 2016-06-07 18:03:06]

Patchset #3 (id:160001) has been deleted

[Ken Rockot(use gerrit already), 2016-06-07 18:03:11]

Patchset #2 (id:140001) has been deleted

[Ken Rockot(use gerrit already), 2016-06-07 18:03:16]

Patchset #1 (id:120001) has been deleted

[jam, 2016-06-07 21:05:05]

lgtm

[Ken Rockot(use gerrit already), 2016-06-08 21:30:21]

rockot@chromium.org changed reviewers:
+ raymes@chromium.org, sievers@chromium.org

[Ken Rockot(use gerrit already), 2016-06-08 21:30:22]

+sievers could you please take a look at GPU. Just moves from using base's
run_all_unittests to IPC's (which brings up Mojo as it's now needed for IPC
SyncChannel at runtime)

+raymes could you please look at ppapi? Similar to gpu, this initializes the EDK
for pappi unit and perf tests, but this also builds mojo public libraries for
NaCl and initializes Mojo within the IRT. I believe this should be sufficient to
bring up the new SyncChannel implementation within the sandbox, but please let
me know if I've missed something.

[no sievers, 2016-06-08 21:57:19]

gpu lgtm

(ipc/ seems a bit odd for the main() function for run_all_unittests, but then i
can't think of a better place and creating a layer seems overkill.)

[Ken Rockot(use gerrit already), 2016-06-08 22:01:24]

On Wed, Jun 8, 2016 at 2:57 PM, <sievers@chromium.org> wrote:

> gpu lgtm
>
> (ipc/ seems a bit odd for the main() function for run_all_unittests, but
> then i
> can't think of a better place and creating a layer seems overkill.)
>

I originally had just added a run_all_unittests.cc in gpu/ipc/server for
this, which is another reasonable option. Just adds a little extra churn.


>
> https://codereview.chromium.org/2033243003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[no sievers, 2016-06-08 22:10:14]

On 2016/06/08 22:01:24, Ken Rockot wrote:
> On Wed, Jun 8, 2016 at 2:57 PM, <mailto:sievers@chromium.org> wrote:
> 
> > gpu lgtm
> >
> > (ipc/ seems a bit odd for the main() function for run_all_unittests, but
> > then i
> > can't think of a better place and creating a layer seems overkill.)
> >
> 
> I originally had just added a run_all_unittests.cc in gpu/ipc/server for
> this, which is another reasonable option. Just adds a little extra churn.
> 

That's maybe just a tiny bit nicer.

[raymes, 2016-06-09 00:54:27]

raymes@chromium.org changed reviewers:
+ bbudge@chromium.org

[raymes, 2016-06-09 00:54:28]

lgtm but please chat with bbudge about the changes in
ppapi/nacl_irt/irt_ppapi.cc. I know nothing about the IRT (amistry seemed to
think this looked good though).

[Ken Rockot(use gerrit already), 2016-06-09 01:07:54]

On Wed, Jun 8, 2016 at 5:54 PM, <raymes@chromium.org> wrote:

> lgtm but please chat with bbudge about the changes in
> ppapi/nacl_irt/irt_ppapi.cc. I know nothing about the IRT (amistry seemed
> to
> think this looked good though).
>

Thanks, will do!


>
> https://codereview.chromium.org/2033243003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[bbudge, 2016-06-09 17:16:58]

On 2016/06/09 01:07:54, Ken Rockot wrote:
> On Wed, Jun 8, 2016 at 5:54 PM, <mailto:raymes@chromium.org> wrote:
> 
> > lgtm but please chat with bbudge about the changes in
> > ppapi/nacl_irt/irt_ppapi.cc. I know nothing about the IRT (amistry seemed
> > to
> > think this looked good though).
> >
> 
> Thanks, will do!
> 
> 
> >
> > https://codereview.chromium.org/2033243003/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

My only concern would be with how much bigger the NaCl IRT is after this change.
I'm guessing it's probably minimal but could you check?

[Ken Rockot(use gerrit already), 2016-06-09 17:19:56]

On 2016/06/09 at 17:16:58, bbudge wrote:
> On 2016/06/09 01:07:54, Ken Rockot wrote:
> > On Wed, Jun 8, 2016 at 5:54 PM, <mailto:raymes@chromium.org> wrote:
> > 
> > > lgtm but please chat with bbudge about the changes in
> > > ppapi/nacl_irt/irt_ppapi.cc. I know nothing about the IRT (amistry seemed
> > > to
> > > think this looked good though).
> > >
> > 
> > Thanks, will do!
> > 
> > 
> > >
> > > https://codereview.chromium.org/2033243003/
> > >
> > 
> > -- 
> > You received this message because you are subscribed to the Google Groups
> > "Chromium-reviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
> 
> My only concern would be with how much bigger the NaCl IRT is after this
change. I'm guessing it's probably minimal but could you check?

My familiarity with the IRT is unfortunately somewhat nonexistent. Can you
please point me to exactly which resources I should be comparing?

[bbudge, 2016-06-09 17:25:45]

On 2016/06/09 17:19:56, Ken Rockot wrote:
> On 2016/06/09 at 17:16:58, bbudge wrote:
> > On 2016/06/09 01:07:54, Ken Rockot wrote:
> > > On Wed, Jun 8, 2016 at 5:54 PM, <mailto:raymes@chromium.org> wrote:
> > > 
> > > > lgtm but please chat with bbudge about the changes in
> > > > ppapi/nacl_irt/irt_ppapi.cc. I know nothing about the IRT (amistry
seemed
> > > > to
> > > > think this looked good though).
> > > >
> > > 
> > > Thanks, will do!
> > > 
> > > 
> > > >
> > > > https://codereview.chromium.org/2033243003/
> > > >
> > > 
> > > -- 
> > > You received this message because you are subscribed to the Google Groups
> > > "Chromium-reviews" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > > to mailto:chromium-reviews+unsubscribe@chromium.org.
> > 
> > My only concern would be with how much bigger the NaCl IRT is after this
> change. I'm guessing it's probably minimal but could you check?
> 
> My familiarity with the IRT is unfortunately somewhat nonexistent. Can you
> please point me to exactly which resources I should be comparing?

Look at the stripped nexe named 'nacl_irt_*.nexe' in the top level build product
directory. It should be around 4 MB.

[Ken Rockot(use gerrit already), 2016-06-09 17:47:05]

On 2016/06/09 at 17:25:45, bbudge wrote:
> On 2016/06/09 17:19:56, Ken Rockot wrote:
> > On 2016/06/09 at 17:16:58, bbudge wrote:
> > > On 2016/06/09 01:07:54, Ken Rockot wrote:
> > > > On Wed, Jun 8, 2016 at 5:54 PM, <mailto:raymes@chromium.org> wrote:
> > > > 
> > > > > lgtm but please chat with bbudge about the changes in
> > > > > ppapi/nacl_irt/irt_ppapi.cc. I know nothing about the IRT (amistry
seemed
> > > > > to
> > > > > think this looked good though).
> > > > >
> > > > 
> > > > Thanks, will do!
> > > > 
> > > > 
> > > > >
> > > > > https://codereview.chromium.org/2033243003/
> > > > >
> > > > 
> > > > -- 
> > > > You received this message because you are subscribed to the Google
Groups
> > > > "Chromium-reviews" group.
> > > > To unsubscribe from this group and stop receiving emails from it, send
an
> > email
> > > > to mailto:chromium-reviews+unsubscribe@chromium.org.
> > > 
> > > My only concern would be with how much bigger the NaCl IRT is after this
> > change. I'm guessing it's probably minimal but could you check?
> > 
> > My familiarity with the IRT is unfortunately somewhat nonexistent. Can you
> > please point me to exactly which resources I should be comparing?
> 
> Look at the stripped nexe named 'nacl_irt_*.nexe' in the top level build
product directory. It should be around 4 MB.

This adds exactly 256 kB to the binary, taking it from 3.63 MB to 3.88 MB. That
seems a bit larger than I expected.

[Ken Rockot(use gerrit already), 2016-06-09 20:01:49]

rockot@chromium.org changed reviewers:
+ mseaborn@chromium.org

[Ken Rockot(use gerrit already), 2016-06-09 20:01:50]

+mseaborn for components/nacl OWNERS and a question: it looks like I'll need to
increase the max pthread key count
(https://cs.chromium.org/chromium/src/native_client/src/untrusted/irt/irt_priv...)
to support this change since Mojo requires some additional thread-local storage
and I see us crossing the current max in some browser tests. Would you have any
concerns with me doing this?

native_client CL is here: https://codereview.chromium.org/2050043004

[Ken Rockot(use gerrit already), 2016-06-14 00:29:33]

Friendly ping mseaborn@ for components/nacl

[Ken Rockot(use gerrit already), 2016-06-14 16:43:02]

rockot@chromium.org changed reviewers:
+ bradnelson@chromium.org, dschuff@chromium.org

[Ken Rockot(use gerrit already), 2016-06-14 16:43:50]

rockot@chromium.org changed reviewers:
- bradnelson@chromium.org, dschuff@chromium.org

[Ken Rockot(use gerrit already), 2016-06-14 16:43:52]

Adding other components/nacl OWNERS. Could someone please take a look at this?
The change is fairly small, but non-trivial enough that a TBR seems
inappropriate.

[Mark Seaborn, 2016-06-14 19:30:21]

https://codereview.chromium.org/2033243003/diff/640001/ppapi/nacl_irt/irt_ppa...
File ppapi/nacl_irt/irt_ppapi.cc (right):

https://codereview.chromium.org/2033243003/diff/640001/ppapi/nacl_irt/irt_ppa...
ppapi/nacl_irt/irt_ppapi.cc:36: mojo::edk::Init();
It looks like you're doing this twice, once here and once in StartUpPlugin() in
ppapi/nacl_irt/plugin_startup.cc.  Only once should be necessary.

[Ken Rockot(use gerrit already), 2016-06-14 20:17:03]

Thanks!

https://codereview.chromium.org/2033243003/diff/640001/ppapi/nacl_irt/irt_ppa...
File ppapi/nacl_irt/irt_ppapi.cc (right):

https://codereview.chromium.org/2033243003/diff/640001/ppapi/nacl_irt/irt_ppa...
ppapi/nacl_irt/irt_ppapi.cc:36: mojo::edk::Init();
On 2016/06/14 at 19:30:21, Mark Seaborn wrote:
> It looks like you're doing this twice, once here and once in StartUpPlugin()
in ppapi/nacl_irt/plugin_startup.cc.  Only once should be necessary.

Oops, removed from here

[Mark Seaborn, 2016-06-14 20:43:15]

LGTM

https://codereview.chromium.org/2033243003/diff/640001/components/nacl/loader...
File components/nacl/loader/nacl_listener.cc (right):

https://codereview.chromium.org/2033243003/diff/640001/components/nacl/loader...
components/nacl/loader/nacl_listener.cc:171: mojo::edk::Init();
NaClMain() in components/nacl/loader/nacl_main.cc is probably a more appropriate
place to put initialisation calls like this.

https://codereview.chromium.org/2033243003/diff/640001/mojo/mojo_public.gyp
File mojo/mojo_public.gyp (right):

https://codereview.chromium.org/2033243003/diff/640001/mojo/mojo_public.gyp#n...
mojo/mojo_public.gyp:1: # Copyright 2014 The Chro`ium Authors. All rights
reserved.
Accidental change here.

[Ken Rockot(use gerrit already), 2016-06-14 20:53:08]

Thanks again

https://codereview.chromium.org/2033243003/diff/640001/components/nacl/loader...
File components/nacl/loader/nacl_listener.cc (right):

https://codereview.chromium.org/2033243003/diff/640001/components/nacl/loader...
components/nacl/loader/nacl_listener.cc:171: mojo::edk::Init();
On 2016/06/14 at 20:43:14, Mark Seaborn wrote:
> NaClMain() in components/nacl/loader/nacl_main.cc is probably a more
appropriate place to put initialisation calls like this.

Done

https://codereview.chromium.org/2033243003/diff/640001/mojo/mojo_public.gyp
File mojo/mojo_public.gyp (right):

https://codereview.chromium.org/2033243003/diff/640001/mojo/mojo_public.gyp#n...
mojo/mojo_public.gyp:1: # Copyright 2014 The Chro`ium Authors. All rights
reserved.
On 2016/06/14 at 20:43:14, Mark Seaborn wrote:
> Accidental change here.

Done

[Ken Rockot(use gerrit already), 2016-06-14 20:53:13]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-06-14 20:53:15]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org,
jam@chromium.org, sievers@chromium.org, raymes@chromium.org,
mseaborn@chromium.org
Link to the patchset: https://codereview.chromium.org/2033243003/#ps680001
(title: ".")

[commit-bot: I haz the power, 2016-06-14 20:53:39]

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

[commit-bot: I haz the power, 2016-06-14 22:12:01]

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

[commit-bot: I haz the power, 2016-06-14 22:12:03]

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

[Mark Seaborn, 2016-06-15 00:35:49]

https://codereview.chromium.org/2033243003/diff/640001/components/nacl/loader...
File components/nacl/loader/nacl_listener.cc (right):

https://codereview.chromium.org/2033243003/diff/640001/components/nacl/loader...
components/nacl/loader/nacl_listener.cc:171: mojo::edk::Init();
On 2016/06/14 20:43:14, Mark Seaborn wrote:
> NaClMain() in components/nacl/loader/nacl_main.cc is probably a more
appropriate
> place to put initialisation calls like this.

Sorry, scratch that -- nacl_main.cc is not used for the Linux case, as you
reminded me in your IM.  You can leave this initialisation part as it was.

[Ken Rockot(use gerrit already), 2016-06-15 03:32:53]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-06-15 03:32:53]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org,
raymes@chromium.org, sievers@chromium.org, mseaborn@chromium.org,
yzshen@chromium.org
Link to the patchset: https://codereview.chromium.org/2033243003/#ps760001
(title: "undo accidental nonsfi test enable")

[commit-bot: I haz the power, 2016-06-15 03:33:12]

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

[commit-bot: I haz the power, 2016-06-15 05:29:01]

Committed patchset #30 (id:760001)

[commit-bot: I haz the power, 2016-06-15 05:30:50]

Description was changed from

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
support correct FIFO between ChannelProxy and Mojo Channel
associated interfaces. 

Specifically:

 - Introduces a new IPC::MojoEvent type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with MojoEvent for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

BUG=612500
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
support correct FIFO between ChannelProxy and Mojo Channel
associated interfaces.

Specifically:

 - Introduces a new IPC::MojoEvent type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with MojoEvent for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

BUG=612500
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/97912711c72d91e50bf6999f6842f9de1c05fab1
Cr-Commit-Position: refs/heads/master@{#399848}
==========

[commit-bot: I haz the power, 2016-06-15 05:30:52]

Patchset 30 (id:??) landed as
https://crrev.com/97912711c72d91e50bf6999f6842f9de1c05fab1
Cr-Commit-Position: refs/heads/master@{#399848}

[tommycli, 2016-06-15 15:53:04]

A revert of this CL (patchset #30 id:760001) has been created in
https://codereview.chromium.org/2067233002/ by tommycli@chromium.org.

The reason for reverting is: Breaks DrMemory builder. Sorry:

https://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20Windows%...

First breaking build:

https://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20Windows%...

Error:

Updating projects from gyp files...

Exception: Unknown AST node at key path 'includes':
Set([Const('mojo_public.gypi')]) while reading
C:\b\build\slave\drm-cr\build\src\mojo\mojo_public.gyp

Traceback (most recent call last):

  File "C:\b\build\slave\drm-cr\build\src\tools\gyp\pylib\gyp\input.py", line
496, in CallLoadTargetBuildFile

    includes, depth, check, False)

  File "C:\b\build\slave\drm-cr\build\src\tools\gyp\pylib\gyp\input.py", line
391, in LoadTargetBuildFile

    includes, True, check)

  File "C:\b\build\slave\drm-cr\build\src\tools\gyp\pylib\gyp\input.py", line
240, in LoadOneBuildFile

    build_file_data = CheckedEval(build_file_contents)

  File "C:\b\build\slave\drm-cr\build\src\tools\gyp\pylib\gyp\input.py", line
194, in CheckedEval

    return CheckNode(c3[0], [])

  File "C:\b\build\slave\drm-cr\build\src\tools\gyp\pylib\gyp\input.py", line
210, in CheckNode

    dict[key] = CheckNode(c[n + 1], kp)

  File "C:\b\build\slave\drm-cr\build\src\tools\gyp\pylib\gyp\input.py", line
224, in CheckNode

    "': " + repr(node))

TypeError: Unknown AST node at key path 'includes':
Set([Const('mojo_public.gypi')]) while reading
C:\b\build\slave\drm-cr\build\src\mojo\mojo_public.gyp

.

[Ken Rockot(use gerrit already), 2016-06-15 16:33:46]

On 2016/06/15 at 15:53:04, tommycli wrote:
> A revert of this CL (patchset #30 id:760001) has been created in
https://codereview.chromium.org/2067233002/ by tommycli@chromium.org.
> 
> The reason for reverting is: Breaks DrMemory builder. Sorry:
> 
>
https://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20Windows%...
> 
> First breaking build:
> 
>
https://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20Windows%...
> 
> Error:
> 
> Updating projects from gyp files...
> 
> Exception: Unknown AST node at key path 'includes':
Set([Const('mojo_public.gypi')]) while reading
C:\b\build\slave\drm-cr\build\src\mojo\mojo_public.gyp
> 
> Traceback (most recent call last):
> 
>   File "C:\b\build\slave\drm-cr\build\src\tools\gyp\pylib\gyp\input.py", line
496, in CallLoadTargetBuildFile
> 
>     includes, depth, check, False)
> 
>   File "C:\b\build\slave\drm-cr\build\src\tools\gyp\pylib\gyp\input.py", line
391, in LoadTargetBuildFile
> 
>     includes, True, check)
> 
>   File "C:\b\build\slave\drm-cr\build\src\tools\gyp\pylib\gyp\input.py", line
240, in LoadOneBuildFile
> 
>     build_file_data = CheckedEval(build_file_contents)
> 
>   File "C:\b\build\slave\drm-cr\build\src\tools\gyp\pylib\gyp\input.py", line
194, in CheckedEval
> 
>     return CheckNode(c3[0], [])
> 
>   File "C:\b\build\slave\drm-cr\build\src\tools\gyp\pylib\gyp\input.py", line
210, in CheckNode
> 
>     dict[key] = CheckNode(c[n + 1], kp)
> 
>   File "C:\b\build\slave\drm-cr\build\src\tools\gyp\pylib\gyp\input.py", line
224, in CheckNode
> 
>     "': " + repr(node))
> 
> TypeError: Unknown AST node at key path 'includes':
Set([Const('mojo_public.gypi')]) while reading
C:\b\build\slave\drm-cr\build\src\mojo\mojo_public.gyp
> 
> .

Huh. I'm surprised the CQ didn't catch this. Fixed, relanding.

[Ken Rockot(use gerrit already), 2016-06-15 16:33:54]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-06-15 16:33:55]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org,
raymes@chromium.org, sievers@chromium.org, mseaborn@chromium.org,
yzshen@chromium.org
Link to the patchset: https://codereview.chromium.org/2033243003/#ps780001
(title: "fix gyp")

[commit-bot: I haz the power, 2016-06-15 16:34:18]

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

[commit-bot: I haz the power, 2016-06-15 18:26:50]

Committed patchset #31 (id:780001)

[commit-bot: I haz the power, 2016-06-15 18:28:46]

Description was changed from

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
support correct FIFO between ChannelProxy and Mojo Channel
associated interfaces.

Specifically:

 - Introduces a new IPC::MojoEvent type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with MojoEvent for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

BUG=612500
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/97912711c72d91e50bf6999f6842f9de1c05fab1
Cr-Commit-Position: refs/heads/master@{#399848}
==========

to

==========
Use Mojo pipes to signal sync IPC events

This transitions legacy sync IPC to use Mojo message pipe handles
for all of its sync event waiting. This is a necessary precursor to
mixing sync legacy IPC with sync Mojo IPC, and is also required to
support correct FIFO between ChannelProxy and Mojo Channel
associated interfaces.

Specifically:

 - Introduces a new IPC::MojoEvent type which is a WaitableEvent-like
   interface around a local message pipe.
 - Moves mojo::SyncHandleRegistry out of internal bindings API and
   exposes it publicly.
 - Replaces most uses of WaitableEvent with MojoEvent for sync IPC
 - Replaces all use of WaitableEvent::WaitMany for sync IPC
   with mojo::SyncHandleRegistry::WatchAllHandles.
 - Cleans up some unnecessary complexity in SyncMessage since
   pump_messages_event() was only being used with a single
   global event that's always signaled.

The system's behavior should be effectively unchanged by this CL,
but legacy sync IPC and mojo sync IPC can now be mixed freely.

BUG=612500
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/97912711c72d91e50bf6999f6842f9de1c05fab1
Committed: https://crrev.com/6d7be626fdfc8e1e85bc2faa789a12a3ae3960a1
Cr-Original-Commit-Position: refs/heads/master@{#399848}
Cr-Commit-Position: refs/heads/master@{#399963}
==========

[commit-bot: I haz the power, 2016-06-15 18:28:48]

Patchset 31 (id:??) landed as
https://crrev.com/6d7be626fdfc8e1e85bc2faa789a12a3ae3960a1
Cr-Commit-Position: refs/heads/master@{#399963}