Reland "arc-bridge: Move most methods to Mojo interfaces"

Reland "arc-bridge: Move most methods to Mojo interfaces"

This reverts commit b988466841a1f7b77d0c2bd6824273b7f82b4a36 with the
fix found in https://codereview.chromium.org/1534413002/

This should make the ArcBridgeService surface area smaller by moving all
methods to separate Mojo interfaces. This also lets each individual
service handle its own lifetime and should simplify the Observers
significatively.

BUG=569446
TEST=unit_tests, components_unittests

Committed: https://crrev.com/bf5a21ce606da25b2925c76d8e59be628a3dce51
Cr-Commit-Position: refs/heads/master@{#366477}

[lhc(google), 2015-12-20 00:34:44]

lhchavez@google.com changed reviewers:
+ afakhry@chromium.org, dcheng@chromium.org, elijahtaylor@chromium.org,
hidehiko@chromium.org, lhchavez@google.com, xiyuan@chromium.org

[lhc(google), 2015-12-20 00:34:45]

PTAL

[dcheng, 2015-12-20 00:44:40]

Can you upload a patchset with the original patch as committed, and then one
with the fixes? That'll make it easier to diff and review.

[lhc(google), 2015-12-20 00:53:51]

Good idea, done.

[dcheng, 2015-12-20 01:49:46]

mojom lgtm

https://codereview.chromium.org/1541653002/diff/40001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_app_unittest.cc (right):

https://codereview.chromium.org/1541653002/diff/40001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_unittest.cc:70:
app_instance_->WaitForIncomingMethodCall();
Too bad there isn't some sort of way to be more explicit about the calls being
waited for.

[elijahtaylor1, 2015-12-21 00:10:23]

lgtm

[xiyuan, 2015-12-21 17:05:53]

https://codereview.chromium.org/1541653002/diff/40001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_app_unittest.cc (right):

https://codereview.chromium.org/1541653002/diff/40001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_unittest.cc:70:
app_instance_->WaitForIncomingMethodCall();
On 2015/12/20 01:49:45, dcheng wrote:
> Too bad there isn't some sort of way to be more explicit about the calls being
> waited for.

Yeah, this is confusing to read. Can we add something to FakeAppInstance so that
we can wait on Init() and RefreshAppList() explicitly?

For now, it could even just be a wrapper for the two WaitForIncomingMethodCall()
e.g.

void FakeAppInstance::WaitForInitAndRefreshAppList() {
  WaitForIncomingMethodCall();  // Wait for Init call
  WaitForIncomingMethodCall();  // Wait for RefreshAppList call
}

And call this here. This would make it easier to read and the details of
FakeAppInstance stays in FakeAppInstance.

[Luis Héctor Chávez, 2015-12-21 17:24:21]

https://codereview.chromium.org/1541653002/diff/40001/chrome/browser/ui/app_l...
File chrome/browser/ui/app_list/arc/arc_app_unittest.cc (right):

https://codereview.chromium.org/1541653002/diff/40001/chrome/browser/ui/app_l...
chrome/browser/ui/app_list/arc/arc_app_unittest.cc:70:
app_instance_->WaitForIncomingMethodCall();
On 2015/12/21 17:05:53, xiyuan wrote:
> On 2015/12/20 01:49:45, dcheng wrote:
> > Too bad there isn't some sort of way to be more explicit about the calls
being
> > waited for.
> 
> Yeah, this is confusing to read. Can we add something to FakeAppInstance so
that
> we can wait on Init() and RefreshAppList() explicitly?
> 
> For now, it could even just be a wrapper for the two
WaitForIncomingMethodCall()
> e.g.
> 
> void FakeAppInstance::WaitForInitAndRefreshAppList() {
>   WaitForIncomingMethodCall();  // Wait for Init call
>   WaitForIncomingMethodCall();  // Wait for RefreshAppList call
> }
> 
> And call this here. This would make it easier to read and the details of
> FakeAppInstance stays in FakeAppInstance.

Done.

[xiyuan, 2015-12-21 18:03:19]

lgtm

Cool.

[afakhry, 2015-12-21 21:15:31]

task_management/ lgtm

[lhc(google), 2015-12-21 21:33:41]

The CQ bit was checked by lhchavez@google.com

[lhc(google), 2015-12-21 21:33:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
elijahtaylor@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/1541653002/#ps60001
(title: "Added explicit wait call for OnInstanceReady.")

[commit-bot: I haz the power, 2015-12-21 21:34:20]

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

[commit-bot: I haz the power, 2015-12-21 22:40:20]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-12-21 22:41:07]

Description was changed from

==========
Reland "arc-bridge: Move most methods to Mojo interfaces"

This reverts commit b988466841a1f7b77d0c2bd6824273b7f82b4a36 with the
fix found in https://codereview.chromium.org/1534413002/

This should make the ArcBridgeService surface area smaller by moving all
methods to separate Mojo interfaces. This also lets each individual
service handle its own lifetime and should simplify the Observers
significatively.

BUG=569446
TEST=unit_tests, components_unittests
==========

to

==========
Reland "arc-bridge: Move most methods to Mojo interfaces"

This reverts commit b988466841a1f7b77d0c2bd6824273b7f82b4a36 with the
fix found in https://codereview.chromium.org/1534413002/

This should make the ArcBridgeService surface area smaller by moving all
methods to separate Mojo interfaces. This also lets each individual
service handle its own lifetime and should simplify the Observers
significatively.

BUG=569446
TEST=unit_tests, components_unittests

Committed: https://crrev.com/bf5a21ce606da25b2925c76d8e59be628a3dce51
Cr-Commit-Position: refs/heads/master@{#366477}
==========

[commit-bot: I haz the power, 2015-12-21 22:41:09]

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

[Pete Williamson, 2015-12-21 23:33:19]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1542003002/ by petewil@chromium.org.

The reason for reverting is: Breaks the compile, reverting as sheriff.  Missing
a gypi entry..