Migrate device_utils.RestartServer to adb_wrapper

Migrate device_utils.RestartServer to adb_wrapper

The implementation is based on the new AdbWrapper class methods:
KillServer and StartServer.

BUG=267773
Committed: https://crrev.com/872e2f0d24529365451323c77fe52a56687b32bf
Cr-Commit-Position: refs/heads/master@{#308336}

[perezju, 2014-12-09 14:32:50]

perezju@chromium.org changed reviewers:
+ jbudorick@chromium.org

[perezju, 2014-12-09 14:32:50]

Please have a look.

Right after uploading this CL, however, I thought that maybe AdbWrapper should
also provide an IsServerOnline class method (that internally does the pgrep
thing). This would be needed so that a theoretical implementation of adb_wrapper
based on, e.g., pyadb, could override this method appropriately.

[jbudorick, 2014-12-11 01:43:13]

AdbWrapper is designed to basically just be a python interface to adb. As such,
I'm hesitant to add higher level things involving adb to AdbWrapper when that's
generally the idea behind DeviceUtils.

https://codereview.chromium.org/724413003/diff/1/build/android/pylib/device/a...
File build/android/pylib/device/adb_wrapper.py (right):

https://codereview.chromium.org/724413003/diff/1/build/android/pylib/device/a...
build/android/pylib/device/adb_wrapper.py:61: check_error=True,
cpu_affinity=None):
Probably doesn't surprise you to hear that I'm not a fan of adding arguments for
a one-off, although I'm not immediately sure how else you'd do this since it
needs to precede the adb call.

https://codereview.chromium.org/724413003/diff/1/build/android/pylib/device/a...
build/android/pylib/device/adb_wrapper.py:63: cmd = ['taskset', '-c',
str(cpu_affinity)]
How is this going to interact with _BuildAdbCmd in your Logcat CL?

[jbudorick, 2014-12-11 01:43:36]

On 2014/12/11 01:43:13, jbudorick wrote:
> AdbWrapper is designed to basically just be a python interface to adb. As
such,
> I'm hesitant to add higher level things involving adb to AdbWrapper when
that's
> generally the idea behind DeviceUtils.

(this is w.r.t. your IsServerOnline idea)

>
https://codereview.chromium.org/724413003/diff/1/build/android/pylib/device/a...
> File build/android/pylib/device/adb_wrapper.py (right):
> 
>
https://codereview.chromium.org/724413003/diff/1/build/android/pylib/device/a...
> build/android/pylib/device/adb_wrapper.py:61: check_error=True,
> cpu_affinity=None):
> Probably doesn't surprise you to hear that I'm not a fan of adding arguments
for
> a one-off, although I'm not immediately sure how else you'd do this since it
> needs to precede the adb call.
> 
>
https://codereview.chromium.org/724413003/diff/1/build/android/pylib/device/a...
> build/android/pylib/device/adb_wrapper.py:63: cmd = ['taskset', '-c',
> str(cpu_affinity)]
> How is this going to interact with _BuildAdbCmd in your Logcat CL?

[perezju, 2014-12-11 11:38:00]

https://codereview.chromium.org/724413003/diff/1/build/android/pylib/device/a...
File build/android/pylib/device/adb_wrapper.py (right):

https://codereview.chromium.org/724413003/diff/1/build/android/pylib/device/a...
build/android/pylib/device/adb_wrapper.py:61: check_error=True,
cpu_affinity=None):
On 2014/12/11 01:43:13, jbudorick wrote:
> Probably doesn't surprise you to hear that I'm not a fan of adding arguments
for
> a one-off, although I'm not immediately sure how else you'd do this since it
> needs to precede the adb call.

Yeah, I don't like this too much either. But couldn't think of other way to
achieve what was needed. Also I don't think setting the cpu_affinity should be a
matter of DeviceUtils, because it's really an issue of adb (imagine a
theoretical PyAdbWrapper, presumably this "fix" for adb instability shouldn't be
needed at all).

https://codereview.chromium.org/724413003/diff/1/build/android/pylib/device/a...
build/android/pylib/device/adb_wrapper.py:63: cmd = ['taskset', '-c',
str(cpu_affinity)]
On 2014/12/11 01:43:13, jbudorick wrote:
> How is this going to interact with _BuildAdbCmd in your Logcat CL?

I think cpu_affinity (or something like that) would end up as an option to
_BuildAdbCmd. And yes, one of the two CLs should land first, and the other will
have to be rebased against the first one.

[jbudorick, 2014-12-11 16:27:17]

lgtm

order between this & Logcat is up to you.

https://codereview.chromium.org/724413003/diff/1/build/android/pylib/device/a...
File build/android/pylib/device/adb_wrapper.py (right):

https://codereview.chromium.org/724413003/diff/1/build/android/pylib/device/a...
build/android/pylib/device/adb_wrapper.py:61: check_error=True,
cpu_affinity=None):
On 2014/12/11 11:38:00, perezju wrote:
> On 2014/12/11 01:43:13, jbudorick wrote:
> > Probably doesn't surprise you to hear that I'm not a fan of adding arguments
> for
> > a one-off, although I'm not immediately sure how else you'd do this since it
> > needs to precede the adb call.
> 
> Yeah, I don't like this too much either. But couldn't think of other way to
> achieve what was needed. Also I don't think setting the cpu_affinity should be
a
> matter of DeviceUtils, because it's really an issue of adb (imagine a
> theoretical PyAdbWrapper, presumably this "fix" for adb instability shouldn't
be
> needed at all).

Agreed.

[perezju, 2014-12-15 10:31:08]

The CQ bit was checked by perezju@chromium.org

[commit-bot: I haz the power, 2014-12-15 10:32:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/724413003/20001

[commit-bot: I haz the power, 2014-12-15 11:29:58]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-12-15 11:30:43]

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

[perezju, 2014-12-15 15:38:30]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/804033002/ by perezju@chromium.org.

The reason for reverting is: Broke (even more!) the android perf bots. Reverting
while investigating..