Use ChannelMojo between browser and service processes.

Use ChannelMojo between browser and service processes.

BUG=604282
Committed: https://crrev.com/552aab7a1adc7428456bb2016cb45b501c65a10c
Cr-Commit-Position: refs/heads/master@{#416192}

[Sam McNally, 2016-07-15 03:03:50]

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

[commit-bot: I haz the power, 2016-07-15 03:04:03]

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-07-15 03:19:53]

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

[commit-bot: I haz the power, 2016-07-15 03:19:54]

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

[Sam McNally, 2016-07-15 04:32:13]

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

[commit-bot: I haz the power, 2016-07-15 04:32:27]

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-07-15 05:09:40]

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

[commit-bot: I haz the power, 2016-07-15 05:09:41]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Sam McNally, 2016-07-15 05:39:03]

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

[commit-bot: I haz the power, 2016-07-15 05:39:10]

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-07-15 06:04:16]

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

[commit-bot: I haz the power, 2016-07-15 06:04:18]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Sam McNally, 2016-07-15 06:07:28]

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

[commit-bot: I haz the power, 2016-07-15 06:07:41]

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-07-15 07:19:57]

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

[commit-bot: I haz the power, 2016-07-15 07:19:58]

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

[Sam McNally, 2016-08-30 04:41:11]

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

[commit-bot: I haz the power, 2016-08-30 04:42:13]

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-30 04:44:26]

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

[commit-bot: I haz the power, 2016-08-30 04:46:51]

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-30 05:37:13]

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

[commit-bot: I haz the power, 2016-08-30 05:37:14]

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

[Sam McNally, 2016-08-30 05:52:56]

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

[commit-bot: I haz the power, 2016-08-30 05:54:09]

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-30 06:34:23]

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

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

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-30 06:34:51]

Patchset #1 (id:1) has been deleted

[Sam McNally, 2016-08-30 06:34:51]

Patchset #2 (id:20001) has been deleted

[Sam McNally, 2016-08-30 06:34:54]

Patchset #3 (id:40001) has been deleted

[Sam McNally, 2016-08-30 06:34:55]

Patchset #3 (id:60001) has been deleted

[Sam McNally, 2016-08-30 06:34:58]

Patchset #3 (id:80001) has been deleted

[Sam McNally, 2016-08-30 06:34:59]

Patchset #3 (id:100001) has been deleted

[Sam McNally, 2016-08-30 06:35:01]

Patchset #4 (id:120001) has been deleted

[Sam McNally, 2016-08-30 06:35:02]

Patchset #4 (id:140001) has been deleted

[Sam McNally, 2016-08-30 06:35:03]

Patchset #4 (id:160001) has been deleted

[Sam McNally, 2016-08-30 06:35:05]

Patchset #5 (id:180001) has been deleted

[Sam McNally, 2016-08-30 06:35:06]

Patchset #4 (id:200001) has been deleted

[Sam McNally, 2016-08-30 06:35:08]

Patchset #4 (id:220001) has been deleted

[Sam McNally, 2016-08-30 06:35:09]

Patchset #5 (id:240001) has been deleted

[Sam McNally, 2016-08-30 06:35:12]

Patchset #5 (id:260001) has been deleted

[Sam McNally, 2016-08-30 06:35:14]

Patchset #5 (id:280001) has been deleted

[Sam McNally, 2016-08-30 06:35:16]

Patchset #5 (id:300001) has been deleted

[Sam McNally, 2016-08-30 06:35:18]

Patchset #5 (id:320001) has been deleted

[Sam McNally, 2016-08-30 06:35:22]

Patchset #5 (id:340001) has been deleted

[Sam McNally, 2016-08-30 06:36:13]

Patchset #2 (id:120001) has been deleted

[Sam McNally, 2016-08-30 06:36:14]

Patchset #1 (id:1) has been deleted

[Sam McNally, 2016-08-30 06:36:15]

Patchset #2 (id:200001) has been deleted

[Sam McNally, 2016-08-30 06:36:17]

Patchset #2 (id:220001) has been deleted

[Sam McNally, 2016-08-30 06:36:19]

Patchset #2 (id:360001) has been deleted

[Sam McNally, 2016-08-30 06:36:22]

Patchset #2 (id:380001) has been deleted

[Sam McNally, 2016-08-30 06:36:37]

Patchset #2 (id:400001) has been deleted

[Sam McNally, 2016-08-30 06:36:39]

Patchset #3 (id:420001) has been deleted

[Sam McNally, 2016-08-30 06:36:42]

Patchset #2 (id:440001) has been deleted

[Sam McNally, 2016-08-30 06:36:44]

Patchset #2 (id:460001) has been deleted

[Sam McNally, 2016-08-30 06:37:00]

Patchset #1 (id:200001) has been deleted

[Sam McNally, 2016-08-30 06:45:47]

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

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

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-30 06:46:05]

Patchset #1 (id:480001) has been deleted

[Sam McNally, 2016-08-30 06:55:31]

Description was changed from

==========
Mojo connections over named pipes for service processes.
==========

to

==========
Use ChannelMojo between browser and service processes.

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

[Sam McNally, 2016-08-30 06:55:41]

Description was changed from

==========
Use ChannelMojo between browser and service processes.

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

to

==========
Use ChannelMojo between browser and service processes.

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

[Sam McNally, 2016-08-30 07:25:23]

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

[Sam McNally, 2016-08-30 07:25:32]

Patchset #1 (id:500001) has been deleted

[commit-bot: I haz the power, 2016-08-30 07:25:47]

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-30 09:40:35]

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

[commit-bot: I haz the power, 2016-08-30 09:40:36]

Dry run: Exceeded global retry quota

[Sam McNally, 2016-08-31 01:42:07]

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

[commit-bot: I haz the power, 2016-08-31 01:42:34]

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-31 02:50:13]

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

[commit-bot: I haz the power, 2016-08-31 02:50:15]

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

[Sam McNally, 2016-08-31 03:23:30]

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

[commit-bot: I haz the power, 2016-08-31 03:23:44]

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-31 03:40:57]

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

[commit-bot: I haz the power, 2016-08-31 03:41:00]

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

[Sam McNally, 2016-08-31 04:47:03]

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

[commit-bot: I haz the power, 2016-08-31 04:47:19]

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-31 05:12:38]

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

[commit-bot: I haz the power, 2016-08-31 05:12:40]

Dry run: This issue passed the CQ dry run.

[Sam McNally, 2016-08-31 05:15:39]

Patchset #1 (id:520001) has been deleted

[Sam McNally, 2016-08-31 05:16:00]

Patchset #1 (id:540001) has been deleted

[Sam McNally, 2016-08-31 06:04:10]

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

[commit-bot: I haz the power, 2016-08-31 06:04:23]

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-31 06:10:10]

Patchset #1 (id:560001) has been deleted

[Sam McNally, 2016-08-31 06:29:50]

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

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

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

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

Dry run: This issue passed the CQ dry run.

[Ken Rockot(use gerrit already), 2016-08-31 17:11:44]

LGTM with one suggestion: Can we just not use IPC::ChannelHandle and instead
have a CreateChannelMessagePipe method?

I have a slight preference for channel setup code explicitly referencing
ChannelMojo::Create*Factory so it's more obvious what's left to convert, and if
this is always wrapping a pipe handle anyway...

[Vitaly Buka (NO REVIEWS), 2016-08-31 23:44:41]

+Lei Zhang <thestig@chromium.org>
I am OOO. And probably after that I'll remove myself from service owners.

On Wed, Aug 31, 2016 at 10:11 AM <rockot@chromium.org> wrote:

> LGTM with one suggestion: Can we just not use IPC::ChannelHandle and
> instead
> have a CreateChannelMessagePipe method?
>
> I have a slight preference for channel setup code explicitly referencing
> ChannelMojo::Create*Factory so it's more obvious what's left to convert,
> and if
> this is always wrapping a pipe handle anyway...
>
> https://codereview.chromium.org/2139643003/
>

-- 
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.

[Sam McNally, 2016-09-01 00:40:50]

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

[commit-bot: I haz the power, 2016-09-01 00:41:32]

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-09-01 00:46:44]

sammc@chromium.org changed reviewers:
+ thestig@chromium.org
- vitalybuka@chromium.org

[Sam McNally, 2016-09-01 01:03:55]

On 2016/08/31 17:11:44, Ken Rockot wrote:
> LGTM with one suggestion: Can we just not use IPC::ChannelHandle and instead
> have a CreateChannelMessagePipe method?
> 
> I have a slight preference for channel setup code explicitly referencing
> ChannelMojo::Create*Factory so it's more obvious what's left to convert, and
if
> this is always wrapping a pipe handle anyway...

Done.

[commit-bot: I haz the power, 2016-09-01 02:25:28]

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

[commit-bot: I haz the power, 2016-09-01 02:25:29]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-09-01 06:12:01]

lgtm

https://codereview.chromium.org/2139643003/diff/590001/chrome/browser/service...
File chrome/browser/service_process/service_process_control_browsertest.cc
(right):

https://codereview.chromium.org/2139643003/diff/590001/chrome/browser/service...
chrome/browser/service_process/service_process_control_browsertest.cc:46: void
LaunchServiceProcessControl(bool wait = true) {
Can we avoid having default arguments? There can't be that many callers.

https://codereview.chromium.org/2139643003/diff/590001/chrome/service/service...
File chrome/service/service_ipc_server_unittest.cc (right):

https://codereview.chromium.org/2139643003/diff/590001/chrome/service/service...
chrome/service/service_ipc_server_unittest.cc:46: mojo::ScopedMessagePipeHandle
channel_handle;
channel_handle_ ? Fix the over variables if you'd like to.

[Sam McNally, 2016-09-01 06:57:04]

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

[commit-bot: I haz the power, 2016-09-01 06:57:19]

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-09-01 06:58:44]

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

[Sam McNally, 2016-09-01 06:58:45]

+dcheng for chrome/common/service_messages.h

https://codereview.chromium.org/2139643003/diff/590001/chrome/browser/service...
File chrome/browser/service_process/service_process_control_browsertest.cc
(right):

https://codereview.chromium.org/2139643003/diff/590001/chrome/browser/service...
chrome/browser/service_process/service_process_control_browsertest.cc:46: void
LaunchServiceProcessControl(bool wait = true) {
On 2016/09/01 06:12:01, Lei Zhang wrote:
> Can we avoid having default arguments? There can't be that many callers.

Done.

https://codereview.chromium.org/2139643003/diff/590001/chrome/service/service...
File chrome/service/service_ipc_server_unittest.cc (right):

https://codereview.chromium.org/2139643003/diff/590001/chrome/service/service...
chrome/service/service_ipc_server_unittest.cc:46: mojo::ScopedMessagePipeHandle
channel_handle;
On 2016/09/01 06:12:01, Lei Zhang wrote:
> channel_handle_ ? Fix the over variables if you'd like to.

Done.

[commit-bot: I haz the power, 2016-09-01 07:30:12]

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

[commit-bot: I haz the power, 2016-09-01 07:30:16]

Dry run: This issue passed the CQ dry run.

[Sam McNally, 2016-09-01 08:11:55]

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

[commit-bot: I haz the power, 2016-09-01 08:12:03]

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-09-01 09:29:00]

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

[commit-bot: I haz the power, 2016-09-01 09:29:02]

Dry run: This issue passed the CQ dry run.

[dcheng, 2016-09-01 16:54:16]

Per our chat, it'd be nice to do this without the test-only IPC. This one seems
fairly innocuous, but test-only IPCs are exposed in production code as well.
It'd be nice to avoid adding too many of these.

[Sam McNally, 2016-09-01 23:35:02]

On 2016/09/01 16:54:16, dcheng wrote:
> Per our chat, it'd be nice to do this without the test-only IPC. This one
seems
> fairly innocuous, but test-only IPCs are exposed in production code as well.
> It'd be nice to avoid adding too many of these.

Done.

[Sam McNally, 2016-09-02 05:15:23]

The CQ bit was checked by sammc@chromium.org

[Sam McNally, 2016-09-02 05:15:24]

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

[commit-bot: I haz the power, 2016-09-02 05:15:39]

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-09-02 05:20:00]

Description was changed from

==========
Use ChannelMojo between browser and service processes.

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

to

==========
Use ChannelMojo between browser and service processes.

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

[commit-bot: I haz the power, 2016-09-02 05:20:04]

Committed patchset #4 (id:630001)

[commit-bot: I haz the power, 2016-09-02 05:23:06]

Description was changed from

==========
Use ChannelMojo between browser and service processes.

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

to

==========
Use ChannelMojo between browser and service processes.

BUG=604282

Committed: https://crrev.com/552aab7a1adc7428456bb2016cb45b501c65a10c
Cr-Commit-Position: refs/heads/master@{#416192}
==========

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

Patchset 4 (id:??) landed as
https://crrev.com/552aab7a1adc7428456bb2016cb45b501c65a10c
Cr-Commit-Position: refs/heads/master@{#416192}