[Android] Move screenshot implementation back into AndroidCommands.

[Android] Move screenshot implementation back into AndroidCommands.

BUG=267773
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=264674

[jbudorick, 2014-04-11 16:38:56]

See the description in https://codereview.chromium.org/234533003/

[jbudorick, 2014-04-17 15:56:47]

ping

[cjhopman, 2014-04-17 16:18:47]

On 2014/04/17 15:56:47, jbudorick wrote:
> ping

Does it make more sense considering the refactoring of this stuff (that I,
admittedly, don't know much about) to just remove AndroidCommands.TakeScreenshot
and update all the callers to use the one in screenshot.py?

[jbudorick, 2014-04-17 16:29:46]

On 2014/04/17 16:18:47, cjhopman wrote:
> On 2014/04/17 15:56:47, jbudorick wrote:
> > ping
> 
> Does it make more sense considering the refactoring of this stuff (that I,
> admittedly, don't know much about) to just remove
AndroidCommands.TakeScreenshot
> and update all the callers to use the one in screenshot.py?

I don't think so (though I'd be happy to hear Craig's opinion on this). The
entire goal of the DeviceUtils class is (or will be) to provide this kind of
high(er) level utility built on adb. Given that, I think moving TakeScreenshot's
implementation back into AndroidCommands will make the eventual transition less
painful.

[craigdh, 2014-04-17 16:31:02]

lgtm with optional suggestion.

Generally I think it makes sense to have a single interface for all device
utilities, rather than having random scripts like screenshot.py.

https://codereview.chromium.org/234803006/diff/1/build/android/pylib/android_...
File build/android/pylib/android_commands.py (right):

https://codereview.chromium.org/234803006/diff/1/build/android/pylib/android_...
build/android/pylib/android_commands.py:1723: def TakeScreenshot(self,
host_file):
should host_file default to None?

[jbudorick, 2014-04-17 16:47:40]

On 2014/04/17 16:31:02, craigdh wrote:
> lgtm with optional suggestion.
> 
> Generally I think it makes sense to have a single interface for all device
> utilities, rather than having random scripts like screenshot.py.
> 
>
https://codereview.chromium.org/234803006/diff/1/build/android/pylib/android_...
> File build/android/pylib/android_commands.py (right):
> 
>
https://codereview.chromium.org/234803006/diff/1/build/android/pylib/android_...
> build/android/pylib/android_commands.py:1723: def TakeScreenshot(self,
> host_file):
> should host_file default to None?

Sounds reasonable to me, both here and (eventually) in DeviceUtils.

[jbudorick, 2014-04-17 17:35:19]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2014-04-17 17:35:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbudorick@chromium.org/234803006/20001

[commit-bot: I haz the power, 2014-04-17 21:30:15]

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

[commit-bot: I haz the power, 2014-04-17 21:30:16]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_rel

[jbudorick, 2014-04-17 21:38:08]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2014-04-17 21:39:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbudorick@chromium.org/234803006/20001

[commit-bot: I haz the power, 2014-04-17 23:46:37]

Change committed as 264674