Add service name constant for UI service.

Add service name constant for UI service.

R=jamescook@chromium.org
TBR=tsepez@chromium.org

Committed: https://crrev.com/06065b68c15e9dff715a04d5d0e4bd93339a806b
Cr-Commit-Position: refs/heads/master@{#432393}

[Ben Goodger (Google), 2016-11-15 21:38:48]

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

[commit-bot: I haz the power, 2016-11-15 21:39:05]

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-11-15 21:53:39]

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

[commit-bot: I haz the power, 2016-11-15 21:53:40]

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...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[Ben Goodger (Google), 2016-11-15 22:02:25]

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

[commit-bot: I haz the power, 2016-11-15 22:03:18]

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-11-15 22:15:17]

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

[commit-bot: I haz the power, 2016-11-15 22:15:18]

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...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  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), 2016-11-15 22:50:54]

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

[commit-bot: I haz the power, 2016-11-15 22:51:29]

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-11-15 23:00:04]

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

[commit-bot: I haz the power, 2016-11-15 23:00:05]

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...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Ben Goodger (Google), 2016-11-15 23:12:25]

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

[commit-bot: I haz the power, 2016-11-15 23:12: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-11-15 23:40:01]

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

[commit-bot: I haz the power, 2016-11-15 23:40:02]

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), 2016-11-16 00:40:30]

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

[commit-bot: I haz the power, 2016-11-16 00:41:17]

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

[James Cook, 2016-11-16 00:46:03]

LGTM with a question about how content will work

https://codereview.chromium.org/2503063003/diff/60001/mash/init/init.cc
File mash/init/init.cc (right):

https://codereview.chromium.org/2503063003/diff/60001/mash/init/init.cc#newco...
mash/init/init.cc:24: context()->connector()->Connect(ui::mojom::kServiceName);
I like this. It's clear to me that it's connecting to a mojo service, even if I
don't know what a context or a connector are.

https://codereview.chromium.org/2503063003/diff/60001/services/ui/public/inte...
File services/ui/public/interfaces/constants.mojom (right):

https://codereview.chromium.org/2503063003/diff/60001/services/ui/public/inte...
services/ui/public/interfaces/constants.mojom:7: const string kServiceName =
"ui";
How is this going to work for things like the content module that have more than
1 service? content::mojom::kBrowserServiceName?
content::browser::mojom::kServiceName?

[Ben Goodger (Google), 2016-11-16 00:48:38]

On 2016/11/16 00:46:03, James Cook wrote:
> LGTM with a question about how content will work
> 
> https://codereview.chromium.org/2503063003/diff/60001/mash/init/init.cc
> File mash/init/init.cc (right):
> 
>
https://codereview.chromium.org/2503063003/diff/60001/mash/init/init.cc#newco...
> mash/init/init.cc:24:
context()->connector()->Connect(ui::mojom::kServiceName);
> I like this. It's clear to me that it's connecting to a mojo service, even if
I
> don't know what a context or a connector are.
> 
>
https://codereview.chromium.org/2503063003/diff/60001/services/ui/public/inte...
> File services/ui/public/interfaces/constants.mojom (right):
> 
>
https://codereview.chromium.org/2503063003/diff/60001/services/ui/public/inte...
> services/ui/public/interfaces/constants.mojom:7: const string kServiceName =
> "ui";
> How is this going to work for things like the content module that have more
than
> 1 service? content::mojom::kBrowserServiceName?
> content::browser::mojom::kServiceName?

I'd go for kFooServiceName for content. It already doesn't conform to the
directory layout standard of //services. Content is providing multiple services
due to an anti-pattern (it being monolithic) so ultimately we'll end up with
network::mojom::kServiceName, device::mojom::kServiceName,
navigation::mojom::kServiceName and so on. Seem reasonable?

[James Cook, 2016-11-16 00:50:58]

On 2016/11/16 00:48:38, Ben Goodger (Google) wrote:
> On 2016/11/16 00:46:03, James Cook wrote:
> > LGTM with a question about how content will work
> > 
> > https://codereview.chromium.org/2503063003/diff/60001/mash/init/init.cc
> > File mash/init/init.cc (right):
> > 
> >
>
https://codereview.chromium.org/2503063003/diff/60001/mash/init/init.cc#newco...
> > mash/init/init.cc:24:
> context()->connector()->Connect(ui::mojom::kServiceName);
> > I like this. It's clear to me that it's connecting to a mojo service, even
if
> I
> > don't know what a context or a connector are.
> > 
> >
>
https://codereview.chromium.org/2503063003/diff/60001/services/ui/public/inte...
> > File services/ui/public/interfaces/constants.mojom (right):
> > 
> >
>
https://codereview.chromium.org/2503063003/diff/60001/services/ui/public/inte...
> > services/ui/public/interfaces/constants.mojom:7: const string kServiceName =
> > "ui";
> > How is this going to work for things like the content module that have more
> than
> > 1 service? content::mojom::kBrowserServiceName?
> > content::browser::mojom::kServiceName?
> 
> I'd go for kFooServiceName for content. It already doesn't conform to the
> directory layout standard of //services. Content is providing multiple
services
> due to an anti-pattern (it being monolithic) so ultimately we'll end up with
> network::mojom::kServiceName, device::mojom::kServiceName,
> navigation::mojom::kServiceName and so on. Seem reasonable?

SGTM

[commit-bot: I haz the power, 2016-11-16 03:25:50]

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

[commit-bot: I haz the power, 2016-11-16 03:25:51]

Dry run: This issue passed the CQ dry run.

[Ben Goodger (Google), 2016-11-16 04:00:01]

The CQ bit was checked by ben@chromium.org

[commit-bot: I haz the power, 2016-11-16 04:00:26]

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

[Ben Goodger (Google), 2016-11-16 04:04:31]

Description was changed from

==========
Add service name constant for UI service.

R=jamescook@chromium.org
==========

to

==========
Add service name constant for UI service.

R=jamescook@chromium.org
TBR=tsepez@chromium.org
==========

[Ben Goodger (Google), 2016-11-16 04:04:31]

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

[Ben Goodger (Google), 2016-11-16 04:05:05]

+tbr tsepez, moving service name into a mojom constant for consistency. (no new
capabilities exposed).

[commit-bot: I haz the power, 2016-11-16 06:30:15]

Description was changed from

==========
Add service name constant for UI service.

R=jamescook@chromium.org
TBR=tsepez@chromium.org
==========

to

==========
Add service name constant for UI service.

R=jamescook@chromium.org
TBR=tsepez@chromium.org
==========

[commit-bot: I haz the power, 2016-11-16 06:30:17]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-16 06:33:31]

Description was changed from

==========
Add service name constant for UI service.

R=jamescook@chromium.org
TBR=tsepez@chromium.org
==========

to

==========
Add service name constant for UI service.

R=jamescook@chromium.org
TBR=tsepez@chromium.org

Committed: https://crrev.com/06065b68c15e9dff715a04d5d0e4bd93339a806b
Cr-Commit-Position: refs/heads/master@{#432393}
==========

[commit-bot: I haz the power, 2016-11-16 06:33:32]

Patchset 5 (id:??) landed as
https://crrev.com/06065b68c15e9dff715a04d5d0e4bd93339a806b
Cr-Commit-Position: refs/heads/master@{#432393}