Organize adb install cmds and reboot on failure

Organize adb install cmds and reboot on failure

- Rewrote adb_install_content_shell to not call adb install directly
- New function in android_commands 'ManagedInstall' which attempts
  to work around package manager freezes by restarting device

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

[bulach, 2012-08-09 12:36:52]

lgtm, thanks isaac! however, I'd really like to avoid forking third party code..

the suggestion is not the cleanest one, but at least avoids changing their code.

a few nits below:

https://chromiumcodereview.appspot.com/10824227/diff/2001/build/android/adb_i...
File build/android/adb_install_content_shell (right):

https://chromiumcodereview.appspot.com/10824227/diff/2001/build/android/adb_i...
build/android/adb_install_content_shell:1: #!/usr/bin/env python
nice! :)

https://chromiumcodereview.appspot.com/10824227/diff/2001/build/android/pylib...
File build/android/pylib/android_commands.py (right):

https://chromiumcodereview.appspot.com/10824227/diff/2001/build/android/pylib...
build/android/pylib/android_commands.py:318: reinstall: Whether to reinstall
over existing package
nit: remove retry_count from 313, since it's undocumented and unused?

https://chromiumcodereview.appspot.com/10824227/diff/2001/build/android/pylib...
build/android/pylib/android_commands.py:331: timeout_time=2*60,
cmd_logger=logging,
nit: I think it needs space around *, timeout_time=2 * 60

https://chromiumcodereview.appspot.com/10824227/diff/2001/build/android/pylib...
build/android/pylib/android_commands.py:342: package_name: Package name (only
needed if keep_data=False
nit: )

https://chromiumcodereview.appspot.com/10824227/diff/2001/build/android/pylib...
build/android/pylib/android_commands.py:357: if reboots_left < 0:
nit: seems that this should be <= 0, that is, passing reboots_on_failure=0
should never reboot.. also, 360 would need to be == 1

https://chromiumcodereview.appspot.com/10824227/diff/2001/build/android/pylib...
build/android/pylib/android_commands.py:360:
self.Reboot(full_reboot=(reboots_left==0))
nit: space around ==

https://chromiumcodereview.appspot.com/10824227/diff/2001/third_party/android...
File third_party/android_testrunner/README.chromium (right):

https://chromiumcodereview.appspot.com/10824227/diff/2001/third_party/android...
third_party/android_testrunner/README.chromium:16: 3. Added optional cmd_logger
arguments to |SendCommand| and |SendShellCommand|
hmm... I'm not sure this would be acceptable on the upstream of this repository
since it already contains a logging mechanim.. and it'd be nice to avoid
forking... :)

since both of them already log via their logger.SilentLog, which we can turn on
via logger.SetVerbose(), how about just add that on the calling code?

https://chromiumcodereview.appspot.com/10824227/diff/2001/third_party/android...
File third_party/android_testrunner/adb_interface.py (right):

https://chromiumcodereview.appspot.com/10824227/diff/2001/third_party/android...
third_party/android_testrunner/adb_interface.py:62: retry_count: number of times
to retry command before raising
new param requires docstring..

[cmp, 2012-08-10 02:23:07]

lgtm with bulach's comments, thanks

[Isaac (away), 2012-08-10 04:44:35]

The adb_interface class isn't doing anything particularly complex and if
SendShellCommand isn't acting the way we want, I don't share as many concerns
forking it and never merging in updates to this function.  But the changes were
just for clarity so I'm happy to remove them.

Changed adb_install_content_shell to install in parallel.

https://chromiumcodereview.appspot.com/10824227/diff/2001/build/android/adb_i...
File build/android/adb_install_content_shell (right):

https://chromiumcodereview.appspot.com/10824227/diff/2001/build/android/adb_i...
build/android/adb_install_content_shell:1: #!/usr/bin/env python
On 2012/08/09 12:36:52, bulach wrote:
> nice! :)

I thought you'd like that :-)

https://chromiumcodereview.appspot.com/10824227/diff/2001/build/android/pylib...
File build/android/pylib/android_commands.py (right):

https://chromiumcodereview.appspot.com/10824227/diff/2001/build/android/pylib...
build/android/pylib/android_commands.py:331: timeout_time=2*60,
cmd_logger=logging,
It's like this a couple other places in this file.  You sure I should change?

On 2012/08/09 12:36:52, bulach wrote:
> nit: I think it needs space around *, timeout_time=2 * 60

https://chromiumcodereview.appspot.com/10824227/diff/2001/third_party/android...
File third_party/android_testrunner/README.chromium (right):

https://chromiumcodereview.appspot.com/10824227/diff/2001/third_party/android...
third_party/android_testrunner/README.chromium:16: 3. Added optional cmd_logger
arguments to |SendCommand| and |SendShellCommand|
On 2012/08/09 12:36:52, bulach wrote:
> hmm... I'm not sure this would be acceptable on the upstream of this
repository
> since it already contains a logging mechanim.. and it'd be nice to avoid
> forking... :)
> 
> since both of them already log via their logger.SilentLog, which we can turn
on
> via logger.SetVerbose(), how about just add that on the calling code?

I didn't do that because I'm not sure about the side effects of enabling the
silentlogger and don't want to enable and disable for each command because it
significantly complicates design..  I will just undo these changes I guess.

[Isaac (away), 2012-08-10 06:47:49]

Not sure why this is breaking.  I added a sanity check to fail if the android
command could not be sent.  It appears this happened in the tryjob right after a
call to adb root -- maybe adb root is starting adb even though it can't run as
root?

[bulach, 2012-08-10 11:47:07]

lgtm, just some nits..

as for adb_interface: I understand your point of not merging it now as it's
really simple, but we can't guarantee the future :) 
the android team may change for whatever reason they have: is this API stable?
and what about the underlying implementation? anyways, if keep their code
unchanged, it'll make picking up their changes in the future less painful, I
guess..

https://chromiumcodereview.appspot.com/10824227/diff/8001/build/android/pylib...
File build/android/pylib/android_commands.py (right):

https://chromiumcodereview.appspot.com/10824227/diff/8001/build/android/pylib...
build/android/pylib/android_commands.py:331: timeout_time=2*60,
cmd_logger=logging,
nit: remove cmd_logger? also, spaces: 2 * 60

[Isaac (away), 2012-08-17 04:41:49]

https://chromiumcodereview.appspot.com/10824227/diff/8001/build/android/pylib...
File build/android/pylib/android_commands.py (right):

https://chromiumcodereview.appspot.com/10824227/diff/8001/build/android/pylib...
build/android/pylib/android_commands.py:331: timeout_time=2*60,
cmd_logger=logging,
On 2012/08/10 11:47:07, bulach wrote:
> nit: remove cmd_logger? also, spaces: 2 * 60

removed cmd_logger.

as far as spacing, would rather not create inconstancy and pylint/gyplint isn't
complaining.

[commit-bot: I haz the power, 2012-08-17 04:59:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ilevy@chromium.org/10824227/1007