Add RegisterService, split out of Connect().

Add RegisterService, split out of Connect().

- Eliminates client lib's ConnectParams struct as this is no longer needed.
- Converts call sites.
- In Service Manager, RegisterService just calls Connect() with some empty values. The codepath is mostly the same. Note that when the ClientProcessConnection is passed now OnGotResolvedName returns early prior to completing the connection, as RegisterService is not a "connect" action, just a "start" one.

R=rockot@chromium.org,tsepez@chromium.org

Review-Url: https://codereview.chromium.org/2610173003
Cr-Commit-Position: refs/heads/master@{#442114}
Committed: https://chromium.googlesource.com/chromium/src/+/e6a9f01dbfbc97e63b90de01b329a1b1f8fe122f

[Ben Goodger (Google), 2017-01-05 20:08:38]

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

[commit-bot: I haz the power, 2017-01-05 20:09:13]

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

[Ben Goodger (Google), 2017-01-05 20:12:53]

Description was changed from

==========
Add RegisterService.

BUG=
==========

to

==========
Add RegisterService, split out of Connect().

- Eliminates client lib's ConnectParams struct as this is no longer needed.
- Converts call sites.
- In Service Manager, RegisterService just calls Connect() with some empty
values. The codepath is mostly the same. Note that when the
ClientProcessConnection is passed now OnGotResolvedName returns early prior to
completing the connection, as RegisterService is not a "connect" action, just a
"start" one.

R=rockot@chromium.org
==========

[Ben Goodger (Google), 2017-01-05 20:12:54]

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

[Ben Goodger (Google), 2017-01-05 20:13:40]

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

[commit-bot: I haz the power, 2017-01-05 20:13:56]

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

[Ken Rockot(use gerrit already), 2017-01-05 20:31:48]

lgtm

https://codereview.chromium.org/2610173003/diff/40001/services/service_manage...
File services/service_manager/public/interfaces/connector.mojom (right):

https://codereview.chromium.org/2610173003/diff/40001/services/service_manage...
services/service_manager/public/interfaces/connector.mojom:92:
RegisterService(Identity name,
nit: I think RegisterInstance might be a more accurate name

https://codereview.chromium.org/2610173003/diff/40001/services/service_manage...
services/service_manager/public/interfaces/connector.mojom:93:
ClientProcessConnection client_process_connection);
nit: Maybe just flatten service and pid_receiver_request into the args here?
Also you could use a PIDReceiver& instead of a raw pipe handle.

[commit-bot: I haz the power, 2017-01-05 20:36:27]

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

[commit-bot: I haz the power, 2017-01-05 20:36:27]

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

[Ben Goodger (Google), 2017-01-05 22:28:12]

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

[commit-bot: I haz the power, 2017-01-05 22:28:49]

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

[Ben Goodger (Google), 2017-01-05 22:31:19]

On 2017/01/05 20:31:48, Ken Rockot wrote:
> lgtm
> 
>
https://codereview.chromium.org/2610173003/diff/40001/services/service_manage...
> File services/service_manager/public/interfaces/connector.mojom (right):
> 
>
https://codereview.chromium.org/2610173003/diff/40001/services/service_manage...
> services/service_manager/public/interfaces/connector.mojom:92:
> RegisterService(Identity name,
> nit: I think RegisterInstance might be a more accurate name

Are you sure you want the term "Instance" to leak out of being a private
implementation detail of service manager?

BTW I'm still not sold on Register, FWIW. There's some niceness to "Start" as it
maps cleanly to the Service method that gets called when this method is run -
OnStart, like Connect, which results in OnConnect.......

>
https://codereview.chromium.org/2610173003/diff/40001/services/service_manage...
> services/service_manager/public/interfaces/connector.mojom:93:
> ClientProcessConnection client_process_connection);
> nit: Maybe just flatten service and pid_receiver_request into the args here?
> Also you could use a PIDReceiver& instead of a raw pipe handle.

Done.

[Ben Goodger (Google), 2017-01-05 22:31:47]

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

[commit-bot: I haz the power, 2017-01-05 22:32:10]

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

[Ben Goodger (Google), 2017-01-05 22:40:36]

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

[commit-bot: I haz the power, 2017-01-05 22:41:04]

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, 2017-01-05 22:52:25]

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

[commit-bot: I haz the power, 2017-01-05 22:52:26]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Ben Goodger (Google), 2017-01-05 22:58:53]

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

[commit-bot: I haz the power, 2017-01-05 22:59:35]

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

[Ben Goodger (Google), 2017-01-05 23:14:58]

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

[commit-bot: I haz the power, 2017-01-05 23:15:28]

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, 2017-01-05 23:30:48]

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

[commit-bot: I haz the power, 2017-01-05 23:30:48]

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

[Ben Goodger (Google), 2017-01-05 23:35:44]

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

[commit-bot: I haz the power, 2017-01-05 23:36:35]

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, 2017-01-05 23:52:47]

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

[commit-bot: I haz the power, 2017-01-05 23:52:48]

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

[Ben Goodger (Google), 2017-01-06 19:17:37]

The CQ bit was checked by ben@chromium.org

[Ben Goodger (Google), 2017-01-06 19:17:38]

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/2610173003/#ps170001
(title: ".")

[commit-bot: I haz the power, 2017-01-06 19:18:13]

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

[Ben Goodger (Google), 2017-01-06 19:20:46]

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

[Ben Goodger (Google), 2017-01-06 19:21:17]

Description was changed from

==========
Add RegisterService, split out of Connect().

- Eliminates client lib's ConnectParams struct as this is no longer needed.
- Converts call sites.
- In Service Manager, RegisterService just calls Connect() with some empty
values. The codepath is mostly the same. Note that when the
ClientProcessConnection is passed now OnGotResolvedName returns early prior to
completing the connection, as RegisterService is not a "connect" action, just a
"start" one.

R=rockot@chromium.org
==========

to

==========
Add RegisterService, split out of Connect().

- Eliminates client lib's ConnectParams struct as this is no longer needed.
- Converts call sites.
- In Service Manager, RegisterService just calls Connect() with some empty
values. The codepath is mostly the same. Note that when the
ClientProcessConnection is passed now OnGotResolvedName returns early prior to
completing the connection, as RegisterService is not a "connect" action, just a
"start" one.

R=rockot@chromium.org,tsepez@chromium.org
==========

[Ben Goodger (Google), 2017-01-06 19:22:06]

Tom: this is moving a parameter to Connect() into a separate function call.

[Tom Sepez, 2017-01-06 19:33:35]

mojom LGTM

[commit-bot: I haz the power, 2017-01-06 19:57:44]

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

[commit-bot: I haz the power, 2017-01-06 19:57:45]

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

[Ben Goodger (Google), 2017-01-06 20:40:47]

The CQ bit was checked by ben@chromium.org

[Ben Goodger (Google), 2017-01-06 20:40:49]

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

[commit-bot: I haz the power, 2017-01-06 20:41:30]

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, 2017-01-06 21:02:28]

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

[commit-bot: I haz the power, 2017-01-06 21:02:30]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Ben Goodger (Google), 2017-01-06 21:31:56]

The CQ bit was checked by ben@chromium.org

[commit-bot: I haz the power, 2017-01-06 21:32:29]

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, 2017-01-06 21:52:12]

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

[commit-bot: I haz the power, 2017-01-06 21:52:13]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Ben Goodger (Google), 2017-01-06 23:01:13]

The CQ bit was checked by ben@chromium.org

[commit-bot: I haz the power, 2017-01-06 23:01:58]

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, 2017-01-07 00:43:49]

CQ is committing da patch.

Bot data: {"patchset_id": 190001, "attempt_start_ts": 1483743673380770,
"parent_rev": "c3c37251cda5e7e0f8679de7c0d8df4df1709fad", "commit_rev":
"e6a9f01dbfbc97e63b90de01b329a1b1f8fe122f"}

[commit-bot: I haz the power, 2017-01-07 00:44:15]

Description was changed from

==========
Add RegisterService, split out of Connect().

- Eliminates client lib's ConnectParams struct as this is no longer needed.
- Converts call sites.
- In Service Manager, RegisterService just calls Connect() with some empty
values. The codepath is mostly the same. Note that when the
ClientProcessConnection is passed now OnGotResolvedName returns early prior to
completing the connection, as RegisterService is not a "connect" action, just a
"start" one.

R=rockot@chromium.org,tsepez@chromium.org
==========

to

==========
Add RegisterService, split out of Connect().

- Eliminates client lib's ConnectParams struct as this is no longer needed.
- Converts call sites.
- In Service Manager, RegisterService just calls Connect() with some empty
values. The codepath is mostly the same. Note that when the
ClientProcessConnection is passed now OnGotResolvedName returns early prior to
completing the connection, as RegisterService is not a "connect" action, just a
"start" one.

R=rockot@chromium.org,tsepez@chromium.org

Review-Url: https://codereview.chromium.org/2610173003
Cr-Commit-Position: refs/heads/master@{#442114}
Committed:
https://chromium.googlesource.com/chromium/src/+/e6a9f01dbfbc97e63b90de01b329...
==========

[commit-bot: I haz the power, 2017-01-07 00:44:17]

Committed patchset #11 (id:190001) as
https://chromium.googlesource.com/chromium/src/+/e6a9f01dbfbc97e63b90de01b329...