arc-bridge: Introduce the ArcService class

arc-bridge: Introduce the ArcService class

Several services managed by ARC were introducing empty, opaque classes
just to be able to be constructed outside of components/. This was
becoming ugly, so introduce just one interface that all services must
implement and whose lifecycle will be managed by ArcServiceManager.

BUG=None
TEST=trybots

Committed: https://crrev.com/811769ba67bef5ff1e0a120146c0d593a6ebe5db
Cr-Commit-Position: refs/heads/master@{#370825}

[Luis Héctor Chávez, 2016-01-15 21:41:58]

lhchavez@chromium.org changed reviewers:
+ elijahtaylor@chromium.org, hidehiko@chromium.org, xiyuan@chromium.org

[Luis Héctor Chávez, 2016-01-15 21:41:58]

PTAL

elijahtaylor, hidehiko: components/arc, ui/arc
xiyuan: chrome/browser/chromeos

[hidehiko, 2016-01-18 01:26:45]

https://codereview.chromium.org/1596663002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/arc/arc_auth_service.cc (right):

https://codereview.chromium.org/1596663002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/arc/arc_auth_service.cc:21:
ArcBridgeService::Get()->RemoveObserver(this);
For consistency, could you keep the |arc_bridge| passed to the constructor, and
call its RemoveObserver()?
Ditto for all other classes.

https://codereview.chromium.org/1596663002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/arc/arc_auth_service.h (right):

https://codereview.chromium.org/1596663002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/arc/arc_auth_service.h:1: // Copyright 2015 The Chromium
Authors. All rights reserved.
nit: 2016

https://codereview.chromium.org/1596663002/diff/20001/components/arc/arc_serv...
File components/arc/arc_service.h (right):

https://codereview.chromium.org/1596663002/diff/20001/components/arc/arc_serv...
components/arc/arc_service.h:20: ArcService() {}
Can we move {Add,Remove}Observer(this) to this class, so we do not in inherited
classes?
Also, you may want to move CallValidThread() check function to here, so that we
can share the implementation?

https://codereview.chromium.org/1596663002/diff/20001/components/arc/clipboar...
File components/arc/clipboard/arc_clipboard_bridge.cc (right):

https://codereview.chromium.org/1596663002/diff/20001/components/arc/clipboar...
components/arc/clipboard/arc_clipboard_bridge.cc:32: DCHECK(bridge_service_);
nit: while you're here, could you standardize the name with other classes?

https://codereview.chromium.org/1596663002/diff/20001/components/arc/input/ar...
File components/arc/input/arc_input_bridge.h (right):

https://codereview.chromium.org/1596663002/diff/20001/components/arc/input/ar...
components/arc/input/arc_input_bridge.h:16: #include "ui/aura/env.h"
nit: Looks unnecessary?

https://codereview.chromium.org/1596663002/diff/20001/components/arc/input/ar...
components/arc/input/arc_input_bridge.h:23: class Window;
Can we include ui/aura/window.h directly, as we do for ui/events/event.h ?

[Luis Héctor Chávez, 2016-01-19 17:43:54]

https://codereview.chromium.org/1596663002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/arc/arc_auth_service.cc (right):

https://codereview.chromium.org/1596663002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/arc/arc_auth_service.cc:21:
ArcBridgeService::Get()->RemoveObserver(this);
On 2016/01/18 01:26:44, hidehiko wrote:
> For consistency, could you keep the |arc_bridge| passed to the constructor,
and
> call its RemoveObserver()?
> Ditto for all other classes.

Done.

https://codereview.chromium.org/1596663002/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/arc/arc_auth_service.h (right):

https://codereview.chromium.org/1596663002/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/arc/arc_auth_service.h:1: // Copyright 2015 The Chromium
Authors. All rights reserved.
On 2016/01/18 01:26:45, hidehiko wrote:
> nit: 2016

This is technically a move/rename, but done since I'm not sure how it should be
handled.

https://codereview.chromium.org/1596663002/diff/20001/components/arc/arc_serv...
File components/arc/arc_service.h (right):

https://codereview.chromium.org/1596663002/diff/20001/components/arc/arc_serv...
components/arc/arc_service.h:20: ArcService() {}
On 2016/01/18 01:26:45, hidehiko wrote:
> Can we move {Add,Remove}Observer(this) to this class, so we do not in
inherited
> classes?
> Also, you may want to move CallValidThread() check function to here, so that
we
> can share the implementation?

The problem with moving AddObserver(this) to this class is that ArcService would
need to be an ArcBridgeService::Observer (but would not implement any methods,
which is weird). Also, there is a subclass-specific check that needs to happen
just after adding the observer, which would make it error-prone to omit since it
would work some of the times (most of the interfaces did not follow the best
practices doc and ended up not including it). See what happened with the app
list (crbug.com/575873) for an example of why I'm a bit against doing this.

If I don't add AddObserver to this class, adding RemoveObserver feels
inconsistent.

For CalledOnValidThread, we probably should instead make the subclasses inherit
from NonThreadSafe instead of making ArcService take on responsibilities it
shouldn't. And even that is discouraged in favor of keeping a
ThreadChecker+calling CalledOnValidThread:
https://code.google.com/p/chromium/codesearch#chromium/src/base/threading/thr...

https://codereview.chromium.org/1596663002/diff/20001/components/arc/clipboar...
File components/arc/clipboard/arc_clipboard_bridge.cc (right):

https://codereview.chromium.org/1596663002/diff/20001/components/arc/clipboar...
components/arc/clipboard/arc_clipboard_bridge.cc:32: DCHECK(bridge_service_);
On 2016/01/18 01:26:45, hidehiko wrote:
> nit: while you're here, could you standardize the name with other classes?

Done.

https://codereview.chromium.org/1596663002/diff/20001/components/arc/input/ar...
File components/arc/input/arc_input_bridge.h (right):

https://codereview.chromium.org/1596663002/diff/20001/components/arc/input/ar...
components/arc/input/arc_input_bridge.h:16: #include "ui/aura/env.h"
On 2016/01/18 01:26:45, hidehiko wrote:
> nit: Looks unnecessary?

Done.

https://codereview.chromium.org/1596663002/diff/20001/components/arc/input/ar...
components/arc/input/arc_input_bridge.h:23: class Window;
On 2016/01/18 01:26:45, hidehiko wrote:
> Can we include ui/aura/window.h directly, as we do for ui/events/event.h ?

To reduce compilation time, I'd prefer to remove ui/events/event.h.

[xiyuan, 2016-01-19 18:50:36]

LGTM but please wait for other reviews.

https://codereview.chromium.org/1596663002/diff/40001/components/arc/arc_serv...
File components/arc/arc_service.h (right):

https://codereview.chromium.org/1596663002/diff/40001/components/arc/arc_serv...
components/arc/arc_service.h:12: class ArcBridgeService;
nit: seems not used.

[Luis Héctor Chávez, 2016-01-19 19:17:29]

https://codereview.chromium.org/1596663002/diff/40001/components/arc/arc_serv...
File components/arc/arc_service.h (right):

https://codereview.chromium.org/1596663002/diff/40001/components/arc/arc_serv...
components/arc/arc_service.h:12: class ArcBridgeService;
On 2016/01/19 18:50:36, xiyuan wrote:
> nit: seems not used.

Done.

[elijahtaylor1, 2016-01-20 00:41:19]

mostly clarification questions which might warrant change, defer to hidehiko

https://codereview.chromium.org/1596663002/diff/80001/components/arc/arc_serv...
File components/arc/arc_service.h (right):

https://codereview.chromium.org/1596663002/diff/80001/components/arc/arc_serv...
components/arc/arc_service.h:20: 
all of the children of ArcService hold a weak reference to ArcBridgeService. 
Can that be part of this class instead along with the calls to add/remove
observers?  Or maybe adding the service to the service manager can add the
observer?

https://codereview.chromium.org/1596663002/diff/80001/components/arc/arc_serv...
File components/arc/arc_service_manager.cc (right):

https://codereview.chromium.org/1596663002/diff/80001/components/arc/arc_serv...
components/arc/arc_service_manager.cc:34: AddService(make_scoped_ptr(new
ArcClipboardBridge(arc_bridge_service())));
what is the distinction between services created from outside of this
constructor (chrome_browser_main_chromeos) and the ones created here?

https://codereview.chromium.org/1596663002/diff/80001/components/arc/ime/arc_...
File components/arc/ime/arc_ime_ipc_host.cc (right):

https://codereview.chromium.org/1596663002/diff/80001/components/arc/ime/arc_...
components/arc/ime/arc_ime_ipc_host.cc:83: if
(arc_bridge_service_->ime_instance())
I don't think I understand why this can't be done in AddObserver to avoid this
pattern in many of the sub interfaces

[hidehiko, 2016-01-20 16:04:50]

https://codereview.chromium.org/1596663002/diff/20001/components/arc/arc_serv...
File components/arc/arc_service.h (right):

https://codereview.chromium.org/1596663002/diff/20001/components/arc/arc_serv...
components/arc/arc_service.h:20: ArcService() {}
On 2016/01/19 17:43:54, Luis Héctor Chávez wrote:
> On 2016/01/18 01:26:45, hidehiko wrote:
> > Can we move {Add,Remove}Observer(this) to this class, so we do not in
> inherited
> > classes?
> > Also, you may want to move CallValidThread() check function to here, so that
> we
> > can share the implementation?
> 
> The problem with moving AddObserver(this) to this class is that ArcService
would
> need to be an ArcBridgeService::Observer (but would not implement any methods,
> which is weird). Also, there is a subclass-specific check that needs to happen
> just after adding the observer, which would make it error-prone to omit since
it
> would work some of the times (most of the interfaces did not follow the best
> practices doc and ended up not including it). See what happened with the app
> list (crbug.com/575873) for an example of why I'm a bit against doing this.
> 
> If I don't add AddObserver to this class, adding RemoveObserver feels
> inconsistent.
> 
> For CalledOnValidThread, we probably should instead make the subclasses
inherit
> from NonThreadSafe instead of making ArcService take on responsibilities it
> shouldn't. And even that is discouraged in favor of keeping a
> ThreadChecker+calling CalledOnValidThread:
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/threading/thr...

My understanding is that the goal of this CL is to de-dup the implementation
boilerplate. Then, using skeleton implementation (what I'm suggesting) can
reduce
the code more than a marker interface (the current code), IMHO.

For a skeleton implementation, inheriting ArcBridgeService::Observer looks
straightforward. Invoking OnSomethingReady() method if necessary in a
constructor
followed by AddObserver(this) does not sound problematic, to me. (You can call
all the necessary callback then).

With the implementation, subclasses would not have a problem like
crbug.com/575873.
(Of course, it is worth to be documented anyway).

CalledOnValidThread related stuff is optional actually, but IMHO, again assuming
we move to skeleton implementation concept, it looks clearer if we have the
function
in this class with clear documentation why and when the thread check is needed.

Bonus, if you have a ptr to ArcBridgeService, you can provide a simple getter
with clear document that the object pointing by the pointer is alive while the
instance is alive.

WDYT?

[Luis Héctor Chávez, 2016-01-20 17:34:27]

On 2016/01/20 16:04:50, hidehiko wrote:
>
https://codereview.chromium.org/1596663002/diff/20001/components/arc/arc_serv...
> File components/arc/arc_service.h (right):
> 
>
https://codereview.chromium.org/1596663002/diff/20001/components/arc/arc_serv...
> components/arc/arc_service.h:20: ArcService() {}
> On 2016/01/19 17:43:54, Luis Héctor Chávez wrote:
> > On 2016/01/18 01:26:45, hidehiko wrote:
> > > Can we move {Add,Remove}Observer(this) to this class, so we do not in
> > inherited
> > > classes?
> > > Also, you may want to move CallValidThread() check function to here, so
that
> > we
> > > can share the implementation?
> > 
> > The problem with moving AddObserver(this) to this class is that ArcService
> would
> > need to be an ArcBridgeService::Observer (but would not implement any
methods,
> > which is weird). Also, there is a subclass-specific check that needs to
happen
> > just after adding the observer, which would make it error-prone to omit
since
> it
> > would work some of the times (most of the interfaces did not follow the best
> > practices doc and ended up not including it). See what happened with the app
> > list (crbug.com/575873) for an example of why I'm a bit against doing this.
> > 
> > If I don't add AddObserver to this class, adding RemoveObserver feels
> > inconsistent.
> > 
> > For CalledOnValidThread, we probably should instead make the subclasses
> inherit
> > from NonThreadSafe instead of making ArcService take on responsibilities it
> > shouldn't. And even that is discouraged in favor of keeping a
> > ThreadChecker+calling CalledOnValidThread:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/threading/thr...
> 
> My understanding is that the goal of this CL is to de-dup the implementation
> boilerplate.

The real goal is to stop people from reinventing the wheel (add an identical yet
non-reusable interface) every time they want to add something. Any boilerplate
reduction is a side-effect, but is not the goal.

> Then, using skeleton implementation (what I'm suggesting) can
> reduce
> the code more than a marker interface (the current code), IMHO.
> 
> For a skeleton implementation, inheriting ArcBridgeService::Observer looks
> straightforward. Invoking OnSomethingReady() method if necessary in a
> constructor
> followed by AddObserver(this) does not sound problematic, to me. (You can call
> all the necessary callback then).

What I don't like about this is that I would need to add all OnSomethingReady()
checks into ArcService. You would then need to modify the parent class to add
behavior that most implementations won't use.

There's also the problem that the IME service is split into two components: the
ImeBridge (that doesn't inherit from ArcBridgeService::Observer and does not
actually do IPC like the rest of the *Bridge classes) and the ImeIpcHost (that
does), but currently the ImeBridge is the one that is owned by
ArcServiceManager, so it needs some refactoring to work correctly. Please let me
land the current change and defer the refactoring of IME to reverse the
ownership.

> 
> With the implementation, subclasses would not have a problem like
> crbug.com/575873.
> (Of course, it is worth to be documented anyway).
> 
> CalledOnValidThread related stuff is optional actually, but IMHO, again
assuming
> we move to skeleton implementation concept, it looks clearer if we have the
> function
> in this class with clear documentation why and when the thread check is
needed.

I'm still not sure this is a good idea. The clipboard bridge has an
implementation of CalledOnValidThread that has different semantics (called on a
thread that has access to the clipboard). Even though the implementation is
identical and could be shared with ArcService, losing the flexibility to do so
doesn't seem worth it.

> 
> Bonus, if you have a ptr to ArcBridgeService, you can provide a simple getter
> with clear document that the object pointing by the pointer is alive while the
> instance is alive.
> 
> WDYT?

Having ArcService own the ptr to ArcBridgeService sounds good. Done.

[Luis Héctor Chávez, 2016-01-20 17:39:59]

https://codereview.chromium.org/1596663002/diff/80001/components/arc/arc_serv...
File components/arc/arc_service.h (right):

https://codereview.chromium.org/1596663002/diff/80001/components/arc/arc_serv...
components/arc/arc_service.h:20: 
On 2016/01/20 00:41:19, elijahtaylor1 wrote:
> all of the children of ArcService hold a weak reference to ArcBridgeService. 
> Can that be part of this class instead along with the calls to add/remove
> observers?  Or maybe adding the service to the service manager can add the
> observer?

Made this own the ArcBridgeService*.

https://codereview.chromium.org/1596663002/diff/80001/components/arc/arc_serv...
File components/arc/arc_service_manager.cc (right):

https://codereview.chromium.org/1596663002/diff/80001/components/arc/arc_serv...
components/arc/arc_service_manager.cc:34: AddService(make_scoped_ptr(new
ArcClipboardBridge(arc_bridge_service())));
On 2016/01/20 00:41:19, elijahtaylor1 wrote:
> what is the distinction between services created from outside of this
> constructor (chrome_browser_main_chromeos) and the ones created here?

Dependencies. All the ones created in chrome_browser_main_chromeos depend on
stuff that is not allowed for components/arc to depend on.

https://codereview.chromium.org/1596663002/diff/80001/components/arc/ime/arc_...
File components/arc/ime/arc_ime_ipc_host.cc (right):

https://codereview.chromium.org/1596663002/diff/80001/components/arc/ime/arc_...
components/arc/ime/arc_ime_ipc_host.cc:83: if
(arc_bridge_service_->ime_instance())
On 2016/01/20 00:41:19, elijahtaylor1 wrote:
> I don't think I understand why this can't be done in AddObserver to avoid this
> pattern in many of the sub interfaces

Hmmm adding it to AddObserver might work. Regardless, the IME bridge needs some
refactoring as I mentioned in the reply to hidehiko@ which I don't want to do in
this change.

[Luis Héctor Chávez, 2016-01-20 18:21:50]

On 2016/01/20 17:39:59, Luis Héctor Chávez wrote:
>
https://codereview.chromium.org/1596663002/diff/80001/components/arc/arc_serv...
> File components/arc/arc_service.h (right):
> 
>
https://codereview.chromium.org/1596663002/diff/80001/components/arc/arc_serv...
> components/arc/arc_service.h:20: 
> On 2016/01/20 00:41:19, elijahtaylor1 wrote:
> > all of the children of ArcService hold a weak reference to ArcBridgeService.

> > Can that be part of this class instead along with the calls to add/remove
> > observers?  Or maybe adding the service to the service manager can add the
> > observer?
> 
> Made this own the ArcBridgeService*.
> 
>
https://codereview.chromium.org/1596663002/diff/80001/components/arc/arc_serv...
> File components/arc/arc_service_manager.cc (right):
> 
>
https://codereview.chromium.org/1596663002/diff/80001/components/arc/arc_serv...
> components/arc/arc_service_manager.cc:34: AddService(make_scoped_ptr(new
> ArcClipboardBridge(arc_bridge_service())));
> On 2016/01/20 00:41:19, elijahtaylor1 wrote:
> > what is the distinction between services created from outside of this
> > constructor (chrome_browser_main_chromeos) and the ones created here?
> 
> Dependencies. All the ones created in chrome_browser_main_chromeos depend on
> stuff that is not allowed for components/arc to depend on.
> 
>
https://codereview.chromium.org/1596663002/diff/80001/components/arc/ime/arc_...
> File components/arc/ime/arc_ime_ipc_host.cc (right):
> 
>
https://codereview.chromium.org/1596663002/diff/80001/components/arc/ime/arc_...
> components/arc/ime/arc_ime_ipc_host.cc:83: if
> (arc_bridge_service_->ime_instance())
> On 2016/01/20 00:41:19, elijahtaylor1 wrote:
> > I don't think I understand why this can't be done in AddObserver to avoid
this
> > pattern in many of the sub interfaces
> 
> Hmmm adding it to AddObserver might work. Regardless, the IME bridge needs
some
> refactoring as I mentioned in the reply to hidehiko@ which I don't want to do
in
> this change.

Nevermind, the change to AddObserver is not blocked by anything and I prefer to
do it now to avoid having to go through another review cycle.

Another problem with adding AddObserver to ArcService's constructor I just
realized is that the object might not be fully initialized by the time
OnXXXReady() gets called.

[Luis Héctor Chávez, 2016-01-20 18:40:46]

https://codereview.chromium.org/1596663002/diff/20001/components/arc/arc_serv...
File components/arc/arc_service.h (right):

https://codereview.chromium.org/1596663002/diff/20001/components/arc/arc_serv...
components/arc/arc_service.h:20: ArcService() {}
On 2016/01/20 16:04:50, hidehiko wrote:
> On 2016/01/19 17:43:54, Luis Héctor Chávez wrote:
> > On 2016/01/18 01:26:45, hidehiko wrote:
> > > Can we move {Add,Remove}Observer(this) to this class, so we do not in
> > inherited
> > > classes?
> > > Also, you may want to move CallValidThread() check function to here, so
that
> > we
> > > can share the implementation?
> > 
> > The problem with moving AddObserver(this) to this class is that ArcService
> would
> > need to be an ArcBridgeService::Observer (but would not implement any
methods,
> > which is weird). Also, there is a subclass-specific check that needs to
happen
> > just after adding the observer, which would make it error-prone to omit
since
> it
> > would work some of the times (most of the interfaces did not follow the best
> > practices doc and ended up not including it). See what happened with the app
> > list (crbug.com/575873) for an example of why I'm a bit against doing this.
> > 
> > If I don't add AddObserver to this class, adding RemoveObserver feels
> > inconsistent.
> > 
> > For CalledOnValidThread, we probably should instead make the subclasses
> inherit
> > from NonThreadSafe instead of making ArcService take on responsibilities it
> > shouldn't. And even that is discouraged in favor of keeping a
> > ThreadChecker+calling CalledOnValidThread:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/threading/thr...
> 
> My understanding is that the goal of this CL is to de-dup the implementation
> boilerplate. Then, using skeleton implementation (what I'm suggesting) can
> reduce
> the code more than a marker interface (the current code), IMHO.
> 
> For a skeleton implementation, inheriting ArcBridgeService::Observer looks
> straightforward. Invoking OnSomethingReady() method if necessary in a
> constructor
> followed by AddObserver(this) does not sound problematic, to me. (You can call
> all the necessary callback then).
> 
> With the implementation, subclasses would not have a problem like
> crbug.com/575873.
> (Of course, it is worth to be documented anyway).
> 
> CalledOnValidThread related stuff is optional actually, but IMHO, again
assuming
> we move to skeleton implementation concept, it looks clearer if we have the
> function
> in this class with clear documentation why and when the thread check is
needed.
> 
> Bonus, if you have a ptr to ArcBridgeService, you can provide a simple getter
> with clear document that the object pointing by the pointer is alive while the
> instance is alive.
> 
> WDYT?

Turns out this doesn't work in C++:
https://isocpp.org/wiki/faq/strange-inheritance#calling-virtuals-from-ctors

If I were to move AddObserver/RemoveObserver to this class, only the
implementations of the OnSomethingReady() from ArcService would be called, not
the ones from the subclass, which would cause them to do nothing.

https://codereview.chromium.org/1596663002/diff/80001/components/arc/ime/arc_...
File components/arc/ime/arc_ime_ipc_host.cc (right):

https://codereview.chromium.org/1596663002/diff/80001/components/arc/ime/arc_...
components/arc/ime/arc_ime_ipc_host.cc:83: if
(arc_bridge_service_->ime_instance())
On 2016/01/20 17:39:59, Luis Héctor Chávez wrote:
> On 2016/01/20 00:41:19, elijahtaylor1 wrote:
> > I don't think I understand why this can't be done in AddObserver to avoid
this
> > pattern in many of the sub interfaces
> 
> Hmmm adding it to AddObserver might work. Regardless, the IME bridge needs
some
> refactoring as I mentioned in the reply to hidehiko@ which I don't want to do
in
> this change.

This is now done.

[hidehiko, 2016-01-21 08:08:28]

LGTM.

Thank you for explanation of your design.
Now I believe I understand your goal, and this CL looks enough to me.

https://codereview.chromium.org/1596663002/diff/120001/components/arc/arc_ser...
File components/arc/arc_service.h (right):

https://codereview.chromium.org/1596663002/diff/120001/components/arc/arc_ser...
components/arc/arc_service.h:21: virtual ~ArcService();
nit: let's move this to protected to disallow direct instantiation.

[Luis Héctor Chávez, 2016-01-21 22:42:14]

https://codereview.chromium.org/1596663002/diff/120001/components/arc/arc_ser...
File components/arc/arc_service.h (right):

https://codereview.chromium.org/1596663002/diff/120001/components/arc/arc_ser...
components/arc/arc_service.h:21: virtual ~ArcService();
On 2016/01/21 08:08:28, hidehiko wrote:
> nit: let's move this to protected to disallow direct instantiation.

Only protecting the constructor since scoped_ptr uses the destructor.

[Luis Héctor Chávez, 2016-01-21 22:43:02]

The CQ bit was checked by lhchavez@chromium.org

[Luis Héctor Chávez, 2016-01-21 22:43:03]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org,
hidehiko@chromium.org
Link to the patchset: https://codereview.chromium.org/1596663002/#ps140001
(title: "Rebased + protected ArcService direct instantiation")

[commit-bot: I haz the power, 2016-01-21 22:43:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1596663002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1596663002/140001

[commit-bot: I haz the power, 2016-01-21 23:27:33]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-01-21 23:30:27]

Description was changed from

==========
arc-bridge: Introduce the ArcService class

Several services managed by ARC were introducing empty, opaque classes
just to be able to be constructed outside of components/. This was
becoming ugly, so introduce just one interface that all services must
implement and whose lifecycle will be managed by ArcServiceManager.

BUG=None
TEST=trybots
==========

to

==========
arc-bridge: Introduce the ArcService class

Several services managed by ARC were introducing empty, opaque classes
just to be able to be constructed outside of components/. This was
becoming ugly, so introduce just one interface that all services must
implement and whose lifecycle will be managed by ArcServiceManager.

BUG=None
TEST=trybots

Committed: https://crrev.com/811769ba67bef5ff1e0a120146c0d593a6ebe5db
Cr-Commit-Position: refs/heads/master@{#370825}
==========

[commit-bot: I haz the power, 2016-01-21 23:30:28]

Patchset 8 (id:??) landed as
https://crrev.com/811769ba67bef5ff1e0a120146c0d593a6ebe5db
Cr-Commit-Position: refs/heads/master@{#370825}