[Android] Clean up old_interface in build/android/.

[Android] Clean up old_interface in build/android/.

BUG=267773
Committed: https://crrev.com/32b3e358017c863318fa7cca3a3c41bb5d056c24
Cr-Commit-Position: refs/heads/master@{#323135}

[jbudorick, 2015-03-30 15:27:33]

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

[jbudorick, 2015-03-30 15:29:21]

On 2015/03/30 15:27:33, jbudorick wrote:

(note that this doesn't remove all instances of old_interface in build/android/;
there are some trickier cases that I want to address individually)

[klundberg, 2015-03-30 15:40:18]

lgtm

[perezju, 2015-03-30 16:01:17]

This is looking good! Happy to see all of those old_interface's going away!

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/dev...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:400: self.RunShellCommand(['sh',
tmp.name], as_root=True, check_return=True)
wouldn't RunShellCommand(_RESTART_ADBD_SCRIPT, ...) "just work"?

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:533: cmd = '
'.join(cmd_helper.SingleQuote(s) for s in cmd if s)
Not sure if we want to do this. A client may have a valid reason to want to pass
an empty string as an argument. Maybe "if s is not None"? (Maybe clients should
be expected to pass lists with only valid arguments?)

Edit: Ah, I see where this is used. Yeah, I would rather have the client do the
filtering itself, and pass only the arguments it needs.

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/gte...
File build/android/pylib/gtest/test_package_exe.py (right):

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/gte...
build/android/pylib/gtest/test_package_exe.py:123: lib_path = '%s/%s_deps' %
(constants.TEST_EXECUTABLE_DIR, self.suite_name)
nit: maybe should also use posixpath.join?

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/gte...
build/android/pylib/gtest/test_package_exe.py:125:
[device.GetDevicePieWrapper(), self.tool.GetTestWrapper(),
only pass wrappers if needed.

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/scr...
File build/android/pylib/screenshot.py (right):

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/scr...
build/android/pylib/screenshot.py:88: or 'screen-recording-%s.mp4' %
time.strftime('%Y%m%dT%H%M%S',
there is a _GetTimeStamp in device_utils.py, maybe we want to move it somewhere
else and make it public?

[jbudorick, 2015-03-30 16:42:15]

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/dev...
File build/android/pylib/device/device_utils.py (right):

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:400: self.RunShellCommand(['sh',
tmp.name], as_root=True, check_return=True)
On 2015/03/30 16:01:16, perezju wrote:
> wouldn't RunShellCommand(_RESTART_ADBD_SCRIPT, ...) "just work"?

Not quite (which is probably what I tried before this), but
RunShellCommand(['sh', '-c', _RESTART_ADBD_SCRIPT]) does. Switching to that
instead of the DeviceTempFile version.

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/dev...
build/android/pylib/device/device_utils.py:533: cmd = '
'.join(cmd_helper.SingleQuote(s) for s in cmd if s)
On 2015/03/30 16:01:16, perezju wrote:
> Not sure if we want to do this. A client may have a valid reason to want to
pass
> an empty string as an argument. Maybe "if s is not None"? (Maybe clients
should
> be expected to pass lists with only valid arguments?)
> 

I was trying to think of cases where a client would want to do this and
couldn't. That said, I think that expecting the client to filter out things it
doesn't want is reasonable and will switch to that.

> Edit: Ah, I see where this is used. Yeah, I would rather have the client do
the
> filtering itself, and pass only the arguments it needs.

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/gte...
File build/android/pylib/gtest/test_package_exe.py (right):

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/gte...
build/android/pylib/gtest/test_package_exe.py:123: lib_path = '%s/%s_deps' %
(constants.TEST_EXECUTABLE_DIR, self.suite_name)
On 2015/03/30 16:01:17, perezju wrote:
> nit: maybe should also use posixpath.join?

Done.

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/gte...
build/android/pylib/gtest/test_package_exe.py:125:
[device.GetDevicePieWrapper(), self.tool.GetTestWrapper(),
On 2015/03/30 16:01:16, perezju wrote:
> only pass wrappers if needed.

Done.

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/scr...
File build/android/pylib/screenshot.py (right):

https://codereview.chromium.org/1047703002/diff/20001/build/android/pylib/scr...
build/android/pylib/screenshot.py:88: or 'screen-recording-%s.mp4' %
time.strftime('%Y%m%dT%H%M%S',
On 2015/03/30 16:01:17, perezju wrote:
> there is a _GetTimeStamp in device_utils.py, maybe we want to move it
somewhere
> else and make it public?

Maybe. I'll look at doing that in a separate CL + add a TODO here.

[perezju, 2015-03-31 08:19:58]

lgtm

[jbudorick, 2015-03-31 21:44:51]

The CQ bit was checked by jbudorick@chromium.org

[jbudorick, 2015-03-31 21:44:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
klundberg@chromium.org
Link to the patchset: https://codereview.chromium.org/1047703002/#ps40001
(title: " ")

[commit-bot: I haz the power, 2015-03-31 21:47:06]

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

[commit-bot: I haz the power, 2015-03-31 22:51:38]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-31 22:52:33]

Patchset 3 (id:??) landed as
https://crrev.com/32b3e358017c863318fa7cca3a3c41bb5d056c24
Cr-Commit-Position: refs/heads/master@{#323135}

[Michael Achenbach, 2015-04-01 08:35:32]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1049993003/ by machenbach@chromium.org.

The reason for reverting is: This make sharded perf tests fail everywhere,
example:
http://build.chromium.org/p/chromium.perf/builders/Android%20Nexus4%20Perf/bu....