[Device Service] Move PowerMonitor into the Device Service

[Device Service] Move PowerMonitor into the Device Service

This CL
  - configures Device Service to be singleton service.
  - lets content::ServiceManagerContext initiate a connection to Device,
    Service to create the singleton instance which will serve all clients.
  - moves PowerMonitor to being hosted by the Device Service.

BUG=647247
Review-Url: https://codereview.chromium.org/2510033002
Cr-Commit-Position: refs/heads/master@{#442550}
Committed: https://chromium.googlesource.com/chromium/src/+/918f72f14f7840b8e40c2b0ca5f95817f691ebd6

[leonhsl(Using Gerrit), 2016-11-17 09:56:26]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-17 09:56:43]

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-17 10:00:28]

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

[commit-bot: I haz the power, 2016-11-17 10:00:28]

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

[leonhsl(Using Gerrit), 2016-11-17 10:08:31]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-11-17 10:08:46]

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

[leonhsl(Using Gerrit), 2016-11-17 10:28:02]

leon.han@intel.com changed reviewers:
+ blundell@chromium.org, rockot@chromium.org

[leonhsl(Using Gerrit), 2016-11-17 10:28:03]

PTAL with my attached questions :) Thanks.

https://codereview.chromium.org/2510033002/diff/1/content/browser/service_man...
File content/browser/service_manager/service_manager_context.cc (right):

https://codereview.chromium.org/2510033002/diff/1/content/browser/service_man...
content/browser/service_manager/service_manager_context.cc:260: // Serves
processes other than renderer, such as gpu, plugin, utility etc.
Maybe we should let the device service here also serve renderer process? We can
remove those browser context specific device services, then let renderer process
also connect to this global one.

https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
File content/child/child_thread_impl.cc (right):

https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
content/child/child_thread_impl.cc:546: if (service_manager_connection_) {
Should we use DCHECK(service_manager_connection_) here? I'm not sure whether
child processes are all acting as service or not.

[commit-bot: I haz the power, 2016-11-17 11:04:27]

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

[commit-bot: I haz the power, 2016-11-17 11:04:28]

Dry run: This issue passed the CQ dry run.

[blundell, 2016-11-18 17:16:35]

Thanks! My apologies, I won't be able to review this until Tuesday.

[blundell, 2016-11-22 16:46:44]

Looks good, thanks!

https://codereview.chromium.org/2510033002/diff/1/content/browser/service_man...
File content/browser/service_manager/service_manager_context.cc (right):

https://codereview.chromium.org/2510033002/diff/1/content/browser/service_man...
content/browser/service_manager/service_manager_context.cc:260: // Serves
processes other than renderer, such as gpu, plugin, utility etc.
On 2016/11/17 10:28:03, leonhsl wrote:
> Maybe we should let the device service here also serve renderer process? We
can
> remove those browser context specific device services, then let renderer
process
> also connect to this global one.

Sorry, I didn't understand this question. Could you expand?

https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
File content/child/child_thread_impl.cc (right):

https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
content/child/child_thread_impl.cc:546: if (service_manager_connection_) {
On 2016/11/17 10:28:03, leonhsl wrote:
> Should we use DCHECK(service_manager_connection_) here? I'm not sure whether
> child processes are all acting as service or not.

I'm not sure either. Ken?

[Ken Rockot(use gerrit already), 2016-11-22 17:23:40]

https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
File content/child/child_thread_impl.cc (right):

https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
content/child/child_thread_impl.cc:546: if (service_manager_connection_) {
On 2016/11/22 at 16:46:44, blundell wrote:
> On 2016/11/17 10:28:03, leonhsl wrote:
> > Should we use DCHECK(service_manager_connection_) here? I'm not sure whether
> > child processes are all acting as service or not.
> 
> I'm not sure either. Ken?

There are still a few edge cases where there is no ServiceManagerConnection, so
no, it's not OK to DCHECK here. As a nit though, I would just do

  if (!base::PowerMonitor::Get() && service_manager_connection_) { 
     // ...

[Ken Rockot(use gerrit already), 2016-11-22 17:37:20]

On 2016/11/22 at 16:46:44, blundell wrote:
> Looks good, thanks!
> 
>
https://codereview.chromium.org/2510033002/diff/1/content/browser/service_man...
> File content/browser/service_manager/service_manager_context.cc (right):
> 
>
https://codereview.chromium.org/2510033002/diff/1/content/browser/service_man...
> content/browser/service_manager/service_manager_context.cc:260: // Serves
processes other than renderer, such as gpu, plugin, utility etc.
> On 2016/11/17 10:28:03, leonhsl wrote:
> > Maybe we should let the device service here also serve renderer process? We
can
> > remove those browser context specific device services, then let renderer
process
> > also connect to this global one.

It doesn't work that way. A render process runs as a service with a
BrowserContext-specific user ID. This means all outgoing connections from a
renderer will be routed to the BrowserContext-specific instance of the target
service. For a service embedded in content_browser (like the device service)
this means the BrowserContext-specific service factory will be used to
instantiate the target service.

I've been thinking about redesigning things so that content_browser is a
singleton service, and embedded services can configure whether they should be
instantiated globally or per-BrowserContext. This would mean having a single
ServiceManagerConnection for the browser process, which is a nice property to
have. It also means having only one place to register services.

On the subject of this CL specifically, and in relation to the above point, I
think it's conceptually quite dubious to have an instance of a service
per-BrowserContext while also having a "global" instance of the same service.

> 
> Sorry, I didn't understand this question. Could you expand?
> 
>
https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
> File content/child/child_thread_impl.cc (right):
> 
>
https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
> content/child/child_thread_impl.cc:546: if (service_manager_connection_) {
> On 2016/11/17 10:28:03, leonhsl wrote:
> > Should we use DCHECK(service_manager_connection_) here? I'm not sure whether
> > child processes are all acting as service or not.
> 
> I'm not sure either. Ken?

[leonhsl(Using Gerrit), 2016-11-23 03:18:16]

On 2016/11/22 17:37:20, Ken Rockot wrote:
> On 2016/11/22 at 16:46:44, blundell wrote:
> > Looks good, thanks!
> > 
> >
>
https://codereview.chromium.org/2510033002/diff/1/content/browser/service_man...
> > File content/browser/service_manager/service_manager_context.cc (right):
> > 
> >
>
https://codereview.chromium.org/2510033002/diff/1/content/browser/service_man...
> > content/browser/service_manager/service_manager_context.cc:260: // Serves
> processes other than renderer, such as gpu, plugin, utility etc.
> > On 2016/11/17 10:28:03, leonhsl wrote:
> > > Maybe we should let the device service here also serve renderer process?
We
> can
> > > remove those browser context specific device services, then let renderer
> process
> > > also connect to this global one.
> 
> It doesn't work that way. A render process runs as a service with a
> BrowserContext-specific user ID. This means all outgoing connections from a
> renderer will be routed to the BrowserContext-specific instance of the target
> service. For a service embedded in content_browser (like the device service)
> this means the BrowserContext-specific service factory will be used to
> instantiate the target service.
> 
> I've been thinking about redesigning things so that content_browser is a
> singleton service, and embedded services can configure whether they should be
> instantiated globally or per-BrowserContext. This would mean having a single
> ServiceManagerConnection for the browser process, which is a nice property to
> have. It also means having only one place to register services.
> 
> On the subject of this CL specifically, and in relation to the above point, I
> think it's conceptually quite dubious to have an instance of a service
> per-BrowserContext while also having a "global" instance of the same service.
> 

Yeah I do think that redesigning content_browser as singleton service is an
ideal solution for all.
I also agree this CL's situation is dubious for now,, 
but I found that if we set the connect parameter user id as
'service_manager::mojom::kRootUserID',
content_renderer service CAN also connect to the global content_browser
instance.
Can we take such way as a correct practice? Actually I'm not sure.. 
If it's OK we can avoid registering device service per-BrowserContext, but just
keep this global one.

> > 
> > Sorry, I didn't understand this question. Could you expand?
> > 
> >
>
https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
> > File content/child/child_thread_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
> > content/child/child_thread_impl.cc:546: if (service_manager_connection_) {
> > On 2016/11/17 10:28:03, leonhsl wrote:
> > > Should we use DCHECK(service_manager_connection_) here? I'm not sure
whether
> > > child processes are all acting as service or not.
> > 
> > I'm not sure either. Ken?

[leonhsl(Using Gerrit), 2016-11-28 13:01:09]

Friendly ping Ken, Thanks.

[Ken Rockot(use gerrit already), 2016-11-28 18:24:01]

Is this work currently blocking anything else?

I'd like to hold off on this until we have a better story for the identities of
content_browser and the device service. I suspect that what we really want is
for the device service to be treated as a singleton service, in which case it
doesn't matter what the identity of the connecting user is.

It is however nonsensical to embed a singleton service within a non-singleton
service like content_browser.

On 2016/11/23 at 03:18:16, leon.han wrote:
> On 2016/11/22 17:37:20, Ken Rockot wrote:
> > On 2016/11/22 at 16:46:44, blundell wrote:
> > > Looks good, thanks!
> > > 
> > >
> >
https://codereview.chromium.org/2510033002/diff/1/content/browser/service_man...
> > > File content/browser/service_manager/service_manager_context.cc (right):
> > > 
> > >
> >
https://codereview.chromium.org/2510033002/diff/1/content/browser/service_man...
> > > content/browser/service_manager/service_manager_context.cc:260: // Serves
> > processes other than renderer, such as gpu, plugin, utility etc.
> > > On 2016/11/17 10:28:03, leonhsl wrote:
> > > > Maybe we should let the device service here also serve renderer process?
We
> > can
> > > > remove those browser context specific device services, then let renderer
> > process
> > > > also connect to this global one.
> > 
> > It doesn't work that way. A render process runs as a service with a
> > BrowserContext-specific user ID. This means all outgoing connections from a
> > renderer will be routed to the BrowserContext-specific instance of the
target
> > service. For a service embedded in content_browser (like the device service)
> > this means the BrowserContext-specific service factory will be used to
> > instantiate the target service.
> > 
> > I've been thinking about redesigning things so that content_browser is a
> > singleton service, and embedded services can configure whether they should
be
> > instantiated globally or per-BrowserContext. This would mean having a single
> > ServiceManagerConnection for the browser process, which is a nice property
to
> > have. It also means having only one place to register services.
> > 
> > On the subject of this CL specifically, and in relation to the above point,
I
> > think it's conceptually quite dubious to have an instance of a service
> > per-BrowserContext while also having a "global" instance of the same
service.
> > 
> 
> Yeah I do think that redesigning content_browser as singleton service is an
ideal solution for all.
> I also agree this CL's situation is dubious for now,, 
> but I found that if we set the connect parameter user id as
'service_manager::mojom::kRootUserID',
> content_renderer service CAN also connect to the global content_browser
instance.
> Can we take such way as a correct practice? Actually I'm not sure.. 

We can, but I don't think we should.

> If it's OK we can avoid registering device service per-BrowserContext, but
just keep this global one.
> 
> > > 
> > > Sorry, I didn't understand this question. Could you expand?
> > > 
> > >
> >
https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
> > > File content/child/child_thread_impl.cc (right):
> > > 
> > >
> >
https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
> > > content/child/child_thread_impl.cc:546: if (service_manager_connection_) {
> > > On 2016/11/17 10:28:03, leonhsl wrote:
> > > > Should we use DCHECK(service_manager_connection_) here? I'm not sure
whether
> > > > child processes are all acting as service or not.
> > > 
> > > I'm not sure either. Ken?

[blundell, 2016-11-29 15:47:06]

On 2016/11/28 18:24:01, Ken Rockot wrote:
> Is this work currently blocking anything else?

Nope.

> 
> I'd like to hold off on this until we have a better story for the identities
of
> content_browser and the device service. I suspect that what we really want is
> for the device service to be treated as a singleton service, in which case it
> doesn't matter what the identity of the connecting user is.
> 
> It is however nonsensical to embed a singleton service within a non-singleton
> service like content_browser.

Hi Ken,

Am I understanding correctly that this issue will hit any service that needs to
be connected to from both the renderer process and other child processes, since
services that get connected to from the renderer are currently all per-user
instances? I'd be willing to put in cycles now-ish to change that situation if
so, since it seems like a generally useful problem to fix.

As an aside, I also wouldn't be that surprised if we did eventually end up with
a service that had a use case for both a per-user and a global instance. Is a
useful analogue here something like URLRequestContext, of which there is a
per-Profile instance as well as a global instance representing the system
request context? (I acknowledge that I'm speaking hypothetically here however,
as Power Monitor clearly doesn't itself present that use case.)


> 
> On 2016/11/23 at 03:18:16, leon.han wrote:
> > On 2016/11/22 17:37:20, Ken Rockot wrote:
> > > On 2016/11/22 at 16:46:44, blundell wrote:
> > > > Looks good, thanks!
> > > > 
> > > >
> > >
>
https://codereview.chromium.org/2510033002/diff/1/content/browser/service_man...
> > > > File content/browser/service_manager/service_manager_context.cc (right):
> > > > 
> > > >
> > >
>
https://codereview.chromium.org/2510033002/diff/1/content/browser/service_man...
> > > > content/browser/service_manager/service_manager_context.cc:260: //
Serves
> > > processes other than renderer, such as gpu, plugin, utility etc.
> > > > On 2016/11/17 10:28:03, leonhsl wrote:
> > > > > Maybe we should let the device service here also serve renderer
process?
> We
> > > can
> > > > > remove those browser context specific device services, then let
renderer
> > > process
> > > > > also connect to this global one.
> > > 
> > > It doesn't work that way. A render process runs as a service with a
> > > BrowserContext-specific user ID. This means all outgoing connections from
a
> > > renderer will be routed to the BrowserContext-specific instance of the
> target
> > > service. For a service embedded in content_browser (like the device
service)
> > > this means the BrowserContext-specific service factory will be used to
> > > instantiate the target service.
> > > 
> > > I've been thinking about redesigning things so that content_browser is a
> > > singleton service, and embedded services can configure whether they should
> be
> > > instantiated globally or per-BrowserContext. This would mean having a
single
> > > ServiceManagerConnection for the browser process, which is a nice property
> to
> > > have. It also means having only one place to register services.
> > > 
> > > On the subject of this CL specifically, and in relation to the above
point,
> I
> > > think it's conceptually quite dubious to have an instance of a service
> > > per-BrowserContext while also having a "global" instance of the same
> service.
> > > 
> > 
> > Yeah I do think that redesigning content_browser as singleton service is an
> ideal solution for all.
> > I also agree this CL's situation is dubious for now,, 
> > but I found that if we set the connect parameter user id as
> 'service_manager::mojom::kRootUserID',
> > content_renderer service CAN also connect to the global content_browser
> instance.
> > Can we take such way as a correct practice? Actually I'm not sure.. 
> 
> We can, but I don't think we should.
> 
> > If it's OK we can avoid registering device service per-BrowserContext, but
> just keep this global one.
> > 
> > > > 
> > > > Sorry, I didn't understand this question. Could you expand?
> > > > 
> > > >
> > >
>
https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
> > > > File content/child/child_thread_impl.cc (right):
> > > > 
> > > >
> > >
>
https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
> > > > content/child/child_thread_impl.cc:546: if (service_manager_connection_)
{
> > > > On 2016/11/17 10:28:03, leonhsl wrote:
> > > > > Should we use DCHECK(service_manager_connection_) here? I'm not sure
> whether
> > > > > child processes are all acting as service or not.
> > > > 
> > > > I'm not sure either. Ken?

[Ken Rockot(use gerrit already), 2016-11-29 18:44:29]

On 2016/11/29 at 15:47:06, blundell wrote:
> On 2016/11/28 18:24:01, Ken Rockot wrote:
> > Is this work currently blocking anything else?
> 
> Nope.
> 
> > 
> > I'd like to hold off on this until we have a better story for the identities
of
> > content_browser and the device service. I suspect that what we really want
is
> > for the device service to be treated as a singleton service, in which case
it
> > doesn't matter what the identity of the connecting user is.
> > 
> > It is however nonsensical to embed a singleton service within a
non-singleton
> > service like content_browser.
> 
> Hi Ken,
> 
> Am I understanding correctly that this issue will hit any service that needs
to be connected to from both the renderer process and other child processes,
since services that get connected to from the renderer are currently all
per-user instances? I'd be willing to put in cycles now-ish to change that
situation if so, since it seems like a generally useful problem to fix.

Yes, but I think I need to clarify that *all* services currently embedded in
content are per-user instances, even the ones registered against the global
browser process identity. "Registered on the browser-process-wide
ServiceFactory" should not be conflated with "is a singleton." The former is a
temporary detail of content. In the limit (and at the ServiceManager layer
already) there should be no such thing as a global user ID other than "root",
but we should not use root to mean "singleton" either.

A singleton service is one with the all_users capability. As such, all
connections targeting it are routed to the same instance by the ServiceManager
regardless of the connecting user ID.

I think we should be able to relegate content_browser identity to the
process-wide ServiceManagerConnection and register it as a proper singleton
service. This can package other singleton services like device.

We can create a new identity for packaging per-profile services (strawman name
"content_user_services"), and this can be the service associated with each
BrowserContext.

> 
> As an aside, I also wouldn't be that surprised if we did eventually end up
with a service that had a use case for both a per-user and a global instance. Is
a useful analogue here something like URLRequestContext, of which there is a
per-Profile instance as well as a global instance representing the system
request context? (I acknowledge that I'm speaking hypothetically here however,
as Power Monitor clearly doesn't itself present that use case.)

Given the above explanation - namely that there is no such thing as a global
user - this use case is nonsensical. A service is either a singleton or it is
not a singleton. It's impossible to have both without ambiguity at connection
time (i.e. which instance do you want?)

I would enthusiastically support and review any work to address this issue. Keep
in mind that it's complicated by the existence of manifest overlays for content
services, which are another thing I think we can and should abandon (in favor of
making it easy for content embedders to define and connect their own embedded
service factories.)

Doing the latter bit is a substantial amount of work, but it would reduce the
total long-term effort spent refactoring service identities. Apart from that I
don't have a strong preference for which happens first.

> 
> 
> > 
> > On 2016/11/23 at 03:18:16, leon.han wrote:
> > > On 2016/11/22 17:37:20, Ken Rockot wrote:
> > > > On 2016/11/22 at 16:46:44, blundell wrote:
> > > > > Looks good, thanks!
> > > > > 
> > > > >
> > > >
> >
https://codereview.chromium.org/2510033002/diff/1/content/browser/service_man...
> > > > > File content/browser/service_manager/service_manager_context.cc
(right):
> > > > > 
> > > > >
> > > >
> >
https://codereview.chromium.org/2510033002/diff/1/content/browser/service_man...
> > > > > content/browser/service_manager/service_manager_context.cc:260: //
Serves
> > > > processes other than renderer, such as gpu, plugin, utility etc.
> > > > > On 2016/11/17 10:28:03, leonhsl wrote:
> > > > > > Maybe we should let the device service here also serve renderer
process?
> > We
> > > > can
> > > > > > remove those browser context specific device services, then let
renderer
> > > > process
> > > > > > also connect to this global one.
> > > > 
> > > > It doesn't work that way. A render process runs as a service with a
> > > > BrowserContext-specific user ID. This means all outgoing connections
from a
> > > > renderer will be routed to the BrowserContext-specific instance of the
> > target
> > > > service. For a service embedded in content_browser (like the device
service)
> > > > this means the BrowserContext-specific service factory will be used to
> > > > instantiate the target service.
> > > > 
> > > > I've been thinking about redesigning things so that content_browser is a
> > > > singleton service, and embedded services can configure whether they
should
> > be
> > > > instantiated globally or per-BrowserContext. This would mean having a
single
> > > > ServiceManagerConnection for the browser process, which is a nice
property
> > to
> > > > have. It also means having only one place to register services.
> > > > 
> > > > On the subject of this CL specifically, and in relation to the above
point,
> > I
> > > > think it's conceptually quite dubious to have an instance of a service
> > > > per-BrowserContext while also having a "global" instance of the same
> > service.
> > > > 
> > > 
> > > Yeah I do think that redesigning content_browser as singleton service is
an
> > ideal solution for all.
> > > I also agree this CL's situation is dubious for now,, 
> > > but I found that if we set the connect parameter user id as
> > 'service_manager::mojom::kRootUserID',
> > > content_renderer service CAN also connect to the global content_browser
> > instance.
> > > Can we take such way as a correct practice? Actually I'm not sure.. 
> > 
> > We can, but I don't think we should.
> > 
> > > If it's OK we can avoid registering device service per-BrowserContext, but
> > just keep this global one.
> > > 
> > > > > 
> > > > > Sorry, I didn't understand this question. Could you expand?
> > > > > 
> > > > >
> > > >
> >
https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
> > > > > File content/child/child_thread_impl.cc (right):
> > > > > 
> > > > >
> > > >
> >
https://codereview.chromium.org/2510033002/diff/1/content/child/child_thread_...
> > > > > content/child/child_thread_impl.cc:546: if
(service_manager_connection_) {
> > > > > On 2016/11/17 10:28:03, leonhsl wrote:
> > > > > > Should we use DCHECK(service_manager_connection_) here? I'm not sure
> > whether
> > > > > > child processes are all acting as service or not.
> > > > > 
> > > > > I'm not sure either. Ken?

[leonhsl(Using Gerrit), 2016-11-30 03:27:20]

Hi Ken,
Suppose we have had content_browser as singleton service and
content_user_services as per-profile service, and suppose one Service(For
example Device Service) needs to access something BrowserContext-specific, and
it serves for both render processes and child processes, then how should we
position this Service? Seems it should not be put into content_browser because
it needs something BrowserContext-specific, and if we put it into
content_user_services then how could child processes connect to it?

[Ken Rockot(use gerrit already), 2016-11-30 03:54:24]

On Tue, Nov 29, 2016 at 7:27 PM, <leon.han@intel.com> wrote:

> Hi Ken,
> Suppose we have had content_browser as singleton service and
> content_user_services as per-profile service, and suppose one Service(For
> example Device Service) needs to access something BrowserContext-specific,
> and
> it serves for both render processes and child processes, then how should we
> position this Service? Seems it should not be put into content_browser
> because
> it needs something BrowserContext-specific, and if we put it into
> content_user_services then how could child processes connect to it?
>

It would be helpful to have a concrete example of this before trying to
decide whether it makes sense. In the limit we could just assume every
service needs a BrowserContext, but I don't think that's true.


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

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

[leonhsl(Using Gerrit), 2016-11-30 04:03:50]

On 2016/11/30 03:54:24, Ken Rockot wrote:
> On Tue, Nov 29, 2016 at 7:27 PM, <mailto:leon.han@intel.com> wrote:
> 
> > Hi Ken,
> > Suppose we have had content_browser as singleton service and
> > content_user_services as per-profile service, and suppose one Service(For
> > example Device Service) needs to access something BrowserContext-specific,
> > and
> > it serves for both render processes and child processes, then how should we
> > position this Service? Seems it should not be put into content_browser
> > because
> > it needs something BrowserContext-specific, and if we put it into
> > content_user_services then how could child processes connect to it?
> >
> 
> It would be helpful to have a concrete example of this before trying to
> decide whether it makes sense. In the limit we could just assume every
> service needs a BrowserContext, but I don't think that's true.
> 

Actually I'm considering Device Service would fit such case, because it will
embed multiple 'Device' features, some features(such as PowerMonitor here) has
no dependency on specific BrowserContext but seems other features(NFC and
Geolocation etc.) have such needs.. Meanwhile, as a Service, Device Service
should be able to be connected from both render processes and child processes.

[Ken Rockot(use gerrit already), 2016-11-30 04:11:13]

I think the dependencies on BrowserContext for NFC and Geolocation etc are
details of web platform support, which in my opinion is not a concern of
the device service, but some higher level service(s). The device service
should provide relatively low-level interfaces for acquiring device
resources.

On Tue, Nov 29, 2016 at 8:03 PM, <leon.han@intel.com> wrote:

> On 2016/11/30 03:54:24, Ken Rockot wrote:
> > On Tue, Nov 29, 2016 at 7:27 PM, <mailto:leon.han@intel.com> wrote:
> >
> > > Hi Ken,
> > > Suppose we have had content_browser as singleton service and
> > > content_user_services as per-profile service, and suppose one
> Service(For
> > > example Device Service) needs to access something
> BrowserContext-specific,
> > > and
> > > it serves for both render processes and child processes, then how
> should we
> > > position this Service? Seems it should not be put into content_browser
> > > because
> > > it needs something BrowserContext-specific, and if we put it into
> > > content_user_services then how could child processes connect to it?
> > >
> >
> > It would be helpful to have a concrete example of this before trying to
> > decide whether it makes sense. In the limit we could just assume every
> > service needs a BrowserContext, but I don't think that's true.
> >
>
> Actually I'm considering Device Service would fit such case, because it
> will
> embed multiple 'Device' features, some features(such as PowerMonitor here)
> has
> no dependency on specific BrowserContext but seems other features(NFC and
> Geolocation etc.) have such needs.. Meanwhile, as a Service, Device Service
> should be able to be connected from both render processes and child
> processes.
>
> https://codereview.chromium.org/2510033002/
>

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

[blundell, 2016-11-30 09:50:12]

Thanks for all this information, Ken! I'm tied up today, but I'll process it
tomorrow.

On 2016/11/30 04:11:13, Ken Rockot wrote:
> I think the dependencies on BrowserContext for NFC and Geolocation etc are
> details of web platform support, which in my opinion is not a concern of
> the device service, but some higher level service(s). The device service
> should provide relatively low-level interfaces for acquiring device
> resources.
> 
> On Tue, Nov 29, 2016 at 8:03 PM, <mailto:leon.han@intel.com> wrote:
> 
> > On 2016/11/30 03:54:24, Ken Rockot wrote:
> > > On Tue, Nov 29, 2016 at 7:27 PM, <mailto:leon.han@intel.com> wrote:
> > >
> > > > Hi Ken,
> > > > Suppose we have had content_browser as singleton service and
> > > > content_user_services as per-profile service, and suppose one
> > Service(For
> > > > example Device Service) needs to access something
> > BrowserContext-specific,
> > > > and
> > > > it serves for both render processes and child processes, then how
> > should we
> > > > position this Service? Seems it should not be put into content_browser
> > > > because
> > > > it needs something BrowserContext-specific, and if we put it into
> > > > content_user_services then how could child processes connect to it?
> > > >
> > >
> > > It would be helpful to have a concrete example of this before trying to
> > > decide whether it makes sense. In the limit we could just assume every
> > > service needs a BrowserContext, but I don't think that's true.
> > >
> >
> > Actually I'm considering Device Service would fit such case, because it
> > will
> > embed multiple 'Device' features, some features(such as PowerMonitor here)
> > has
> > no dependency on specific BrowserContext but seems other features(NFC and
> > Geolocation etc.) have such needs.. Meanwhile, as a Service, Device Service
> > should be able to be connected from both render processes and child
> > processes.
> >
> > https://codereview.chromium.org/2510033002/
> >
> 
> -- 
> 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.

[blundell, 2016-12-30 13:28:54]

Hi Han Leon,

Per offline discussion with Ken, it should now "just work" to embed a singleton
service in //content/browser as a result of recent changes. So you should be
able to proceed with this CL now by changing the Device Service to be a
singleton service (e.g.,
https://cs.chromium.org/chromium/src/services/tracing/manifest.json?q=tracing...).

[leonhsl(Using Gerrit), 2016-12-31 09:36:53]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-31 09:36:58]

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-12-31 11:33:40]

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

[commit-bot: I haz the power, 2016-12-31 11:33:41]

Dry run: This issue passed the CQ dry run.

[leonhsl(Using Gerrit), 2016-12-31 13:09:06]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-31 13:09:14]

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

[leonhsl(Using Gerrit), 2016-12-31 13:43:14]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-31 13:43:25]

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

[leonhsl(Using Gerrit), 2016-12-31 13:45:49]

Patchset #4 (id:60001) has been deleted

[leonhsl(Using Gerrit), 2016-12-31 13:49:50]

Description was changed from

==========
[Device Service] Move PowerMonitor into the Device Service

This CL moves PowerMonitor to being hosted by the Device Service.

BUG=647247
==========

to

==========
[Device Service] Move PowerMonitor into the Device Service

This CL
  - configures Device Service to be singleton service,
  - moves PowerMonitor to being hosted by the Device Service.

BUG=647247
==========

[leonhsl(Using Gerrit), 2016-12-31 14:01:13]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2016-12-31 14:01:24]

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

[leonhsl(Using Gerrit), 2016-12-31 14:01:52]

Patchset #4 (id:80001) has been deleted

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

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

[commit-bot: I haz the power, 2016-12-31 15:05:09]

Dry run: This issue passed the CQ dry run.

[leonhsl(Using Gerrit), 2017-01-01 08:28:33]

Uploaded ps#4, PTAnL, Thanks.
  # Locally confirmed device service can work well as singleton.

[blundell, 2017-01-03 15:20:50]

Thanks!

https://codereview.chromium.org/2510033002/diff/100001/content/browser/browse...
File content/browser/browser_context.cc (right):

https://codereview.chromium.org/2510033002/diff/100001/content/browser/browse...
content/browser/browser_context.cc:466: base::Bind(&device::CreateDeviceService,
Does the Device Service not still need to be registered with
ServiceManagerContext for child processes other than the renderer to be able to
connect to it?

https://codereview.chromium.org/2510033002/diff/100001/content/child/child_th...
File content/child/child_thread_impl.cc (right):

https://codereview.chromium.org/2510033002/diff/100001/content/child/child_th...
content/child/child_thread_impl.cc:521:
device_connection->GetRemoteInterfaces();
nit: This could just be inlined into line 524 below.

https://codereview.chromium.org/2510033002/diff/100001/services/device/device...
File services/device/device_service.cc (right):

https://codereview.chromium.org/2510033002/diff/100001/services/device/device...
services/device/device_service.cc:20: static bool g_service_instance_alive =
false;
I wouldn't worry about this -- this is essentially just verifying that the
singleton mechanism is working correctly, which seems more the role of e.g.
service manager integration tests.

[Ken Rockot(use gerrit already), 2017-01-03 20:25:15]

https://codereview.chromium.org/2510033002/diff/100001/content/browser/browse...
File content/browser/browser_context.cc (right):

https://codereview.chromium.org/2510033002/diff/100001/content/browser/browse...
content/browser/browser_context.cc:466: base::Bind(&device::CreateDeviceService,
On 2017/01/03 at 15:20:50, blundell wrote:
> Does the Device Service not still need to be registered with
ServiceManagerContext for child processes other than the renderer to be able to
connect to it?

The answer to your question is yes, but there's another subtle issue this makes
me realize. If a renderer happens to be the first thing to connect to the device
service, then the singleton instance lifetime will be bound to this
BrowserContext via ServiceManagerConnectionImpl. This could lead to service
flakiness if that profile is then destroyed some time later.

I think the right thing to do is completely remove this registration from
BrowserContext, add it to ServiceManagerContext instead; and then also have
ServiceManagerContext initiate and maintain a connection to the device service
on startup. For now, this will ensure that a singleton instance already exists
by the time any real client connects to it.

Once we have a better way to package the device service we can kill the early
connection to it.

[leonhsl(Using Gerrit), 2017-01-04 04:48:27]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-04 04:48:38]

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

[leonhsl(Using Gerrit), 2017-01-04 04:56:28]

https://codereview.chromium.org/2510033002/diff/100001/content/child/child_th...
File content/child/child_thread_impl.cc (right):

https://codereview.chromium.org/2510033002/diff/100001/content/child/child_th...
content/child/child_thread_impl.cc:521:
device_connection->GetRemoteInterfaces();
On 2017/01/03 15:20:50, blundell wrote:
> nit: This could just be inlined into line 524 below.

Done.

https://codereview.chromium.org/2510033002/diff/100001/services/device/device...
File services/device/device_service.cc (right):

https://codereview.chromium.org/2510033002/diff/100001/services/device/device...
services/device/device_service.cc:20: static bool g_service_instance_alive =
false;
On 2017/01/03 15:20:50, blundell wrote:
> I wouldn't worry about this -- this is essentially just verifying that the
> singleton mechanism is working correctly, which seems more the role of e.g.
> service manager integration tests.

Done.

[commit-bot: I haz the power, 2017-01-04 05:56:55]

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

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

Dry run: This issue passed the CQ dry run.

[leonhsl(Using Gerrit), 2017-01-04 07:28:31]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-04 07:28:52]

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

[leonhsl(Using Gerrit), 2017-01-04 07:34:11]

https://codereview.chromium.org/2510033002/diff/100001/content/browser/browse...
File content/browser/browser_context.cc (right):

https://codereview.chromium.org/2510033002/diff/100001/content/browser/browse...
content/browser/browser_context.cc:466: base::Bind(&device::CreateDeviceService,
On 2017/01/03 20:25:15, Ken Rockot wrote:
> On 2017/01/03 at 15:20:50, blundell wrote:
> > Does the Device Service not still need to be registered with
> ServiceManagerContext for child processes other than the renderer to be able
to
> connect to it?
> 
> The answer to your question is yes, but there's another subtle issue this
makes
> me realize. If a renderer happens to be the first thing to connect to the
device
> service, then the singleton instance lifetime will be bound to this
> BrowserContext via ServiceManagerConnectionImpl. This could lead to service
> flakiness if that profile is then destroyed some time later.
> 
> I think the right thing to do is completely remove this registration from
> BrowserContext, add it to ServiceManagerContext instead; and then also have
> ServiceManagerContext initiate and maintain a connection to the device service
> on startup. For now, this will ensure that a singleton instance already exists
> by the time any real client connects to it.
> 
> Once we have a better way to package the device service we can kill the early
> connection to it.

Done and thanks a lot for the kindly explanations!

[commit-bot: I haz the power, 2017-01-04 08:20:19]

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

[commit-bot: I haz the power, 2017-01-04 08:20:20]

Dry run: This issue passed the CQ dry run.

[blundell, 2017-01-05 16:27:08]

Musing for the future re: "a better way to package the device service":

If we implemented your suggestion of having content_browser be a singleton and
the per-Profile version be a different service entirely ("user_services" or
whatnot as you suggested), that should do the trick nicely, right? It looks like
ServiceManager itself isn't currently written to handle the concept of singleton
services that are also service factories, but that seems like it would be a
minor addition. Then any initial connecting client would unambiguously find the
singleton service factory.

https://codereview.chromium.org/2510033002/diff/140001/content/browser/servic...
File content/browser/service_manager/service_manager_context.cc (right):

https://codereview.chromium.org/2510033002/diff/140001/content/browser/servic...
content/browser/service_manager/service_manager_context.cc:331:
std::unique_ptr<service_manager::Connection> device_connection_ =
You're aliasing the ivar here. This brings up a different but related point:
Ken, I think that just making the connection should be enough, right? The Device
Service doesn't quit when its last connection has gone away.

[Ken Rockot(use gerrit already), 2017-01-05 17:14:09]

On Thu, Jan 5, 2017 at 8:27 AM, <blundell@chromium.org> wrote:

> Musing for the future re: "a better way to package the device service":
>
> If we implemented your suggestion of having content_browser be a singleton
> and
> the per-Profile version be a different service entirely ("user_services" or
> whatnot as you suggested), that should do the trick nicely, right? It
> looks like
> ServiceManager itself isn't currently written to handle the concept of
> singleton
> services that are also service factories, but that seems like it would be a
> minor addition. Then any initial connecting client would unambiguously
> find the
> singleton service factory.
>

Yeah, I realized this is still necessary (for this and other patches) and
I'll be doing it today.


>
>
> https://codereview.chromium.org/2510033002/diff/140001/
> content/browser/service_manager/service_manager_context.cc
> File content/browser/service_manager/service_manager_context.cc (right):
>
> https://codereview.chromium.org/2510033002/diff/140001/
> content/browser/service_manager/service_manager_context.cc#newcode331
> content/browser/service_manager/service_manager_context.cc:331:
> std::unique_ptr<service_manager::Connection> device_connection_ =
> You're aliasing the ivar here. This brings up a different but related
> point: Ken, I think that just making the connection should be enough,
> right? The Device Service doesn't quit when its last connection has gone
> away.
>

Yeah, that's correct.


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

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

[leonhsl(Using Gerrit), 2017-01-06 03:25:35]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 03:25:52]

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-06 04:34:12]

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

[commit-bot: I haz the power, 2017-01-06 04:34:13]

Dry run: This issue passed the CQ dry run.

[leonhsl(Using Gerrit), 2017-01-06 05:26:03]

Uploaded ps#7, PTAnL, thanks.

https://codereview.chromium.org/2510033002/diff/140001/content/browser/servic...
File content/browser/service_manager/service_manager_context.cc (right):

https://codereview.chromium.org/2510033002/diff/140001/content/browser/servic...
content/browser/service_manager/service_manager_context.cc:331:
std::unique_ptr<service_manager::Connection> device_connection_ =
On 2017/01/05 16:27:08, blundell wrote:
> You're aliasing the ivar here. This brings up a different but related point:
> Ken, I think that just making the connection should be enough, right? The
Device
> Service doesn't quit when its last connection has gone away.

Ah.. Done and thanks!

[blundell, 2017-01-06 12:28:09]

lgtm, thanks!

https://codereview.chromium.org/2510033002/diff/160001/content/browser/servic...
File content/browser/service_manager/service_manager_context.cc (right):

https://codereview.chromium.org/2510033002/diff/160001/content/browser/servic...
content/browser/service_manager/service_manager_context.cc:280: // for now.
I would eliminate this comment; I don't think it adds too much information and
the Device Service will likely continue to be embedded in the browser process in
the long term.

https://codereview.chromium.org/2510033002/diff/160001/content/browser/servic...
content/browser/service_manager/service_manager_context.cc:331: // instance,
later the same instance will serve all the clients wanting to
Add below this:

TODO(rockot): Eliminate this connection once content_browser is itself a
singleton service.

[leonhsl(Using Gerrit), 2017-01-06 14:14:21]

The CQ bit was checked by leon.han@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 14:14:37]

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

[leonhsl(Using Gerrit), 2017-01-06 14:36:00]

https://codereview.chromium.org/2510033002/diff/160001/content/browser/servic...
File content/browser/service_manager/service_manager_context.cc (right):

https://codereview.chromium.org/2510033002/diff/160001/content/browser/servic...
content/browser/service_manager/service_manager_context.cc:280: // for now.
On 2017/01/06 12:28:09, blundell wrote:
> I would eliminate this comment; I don't think it adds too much information and
> the Device Service will likely continue to be embedded in the browser process
in
> the long term.

Done.

https://codereview.chromium.org/2510033002/diff/160001/content/browser/servic...
content/browser/service_manager/service_manager_context.cc:331: // instance,
later the same instance will serve all the clients wanting to
On 2017/01/06 12:28:09, blundell wrote:
> Add below this:
> 
> TODO(rockot): Eliminate this connection once content_browser is itself a
> singleton service.

OK I added a task to Ken :) Done.

[leonhsl(Using Gerrit), 2017-01-06 14:38:26]

Description was changed from

==========
[Device Service] Move PowerMonitor into the Device Service

This CL
  - configures Device Service to be singleton service,
  - moves PowerMonitor to being hosted by the Device Service.

BUG=647247
==========

to

==========
[Device Service] Move PowerMonitor into the Device Service

This CL
  - configures Device Service to be singleton service.
  - lets content::ServiceManagerContext initiate a connection to Device,
    Service to create the singleton instance which will serve all clients.
  - moves PowerMonitor to being hosted by the Device Service.

BUG=647247
==========

[commit-bot: I haz the power, 2017-01-06 15:25:50]

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

[commit-bot: I haz the power, 2017-01-06 15:25:51]

Dry run: This issue passed the CQ dry run.

[leonhsl(Using Gerrit), 2017-01-09 10:32:10]

leon.han@intel.com changed reviewers:
+ kinuko@chromium.org, tsepez@chromium.org

[leonhsl(Using Gerrit), 2017-01-09 10:32:11]

+kinuko@ : For OWNER review of content/{browser, child}

+tsepez@ : For OWNER review of content/public/app/mojo/*.json

Also ping Ken for overall review :)

[Tom Sepez, 2017-01-09 17:52:50]

OWNERS LGTM

[Ken Rockot(use gerrit already), 2017-01-09 18:12:02]

lgtm

[Ken Rockot(use gerrit already), 2017-01-09 18:15:26]

https://codereview.chromium.org/2510033002/diff/180001/content/browser/servic...
File content/browser/service_manager/service_manager_context.cc (right):

https://codereview.chromium.org/2510033002/diff/180001/content/browser/servic...
content/browser/service_manager/service_manager_context.cc:332: // singleton
service.
Please add http://crbug.com/679407 to this note. Thanks!

[kinuko, 2017-01-10 07:30:52]

content/ lgtm

[leonhsl(Using Gerrit), 2017-01-10 09:23:14]

Thanks all! Will send this to CQ now.

https://codereview.chromium.org/2510033002/diff/180001/content/browser/servic...
File content/browser/service_manager/service_manager_context.cc (right):

https://codereview.chromium.org/2510033002/diff/180001/content/browser/servic...
content/browser/service_manager/service_manager_context.cc:332: // singleton
service.
On 2017/01/09 18:15:26, Ken Rockot wrote:
> Please add http://crbug.com/679407 to this note. Thanks!

Done.

[leonhsl(Using Gerrit), 2017-01-10 09:23:24]

The CQ bit was checked by leon.han@intel.com

[leonhsl(Using Gerrit), 2017-01-10 09:23:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
blundell@chromium.org, kinuko@chromium.org, rockot@chromium.org,
tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/2510033002/#ps200001
(title: "Link TODO with bug")

[commit-bot: I haz the power, 2017-01-10 09:23:47]

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-10 09:25:43]

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

[commit-bot: I haz the power, 2017-01-10 09:25:45]

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

[leonhsl(Using Gerrit), 2017-01-10 09:35:16]

The CQ bit was checked by leon.han@intel.com

[leonhsl(Using Gerrit), 2017-01-10 09:35:17]

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

[commit-bot: I haz the power, 2017-01-10 09:35:28]

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-10 10:43:19]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1484040916654670,
"parent_rev": "cecab1693101adb8c94e3f7f1d1d3675509814be", "commit_rev":
"918f72f14f7840b8e40c2b0ca5f95817f691ebd6"}

[commit-bot: I haz the power, 2017-01-10 10:43:46]

Description was changed from

==========
[Device Service] Move PowerMonitor into the Device Service

This CL
  - configures Device Service to be singleton service.
  - lets content::ServiceManagerContext initiate a connection to Device,
    Service to create the singleton instance which will serve all clients.
  - moves PowerMonitor to being hosted by the Device Service.

BUG=647247
==========

to

==========
[Device Service] Move PowerMonitor into the Device Service

This CL
  - configures Device Service to be singleton service.
  - lets content::ServiceManagerContext initiate a connection to Device,
    Service to create the singleton instance which will serve all clients.
  - moves PowerMonitor to being hosted by the Device Service.

BUG=647247

Review-Url: https://codereview.chromium.org/2510033002
Cr-Commit-Position: refs/heads/master@{#442550}
Committed:
https://chromium.googlesource.com/chromium/src/+/918f72f14f7840b8e40c2b0ca5f9...
==========

[commit-bot: I haz the power, 2017-01-10 10:43:48]

Committed patchset #10 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/918f72f14f7840b8e40c2b0ca5f9...