Refactor ArcBridgeService detection and initialization to a separate class ArcServiceLauncher.

Refactor ArcBridgeService detection and initialization to a separate class ArcServiceLauncher.

BUG=588783
Committed: https://crrev.com/ace7826b196f8dca3e79cd4aba0326f1c357d42c
Cr-Commit-Position: refs/heads/master@{#377968}

[arthurhsu, 2016-02-17 02:16:40]

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

[commit-bot: I haz the power, 2016-02-17 02:17:57]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1701063003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1701063003/20001

[commit-bot: I haz the power, 2016-02-17 02:17:58]

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

[commit-bot: I haz the power, 2016-02-17 02:17:59]

Dry run: No L-G-T-M from a valid reviewer yet. Only full committers are
accepted.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[arthurhsu, 2016-02-17 23:55:14]

Description was changed from

==========
Refactor ArcBridgeService detection and initialization to a separate class
ArcServiceLauncher.

BUG=
==========

to

==========
Refactor ArcBridgeService detection and initialization to a separate class
ArcServiceLauncher.

BUG=
==========

[arthurhsu, 2016-02-17 23:55:14]

arthurhsu@chromium.org changed reviewers:
+ lhchavez@chromium.org

[arthurhsu, 2016-02-19 23:08:17]

arthurhsu@chromium.org changed reviewers:
+ hidehiko@chromium.org

[arthurhsu, 2016-02-19 23:08:43]

+hidehiko for OWNERS review

[hidehiko, 2016-02-22 08:08:40]

Before entering the detailed review:

Could you explain what is your goal of this refactoring (why you need briefly?),
and update the CL description along with it?
Could you add TEST= with what you did?
Could you fill BUG= line properly?
Could you run try bots?

https://codereview.chromium.org/1701063003/diff/60001/components/arc/arc_brid...
File components/arc/arc_bridge_service.h (right):

https://codereview.chromium.org/1701063003/diff/60001/components/arc/arc_brid...
components/arc/arc_bridge_service.h:137: virtual void SetArcAvailability(bool
availability) = 0;
ArcBridgeService::SetArcAbailability and ArcBridgeService::SetAvailable sounds
confusing. Can we re-sort?

[arthurhsu, 2016-02-22 18:56:14]

Description was changed from

==========
Refactor ArcBridgeService detection and initialization to a separate class
ArcServiceLauncher.

BUG=
==========

to

==========
Refactor ArcBridgeService detection and initialization to a separate class
ArcServiceLauncher.

BUG=588783
==========

[arthurhsu, 2016-02-22 19:14:45]

On 2016/02/22 at 08:08:40, hidehiko wrote:
> Before entering the detailed review:
> 
> Could you explain what is your goal of this refactoring (why you need
briefly?),
> and update the CL description along with it?

Filed a BUG and described in the DD linked there.

> Could you add TEST= with what you did?
> Could you fill BUG= line properly?
> Could you run try bots?

Done.

>
https://codereview.chromium.org/1701063003/diff/60001/components/arc/arc_brid...
> File components/arc/arc_bridge_service.h (right):
> 
>
https://codereview.chromium.org/1701063003/diff/60001/components/arc/arc_brid...
> components/arc/arc_bridge_service.h:137: virtual void SetArcAvailability(bool
availability) = 0;
> ArcBridgeService::SetArcAbailability and ArcBridgeService::SetAvailable sounds
confusing. Can we re-sort?

Done.

[hidehiko, 2016-02-25 02:12:56]

lgtm

[arthurhsu, 2016-02-25 17:59:56]

The CQ bit was checked by arthurhsu@chromium.org

[commit-bot: I haz the power, 2016-02-25 18:02:53]

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

[commit-bot: I haz the power, 2016-02-25 18:22:48]

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

[commit-bot: I haz the power, 2016-02-25 18:22:49]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[arthurhsu, 2016-02-25 18:29:59]

arthurhsu@chromium.org changed reviewers:
+ xiyuan@chromium.org

[arthurhsu, 2016-02-25 18:30:47]

+xiyuan for OWNERS review of chrome_browser_main_chromeos.*

[xiyuan, 2016-02-26 17:38:01]

lgtm

https://codereview.chromium.org/1701063003/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/arc/arc_service_launcher.h (right):

https://codereview.chromium.org/1701063003/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/arc/arc_service_launcher.h:22: void Launch();
nit: Launch -> Initialize, Initialize is used in association with Shutdown

https://codereview.chromium.org/1701063003/diff/80001/components/arc/arc_brid...
File components/arc/arc_bridge_service.h (right):

https://codereview.chromium.org/1701063003/diff/80001/components/arc/arc_brid...
components/arc/arc_bridge_service.h:137: virtual void SetDetectionResult(bool
availability) = 0;
nit: SetDetectionResult -> SetDetectedAvailability

[arthurhsu, 2016-02-26 19:10:06]

https://codereview.chromium.org/1701063003/diff/80001/chrome/browser/chromeos...
File chrome/browser/chromeos/arc/arc_service_launcher.h (right):

https://codereview.chromium.org/1701063003/diff/80001/chrome/browser/chromeos...
chrome/browser/chromeos/arc/arc_service_launcher.h:22: void Launch();
On 2016/02/26 at 17:38:01, xiyuan wrote:
> nit: Launch -> Initialize, Initialize is used in association with Shutdown

Done.

https://codereview.chromium.org/1701063003/diff/80001/components/arc/arc_brid...
File components/arc/arc_bridge_service.h (right):

https://codereview.chromium.org/1701063003/diff/80001/components/arc/arc_brid...
components/arc/arc_bridge_service.h:137: virtual void SetDetectionResult(bool
availability) = 0;
On 2016/02/26 at 17:38:01, xiyuan wrote:
> nit: SetDetectionResult -> SetDetectedAvailability

Done

[arthurhsu, 2016-02-26 19:11:02]

The CQ bit was checked by arthurhsu@chromium.org

[arthurhsu, 2016-02-26 19:11:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
hidehiko@chromium.org, xiyuan@chromium.org
Link to the patchset: https://codereview.chromium.org/1701063003/#ps100001
(title: "Address reviewer feedback")

[commit-bot: I haz the power, 2016-02-26 19:12:08]

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

[commit-bot: I haz the power, 2016-02-26 20:49:37]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-02-26 20:59:25]

Description was changed from

==========
Refactor ArcBridgeService detection and initialization to a separate class
ArcServiceLauncher.

BUG=588783
==========

to

==========
Refactor ArcBridgeService detection and initialization to a separate class
ArcServiceLauncher.

BUG=588783

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

[commit-bot: I haz the power, 2016-02-26 20:59:27]

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