Defer running of install_commands.Installed() until necessary

Speed up device_utils's zip pushing, and don't use a constant temp file

- Defers running of a few adb commands so that they happen while the host zip is being constructed.
- Combines unzip & chmod into a single command

Reduces time to re-push all .so files from 23s->21s

BUG=520082
Committed: https://crrev.com/ff506f629332dbeb3efb3df73cf8ff5b168312e8
Cr-Commit-Position: refs/heads/master@{#347478}

[agrieve, 2015-09-01 20:27:55]

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

[jbudorick, 2015-09-01 20:33:20]

https://codereview.chromium.org/1325893002/diff/20001/build/android/pylib/dev...
File build/android/pylib/device/commands/install_commands.py (left):

https://codereview.chromium.org/1325893002/diff/20001/build/android/pylib/dev...
build/android/pylib/device/commands/install_commands.py:25: return
(all(device.FileExists('%s/%s' % (BIN_DIR, c)) for c in _COMMANDS)
If we think that FileExists doesn't handle the multiple-file case well enough,
I'd rather we either improve FileExists to support multiple files or create
another method to do the same than handroll a one-off.

[agrieve, 2015-09-02 15:34:49]

Settled on keeping the existing explicit check for unzip (although we might want
to change it to check if the file has changed like we do for md5sum_bin), but
now do the check while the local .zip is being created.

https://codereview.chromium.org/1325893002/diff/20001/build/android/pylib/dev...
File build/android/pylib/device/commands/install_commands.py (left):

https://codereview.chromium.org/1325893002/diff/20001/build/android/pylib/dev...
build/android/pylib/device/commands/install_commands.py:25: return
(all(device.FileExists('%s/%s' % (BIN_DIR, c)) for c in _COMMANDS)
On 2015/09/01 20:33:20, jbudorick wrote:
> If we think that FileExists doesn't handle the multiple-file case well enough,
> I'd rather we either improve FileExists to support multiple files or create
> another method to do the same than handroll a one-off.

Done.

[agrieve, 2015-09-04 13:56:54]

On 2015/09/02 15:34:49, agrieve wrote:
> Settled on keeping the existing explicit check for unzip (although we might
want
> to change it to check if the file has changed like we do for md5sum_bin), but
> now do the check while the local .zip is being created.
> 
>
https://codereview.chromium.org/1325893002/diff/20001/build/android/pylib/dev...
> File build/android/pylib/device/commands/install_commands.py (left):
> 
>
https://codereview.chromium.org/1325893002/diff/20001/build/android/pylib/dev...
> build/android/pylib/device/commands/install_commands.py:25: return
> (all(device.FileExists('%s/%s' % (BIN_DIR, c)) for c in _COMMANDS)
> On 2015/09/01 20:33:20, jbudorick wrote:
> > If we think that FileExists doesn't handle the multiple-file case well
enough,
> > I'd rather we either improve FileExists to support multiple files or create
> > another method to do the same than handroll a one-off.
> 
> Done.

nudge

[jbudorick, 2015-09-04 16:09:11]

https://codereview.chromium.org/1325893002/diff/60001/build/android/devil/and...
File build/android/devil/android/device_utils.py (left):

https://codereview.chromium.org/1325893002/diff/60001/build/android/devil/and...
build/android/devil/android/device_utils.py:1163: try:
What happened to the exception handler on installation failure?

https://codereview.chromium.org/1325893002/diff/60001/build/android/devil/and...
File build/android/devil/android/device_utils.py (right):

https://codereview.chromium.org/1325893002/diff/60001/build/android/devil/and...
build/android/devil/android/device_utils.py:1212: # While it's zipping, ensure
the unzip command exists on the device.
I'm not crazy about this flow... PuchChangedFilesZipped -> start zipping ->
can't unzip, so kill zipping -> PushChangedFilesIndividually is a pretty nasty
flow. I think I'd prefer if the fallback logic were handled at a higher level.

[agrieve, 2015-09-04 17:21:24]

https://codereview.chromium.org/1325893002/diff/60001/build/android/devil/and...
File build/android/devil/android/device_utils.py (left):

https://codereview.chromium.org/1325893002/diff/60001/build/android/devil/and...
build/android/devil/android/device_utils.py:1163: try:
On 2015/09/04 16:09:11, jbudorick wrote:
> What happened to the exception handler on installation failure?

Yikes! Removed it when debugging. put it back.

https://codereview.chromium.org/1325893002/diff/60001/build/android/devil/and...
File build/android/devil/android/device_utils.py (right):

https://codereview.chromium.org/1325893002/diff/60001/build/android/devil/and...
build/android/devil/android/device_utils.py:1212: # While it's zipping, ensure
the unzip command exists on the device.
On 2015/09/04 16:09:11, jbudorick wrote:
> I'm not crazy about this flow... PuchChangedFilesZipped -> start zipping ->
> can't unzip, so kill zipping -> PushChangedFilesIndividually is a pretty nasty
> flow. I think I'd prefer if the fallback logic were handled at a higher level.

Fair. Moved it up a level.

[jbudorick, 2015-09-04 17:33:27]

lgtm w/ nit

https://codereview.chromium.org/1325893002/diff/80001/build/android/devil/and...
File build/android/devil/android/device_utils.py (right):

https://codereview.chromium.org/1325893002/diff/80001/build/android/devil/and...
build/android/devil/android/device_utils.py:1158: elif not
self._PushChangedFilesZipped(files,
nit: most of this file pushes the first param down and indents four spaces, i.e.
this should be

  elif not self._PushChangedFilesZipped(
      files, [d for _, d in host_device_tuples]):

[agrieve, 2015-09-04 18:39:39]

https://codereview.chromium.org/1325893002/diff/80001/build/android/devil/and...
File build/android/devil/android/device_utils.py (right):

https://codereview.chromium.org/1325893002/diff/80001/build/android/devil/and...
build/android/devil/android/device_utils.py:1158: elif not
self._PushChangedFilesZipped(files,
On 2015/09/04 17:33:27, jbudorick wrote:
> nit: most of this file pushes the first param down and indents four spaces,
i.e.
> this should be
> 
>   elif not self._PushChangedFilesZipped(
>       files, [d for _, d in host_device_tuples]):

Done.

[agrieve, 2015-09-04 18:41:47]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2015-09-04 18:41:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbudorick@chromium.org
Link to the patchset: https://codereview.chromium.org/1325893002/#ps100001
(title: "review nit")

[commit-bot: I haz the power, 2015-09-04 18:42:58]

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

[commit-bot: I haz the power, 2015-09-04 20:11:17]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-09-04 20:11:53]

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