Support mojo connections between unrelated peer processes.

Support mojo connections between unrelated peer processes.

Currently, the mojo system assumes that a single, trusted and long-lived
process will act as the broker process for all other processes. For
connections between browser and service processes, each process can
launch and shut down at arbitrary times relative to the other and as
such neither assumption is satisfied. Thus, to support connections
between browser and service processes, a peer relationship between
processes communicating over mojo is required.

BUG=604282
TBR=rockot@chromium.org to workaround https://crbug.com/636215

Committed: https://crrev.com/b0a39f8c62466dc6c94de125a1671855d30e0968
Cr-Commit-Position: refs/heads/master@{#410981}

[Sam McNally, 2016-08-08 07:45:22]

The CQ bit was checked by sammc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-08 07:45:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-08 08:39:01]

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

[commit-bot: I haz the power, 2016-08-08 08:39:02]

Dry run: This issue passed the CQ dry run.

[Sam McNally, 2016-08-08 09:32:48]

The CQ bit was checked by sammc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-08 09:32:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sam McNally, 2016-08-08 09:36:56]

Patchset #2 (id:20001) has been deleted

[Sam McNally, 2016-08-08 09:41:35]

Patchset #1 (id:1) has been deleted

[Sam McNally, 2016-08-08 09:41:53]

The CQ bit was checked by sammc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-08 09:42:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-08 10:40:37]

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

[commit-bot: I haz the power, 2016-08-08 10:40:38]

Dry run: This issue passed the CQ dry run.

[Sam McNally, 2016-08-09 01:50:44]

The CQ bit was checked by sammc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-09 01:51:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-09 02:44:19]

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

[commit-bot: I haz the power, 2016-08-09 02:44:19]

Dry run: This issue passed the CQ dry run.

[Sam McNally, 2016-08-09 03:00:56]

Description was changed from

==========
Support mojo connections between unrelated processes.
==========

to

==========
Support mojo connections between unrelated peer processes.

Currently, the mojo system assumes that a single, trusted and long-lived
process will act as the broker process for all other processes. For
connections between browser and service processes, each process can
launch and shut down at arbitrary times relative to the other and as
such neither assumption is satisfied. Thus, to support connections
between browser and service processes, a peer relationship between
processes communicating over mojo is required.

BUG=604282
==========

[Sam McNally, 2016-08-09 03:01:16]

Description was changed from

==========
Support mojo connections between unrelated peer processes.

Currently, the mojo system assumes that a single, trusted and long-lived
process will act as the broker process for all other processes. For
connections between browser and service processes, each process can
launch and shut down at arbitrary times relative to the other and as
such neither assumption is satisfied. Thus, to support connections
between browser and service processes, a peer relationship between
processes communicating over mojo is required.

BUG=604282
==========

to

==========
Support mojo connections between unrelated peer processes.

Currently, the mojo system assumes that a single, trusted and long-lived
process will act as the broker process for all other processes. For
connections between browser and service processes, each process can
launch and shut down at arbitrary times relative to the other and as
such neither assumption is satisfied. Thus, to support connections
between browser and service processes, a peer relationship between
processes communicating over mojo is required.

BUG=604282
==========

[Sam McNally, 2016-08-09 03:10:48]

The CQ bit was checked by sammc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-09 03:11:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sam McNally, 2016-08-09 03:17:28]

Patchset #1 (id:40001) has been deleted

[Sam McNally, 2016-08-09 03:17:51]

Patchset #1 (id:60001) has been deleted

[commit-bot: I haz the power, 2016-08-09 04:55:44]

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

[commit-bot: I haz the power, 2016-08-09 04:55:45]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Sam McNally, 2016-08-09 06:55:15]

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

[Ken Rockot(use gerrit already), 2016-08-09 20:54:56]

LGTM
This seems like an elegant solution to the problem. We have to be cautious that
the new NodeChannel message is never** used in a Chrome OS or ARC environment,
but that doesn't seem like an issue since it requires an explicit EDK call.

I wonder what we can do about brokering. Maybe ConnectToPeerProcess could be
changed (not necessarily in this CL) to ConnectToPeerBrokerProcess, and we could
enforce that both ends of the platform pipe are brokers. Then they could
delegate to each other as needed. NodeController would have to keep track of
more than just a peer map in this case, as brokers would need to know if they
are capable of brokering on a peer's behalf of if the need to delegate
elsewhere.

** at least until they next uprev libchrome

https://codereview.chromium.org/2227553002/diff/80001/mojo/edk/embedder/embed...
File mojo/edk/embedder/embedder.h (right):

https://codereview.chromium.org/2227553002/diff/80001/mojo/edk/embedder/embed...
mojo/edk/embedder/embedder.h:73: // Called to connect to a peer process. A
process is a peer when there exists no
nit: Not strictly true. Though we don't expose a "peer" concept otherwise, we
internally refer to "peers" quite a bit and in most cases they do have a common
ancestor.

[Sam McNally, 2016-08-10 00:30:44]

Patchset #2 (id:100001) has been deleted

[Sam McNally, 2016-08-10 01:40:14]

On 2016/08/09 20:54:56, Ken Rockot (OOO 8.11-8.16) wrote:
> LGTM
> This seems like an elegant solution to the problem. We have to be cautious
that
> the new NodeChannel message is never** used in a Chrome OS or ARC environment,
> but that doesn't seem like an issue since it requires an explicit EDK call.
> 
> I wonder what we can do about brokering. Maybe ConnectToPeerProcess could be
> changed (not necessarily in this CL) to ConnectToPeerBrokerProcess, and we
could
> enforce that both ends of the platform pipe are brokers. Then they could
> delegate to each other as needed. NodeController would have to keep track of
> more than just a peer map in this case, as brokers would need to know if they
> are capable of brokering on a peer's behalf of if the need to delegate
> elsewhere.
> 
That sounds reasonable for a two-broker system. Things could get quite
complicated if we allow more complicated topologies. Fortunately, the browser
and service processes just communicate directly so we can punt on it until
something needs it.

https://codereview.chromium.org/2227553002/diff/80001/mojo/edk/embedder/embed...
File mojo/edk/embedder/embedder.h (right):

https://codereview.chromium.org/2227553002/diff/80001/mojo/edk/embedder/embed...
mojo/edk/embedder/embedder.h:73: // Called to connect to a peer process. A
process is a peer when there exists no
On 2016/08/09 20:54:56, Ken Rockot (OOO 8.11-8.16) wrote:
> nit: Not strictly true. Though we don't expose a "peer" concept otherwise, we
> internally refer to "peers" quite a bit and in most cases they do have a
common
> ancestor.

Done.

[Sam McNally, 2016-08-10 01:40:37]

The CQ bit was checked by sammc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 01:41:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 03:10:14]

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

[commit-bot: I haz the power, 2016-08-10 03:10:15]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Sam McNally, 2016-08-10 03:27:26]

The CQ bit was checked by sammc@chromium.org

[Sam McNally, 2016-08-10 03:27:26]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org
Link to the patchset: https://codereview.chromium.org/2227553002/#ps140001
(title: " ")

[commit-bot: I haz the power, 2016-08-10 03:28:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 03:35:35]

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

[commit-bot: I haz the power, 2016-08-10 03:35:36]

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

[Sam McNally, 2016-08-10 05:03:51]

Description was changed from

==========
Support mojo connections between unrelated peer processes.

Currently, the mojo system assumes that a single, trusted and long-lived
process will act as the broker process for all other processes. For
connections between browser and service processes, each process can
launch and shut down at arbitrary times relative to the other and as
such neither assumption is satisfied. Thus, to support connections
between browser and service processes, a peer relationship between
processes communicating over mojo is required.

BUG=604282
==========

to

==========
Support mojo connections between unrelated peer processes.

Currently, the mojo system assumes that a single, trusted and long-lived
process will act as the broker process for all other processes. For
connections between browser and service processes, each process can
launch and shut down at arbitrary times relative to the other and as
such neither assumption is satisfied. Thus, to support connections
between browser and service processes, a peer relationship between
processes communicating over mojo is required.

BUG=604282
TBR=rockot@chromium.org to workaround https://crbug.com/636215
==========

[Sam McNally, 2016-08-10 05:05:22]

The CQ bit was checked by sammc@chromium.org

[commit-bot: I haz the power, 2016-08-10 05:05:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-10 06:30:09]

Description was changed from

==========
Support mojo connections between unrelated peer processes.

Currently, the mojo system assumes that a single, trusted and long-lived
process will act as the broker process for all other processes. For
connections between browser and service processes, each process can
launch and shut down at arbitrary times relative to the other and as
such neither assumption is satisfied. Thus, to support connections
between browser and service processes, a peer relationship between
processes communicating over mojo is required.

BUG=604282
TBR=rockot@chromium.org to workaround https://crbug.com/636215
==========

to

==========
Support mojo connections between unrelated peer processes.

Currently, the mojo system assumes that a single, trusted and long-lived
process will act as the broker process for all other processes. For
connections between browser and service processes, each process can
launch and shut down at arbitrary times relative to the other and as
such neither assumption is satisfied. Thus, to support connections
between browser and service processes, a peer relationship between
processes communicating over mojo is required.

BUG=604282
TBR=rockot@chromium.org to workaround https://crbug.com/636215
==========

[commit-bot: I haz the power, 2016-08-10 06:30:10]

Committed patchset #3 (id:140001)

[commit-bot: I haz the power, 2016-08-10 06:31:48]

Description was changed from

==========
Support mojo connections between unrelated peer processes.

Currently, the mojo system assumes that a single, trusted and long-lived
process will act as the broker process for all other processes. For
connections between browser and service processes, each process can
launch and shut down at arbitrary times relative to the other and as
such neither assumption is satisfied. Thus, to support connections
between browser and service processes, a peer relationship between
processes communicating over mojo is required.

BUG=604282
TBR=rockot@chromium.org to workaround https://crbug.com/636215
==========

to

==========
Support mojo connections between unrelated peer processes.

Currently, the mojo system assumes that a single, trusted and long-lived
process will act as the broker process for all other processes. For
connections between browser and service processes, each process can
launch and shut down at arbitrary times relative to the other and as
such neither assumption is satisfied. Thus, to support connections
between browser and service processes, a peer relationship between
processes communicating over mojo is required.

BUG=604282
TBR=rockot@chromium.org to workaround https://crbug.com/636215

Committed: https://crrev.com/b0a39f8c62466dc6c94de125a1671855d30e0968
Cr-Commit-Position: refs/heads/master@{#410981}
==========

[commit-bot: I haz the power, 2016-08-10 06:31:49]

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