[Android] Move screenshot and video_recorder into devil.

[Android] Move screenshot and video_recorder into devil.

BUG=476719
Committed: https://crrev.com/688462bf65fa163b4b06104f217daf9f00cf8a6a
Cr-Commit-Position: refs/heads/master@{#365837}

[jbudorick, 2015-12-15 02:13:19]

jbudorick@chromium.org changed reviewers:
+ mikecase@chromium.org, rnephew@chromium.org

[jbudorick, 2015-12-15 02:13:19]

I rewrote large swaths of this code, so please review as such.

[rnephew (Wrong account), 2015-12-15 14:48:28]

rnephew@google.com changed reviewers:
+ rnephew@google.com

[rnephew (Wrong account), 2015-12-15 14:48:29]

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
File build/android/devil/android/tools/flash_device.py (right):

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
build/android/devil/android/tools/flash_device.py:11: if __name__ == '__main__':
I thought this was suppose to be in the path on the bots already? Or did that
assumption not hold up.

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
File build/android/devil/android/tools/screenshot.py (right):

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
build/android/devil/android/tools/screenshot.py:24: parser.add_argument('-d',
'--device', metavar='ANDROID_DEVICE',
Should this make a list of devices so you can specify more than one? That might
make the naming a little tricker though.

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
build/android/devil/android/tools/screenshot.py:37: host_file = args.host_file
or args.file
Is there a reason we need both options?

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
File build/android/devil/android/tools/video_recorder.py (right):

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
build/android/devil/android/tools/video_recorder.py:28: Args:
nit: move under __init__

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
build/android/devil/android/tools/video_recorder.py:127: host_file =
args.host_file or args.file
Same question about needing 2 args for this.

[jbudorick, 2015-12-15 15:13:42]

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
File build/android/devil/android/tools/flash_device.py (right):

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
build/android/devil/android/tools/flash_device.py:11: if __name__ == '__main__':
On 2015/12/15 14:48:28, rnephew wrote:
> I thought this was suppose to be in the path on the bots already? Or did that
> assumption not hold up.

I changed my mind for these scripts. This directory probably will be on the path
for the bots, but it's annoying for a user to have to run PYTHONPATH=. python
./devil/android/tools/screenshot.py instead of ./screenshot.py

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
File build/android/devil/android/tools/screenshot.py (right):

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
build/android/devil/android/tools/screenshot.py:24: parser.add_argument('-d',
'--device', metavar='ANDROID_DEVICE',
On 2015/12/15 14:48:29, rnephew wrote:
> Should this make a list of devices so you can specify more than one? That
might
> make the naming a little tricker though.

Hm, maybe. If so, I'd probably also change the behavior when no device is
explicitly requested to capture screenshots from all attached devices. WDYT?

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
build/android/devil/android/tools/screenshot.py:37: host_file = args.host_file
or args.file
On 2015/12/15 14:48:29, rnephew wrote:
> Is there a reason we need both options?

for now, historical compatibility.

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
File build/android/devil/android/tools/video_recorder.py (right):

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
build/android/devil/android/tools/video_recorder.py:28: Args:
On 2015/12/15 14:48:29, rnephew wrote:
> nit: move under __init__

Done.

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
build/android/devil/android/tools/video_recorder.py:127: host_file =
args.host_file or args.file
On 2015/12/15 14:48:29, rnephew wrote:
> Same question about needing 2 args for this.

same "historical compatibility" reason

[rnephew (Reviews Here), 2015-12-15 15:36:26]

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
File build/android/devil/android/tools/screenshot.py (right):

https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
build/android/devil/android/tools/screenshot.py:24: parser.add_argument('-d',
'--device', metavar='ANDROID_DEVICE',
On 2015/12/15 15:13:41, jbudorick wrote:
> On 2015/12/15 14:48:29, rnephew wrote:
> > Should this make a list of devices so you can specify more than one? That
> might
> > make the naming a little tricker though.
> 
> Hm, maybe. If so, I'd probably also change the behavior when no device is
> explicitly requested to capture screenshots from all attached devices. WDYT?

Yeah, I agree with that. Maybe for naming you could just append the device
serial to the end?

[jbudorick, 2015-12-15 23:30:49]

On 2015/12/15 15:36:26, rnephew1 wrote:
>
https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
> File build/android/devil/android/tools/screenshot.py (right):
> 
>
https://codereview.chromium.org/1522313002/diff/1/build/android/devil/android...
> build/android/devil/android/tools/screenshot.py:24: parser.add_argument('-d',
> '--device', metavar='ANDROID_DEVICE',
> On 2015/12/15 15:13:41, jbudorick wrote:
> > On 2015/12/15 14:48:29, rnephew wrote:
> > > Should this make a list of devices so you can specify more than one? That
> > might
> > > make the naming a little tricker though.
> > 
> > Hm, maybe. If so, I'd probably also change the behavior when no device is
> > explicitly requested to capture screenshots from all attached devices. WDYT?
> 
> Yeah, I agree with that. Maybe for naming you could just append the device
> serial to the end?

done.

[rnephew (Reviews Here), 2015-12-15 23:55:10]

lgtm.

https://codereview.chromium.org/1522313002/diff/40001/build/android/devil/and...
File build/android/devil/android/tools/script_common.py (right):

https://codereview.chromium.org/1522313002/diff/40001/build/android/devil/and...
build/android/devil/android/tools/script_common.py:10: def
GetDevices(requested_devices, blacklist_file):
I like this, I'll have to move flash_devices.py to use this; but that should be
a seperate CL.

[jbudorick, 2015-12-15 23:57:20]

https://codereview.chromium.org/1522313002/diff/40001/build/android/devil/and...
File build/android/devil/android/tools/script_common.py (right):

https://codereview.chromium.org/1522313002/diff/40001/build/android/devil/and...
build/android/devil/android/tools/script_common.py:10: def
GetDevices(requested_devices, blacklist_file):
On 2015/12/15 23:55:10, rnephew1 wrote:
> I like this, I'll have to move flash_devices.py to use this; but that should
be
> a seperate CL.

Yeah, agreed.

[jbudorick, 2015-12-15 23:57:30]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2015-12-15 23:59:37]

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

[commit-bot: I haz the power, 2015-12-16 03:50:04]

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

[commit-bot: I haz the power, 2015-12-16 03:50:05]

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

[jbudorick, 2015-12-16 15:02:28]

jbudorick@chromium.org changed reviewers:
+ aiolos@chromium.org, nednguyen@google.com

[jbudorick, 2015-12-16 15:02:28]

+aiolos, nednguyen: had to pull the screenshot telemetry changes over from
https://codereview.chromium.org/1526743004/

[nednguyen, 2015-12-16 15:50:42]

lgtm

[jbudorick, 2015-12-16 15:53:08]

The CQ bit was checked by jbudorick@chromium.org

[jbudorick, 2015-12-16 15:53:08]

The patchset sent to the CQ was uploaded after l-g-t-m from rnephew@chromium.org
Link to the patchset: https://codereview.chromium.org/1522313002/#ps60001
(title: "pull over telemetry fixes")

[commit-bot: I haz the power, 2015-12-16 15:53:19]

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

[commit-bot: I haz the power, 2015-12-16 18:27:30]

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

[commit-bot: I haz the power, 2015-12-16 18:27:31]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[jbudorick, 2015-12-16 18:37:38]

The CQ bit was checked by jbudorick@chromium.org

[jbudorick, 2015-12-16 18:37:38]

The patchset sent to the CQ was uploaded after l-g-t-m from
rnephew@chromium.org, nednguyen@google.com
Link to the patchset: https://codereview.chromium.org/1522313002/#ps80001
(title: "remove use of no-longer-existent VideoRecorder.IsStarted")

[commit-bot: I haz the power, 2015-12-16 18:38:13]

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

[aiolos (Not reviewing), 2015-12-16 21:07:04]

lgtm

[commit-bot: I haz the power, 2015-12-16 21:15:51]

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

[commit-bot: I haz the power, 2015-12-16 21:15:52]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[jbudorick, 2015-12-17 04:28:56]

The CQ bit was checked by jbudorick@chromium.org

[jbudorick, 2015-12-17 04:28:56]

The patchset sent to the CQ was uploaded after l-g-t-m from aiolos@chromium.org,
rnephew@chromium.org, nednguyen@google.com
Link to the patchset: https://codereview.chromium.org/1522313002/#ps100001
(title: "fix video_recorder timing")

[commit-bot: I haz the power, 2015-12-17 04:30:31]

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

[commit-bot: I haz the power, 2015-12-17 05:04:58]

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

[commit-bot: I haz the power, 2015-12-17 05:05:00]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[jbudorick, 2015-12-17 15:32:12]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2015-12-17 15:33:14]

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

[commit-bot: I haz the power, 2015-12-17 16:45:55]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-12-17 16:46:52]

Description was changed from

==========
[Android] Move screenshot and video_recorder into devil.

BUG=476719
==========

to

==========
[Android] Move screenshot and video_recorder into devil.

BUG=476719

Committed: https://crrev.com/688462bf65fa163b4b06104f217daf9f00cf8a6a
Cr-Commit-Position: refs/heads/master@{#365837}
==========

[commit-bot: I haz the power, 2015-12-17 16:46:53]

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