Refactor ArcSessionRunner part 2.

Refactor ArcSessionRunner part 2.

This CL gets rid of inheritance between ArcBridgeService and
ArcSessionRunner, instead uses composition.

BUG=657687
BUG=b/31079732
TEST=Ran bots.

Committed: https://crrev.com/515fe2b8843b39041f625c947393cd01aa4aa8c9
Cr-Commit-Position: refs/heads/master@{#440036}

[hidehiko, 2016-12-19 09:07:47]

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

[commit-bot: I haz the power, 2016-12-19 09:08: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-12-19 09:09:57]

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

[commit-bot: I haz the power, 2016-12-19 09:09:57]

Dry run: 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-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[hidehiko, 2016-12-19 09:21:09]

hidehiko@chromium.org changed reviewers:
+ benwells@chromium.org, lhchavez@chromium.org

[hidehiko, 2016-12-19 09:21:10]

PTAL.

benwells@: c/b/ui/views/apps
lhchavez@: others.

Sorry for rush of code review requests, but considering now it is year end, let
me pile the CLs...

Note: the trybot failure is patch-failure something similar to the one happened
in the previous CL crrev.com/2574013003.
I'll re-run once rebasing issue is resolved. Note that, at least on my local
env, ARC related testss for unittest, browser_test and components_unittest look
passed.

Thank you for review in advance,
- hidehiko

https://codereview.chromium.org/2586183002/diff/1/components/arc/arc_bridge_s...
File components/arc/arc_bridge_service.h (right):

https://codereview.chromium.org/2586183002/diff/1/components/arc/arc_bridge_s...
components/arc/arc_bridge_service.h:83: bool ready() const;
These methods will be gone in a following CL. So, let me keep those names
(ready() and stopped()) as is for now.

[Luis Héctor Chávez, 2016-12-19 21:50:49]

lgtm with a low-priority question (since it might be refactored in the next
change).

https://codereview.chromium.org/2586183002/diff/1/components/arc/arc_bridge_s...
File components/arc/arc_bridge_service.h (right):

https://codereview.chromium.org/2586183002/diff/1/components/arc/arc_bridge_s...
components/arc/arc_bridge_service.h:71: void InitializeArcSessionRunner(
Is there a reason why this is not passed in the constructor? You always call it
immediately after constructing ArcBridgeService and having an ArcBridgeService
without its ArcSessionRunner is kind of unusable.

If you still want to allow to pass in nullptr as the |arc_session_runner|, you
can keep the DCHECKs in the proxy methods.

[benwells, 2016-12-20 01:58:15]

lgtm

[hidehiko, 2016-12-21 05:30:19]

Thank you for review. Rebased, and submitting.

https://codereview.chromium.org/2586183002/diff/1/components/arc/arc_bridge_s...
File components/arc/arc_bridge_service.h (right):

https://codereview.chromium.org/2586183002/diff/1/components/arc/arc_bridge_s...
components/arc/arc_bridge_service.h:71: void InitializeArcSessionRunner(
On 2016/12/19 21:50:49, Luis Héctor Chávez wrote:
> Is there a reason why this is not passed in the constructor? You always call
it
> immediately after constructing ArcBridgeService and having an ArcBridgeService
> without its ArcSessionRunner is kind of unusable.
> 
> If you still want to allow to pass in nullptr as the |arc_session_runner|, you
> can keep the DCHECKs in the proxy methods.

Two reasons;
1) The real factory for ArcSessionRunner requires ArcBridgeService pointer. So,
this instance needs to be created before ArcSessionRunner creation. So, due to
the dependency, we cannot pass ArcSessionRunner pointer to the ArcBridgeService
ctor.
2) As you said, I'm refactoring this in a following CL. Because ArcBridgeService
is created in some unittests, we can avoid "to change the ctor once in this CL,
and to revert them in a following CL", which sounds a kind of noise to me.

[hidehiko, 2016-12-21 05:30:24]

The CQ bit was checked by hidehiko@chromium.org

[hidehiko, 2016-12-21 05:30:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
lhchavez@chromium.org, benwells@chromium.org
Link to the patchset: https://codereview.chromium.org/2586183002/#ps20001
(title: "Rebase")

[commit-bot: I haz the power, 2016-12-21 05:30:48]

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-21 06:09:06]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1482298224902820,
"parent_rev": "05830d089bf0fc913c61417ad9f6c5552c713020", "commit_rev":
"c74180e91f7e218588cf8992e8869d3a142f7ac6"}

[commit-bot: I haz the power, 2016-12-21 06:10:12]

Description was changed from

==========
Refactor ArcSessionRunner part 2.

This CL gets rid of inheritance between ArcBridgeService and
ArcSessionRunner, instead uses composition.

BUG=657687
BUG=b/31079732
TEST=Ran bots.
==========

to

==========
Refactor ArcSessionRunner part 2.

This CL gets rid of inheritance between ArcBridgeService and
ArcSessionRunner, instead uses composition.

BUG=657687
BUG=b/31079732
TEST=Ran bots.

Review-Url: https://codereview.chromium.org/2586183002
==========

[commit-bot: I haz the power, 2016-12-21 06:10:13]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-21 06:13:06]

Description was changed from

==========
Refactor ArcSessionRunner part 2.

This CL gets rid of inheritance between ArcBridgeService and
ArcSessionRunner, instead uses composition.

BUG=657687
BUG=b/31079732
TEST=Ran bots.

Review-Url: https://codereview.chromium.org/2586183002
==========

to

==========
Refactor ArcSessionRunner part 2.

This CL gets rid of inheritance between ArcBridgeService and
ArcSessionRunner, instead uses composition.

BUG=657687
BUG=b/31079732
TEST=Ran bots.

Committed: https://crrev.com/515fe2b8843b39041f625c947393cd01aa4aa8c9
Cr-Commit-Position: refs/heads/master@{#440036}
==========

[commit-bot: I haz the power, 2016-12-21 06:13:07]

Patchset 2 (id:??) landed as
https://crrev.com/515fe2b8843b39041f625c947393cd01aa4aa8c9
Cr-Commit-Position: refs/heads/master@{#440036}