[devil] Revise how EnableRoot waits for the device to come back.

[devil] Revise how EnableRoot waits for the device to come back.

BUG=chromium:713883
Review-Url: https://codereview.chromium.org/2856253007
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/a71975b6f89dc0b61b7a12584a558572b90db174

[jbudorick, 2017-05-05 22:30:56]

Description was changed from

==========
Revise how EnableRoot waits for the device to come back.

BUG=chromium:713883
==========

to

==========
[devil] Revise how EnableRoot waits for the device to come back.

BUG=chromium:713883
==========

[jbudorick, 2017-05-05 22:31:05]

Description was changed from

==========
[devil] Revise how EnableRoot waits for the device to come back.

BUG=chromium:713883
==========

to

==========
[devil] Revise how EnableRoot waits for the device to come back.

BUG=chromium:713883
==========

[jbudorick, 2017-05-05 22:31:14]

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

[jbudorick, 2017-05-05 22:31:26]

The CQ bit was checked by jbudorick@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-05 22:31:30]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-05 23:04:13]

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

[commit-bot: I haz the power, 2017-05-05 23:04:14]

Dry run: This issue passed the CQ dry run.

[perezju, 2017-05-08 09:03:33]

https://codereview.chromium.org/2856253007/diff/1/devil/devil/android/device_...
File devil/devil/android/device_utils.py (left):

https://codereview.chromium.org/2856253007/diff/1/devil/devil/android/device_...
devil/devil/android/device_utils.py:459: self.WaitUntilFullyBooted()
Hmm, I would prefer if the fix is done _within_ WaitUntilFullyBooted; there
might be other places where that method is used, and the same kind of race might
occur?

[jbudorick, 2017-05-08 13:21:26]

https://codereview.chromium.org/2856253007/diff/1/devil/devil/android/device_...
File devil/devil/android/device_utils.py (left):

https://codereview.chromium.org/2856253007/diff/1/devil/devil/android/device_...
devil/devil/android/device_utils.py:459: self.WaitUntilFullyBooted()
On 2017/05/08 09:03:33, perezju wrote:
> Hmm, I would prefer if the fix is done _within_ WaitUntilFullyBooted; there
> might be other places where that method is used, and the same kind of race
might
> occur?

I think that waiting for the device to go down is out of the scope of
WaitUntilFullyBooted. I think Reboot handles this the way I'd expect -- wait for
the device to fall offline, then call WaitUntilFullyBooted.

[perezju, 2017-05-08 14:08:06]

On 2017/05/08 13:21:26, jbudorick wrote:
>
https://codereview.chromium.org/2856253007/diff/1/devil/devil/android/device_...
> File devil/devil/android/device_utils.py (left):
> 
>
https://codereview.chromium.org/2856253007/diff/1/devil/devil/android/device_...
> devil/devil/android/device_utils.py:459: self.WaitUntilFullyBooted()
> On 2017/05/08 09:03:33, perezju wrote:
> > Hmm, I would prefer if the fix is done _within_ WaitUntilFullyBooted; there
> > might be other places where that method is used, and the same kind of race
> might
> > occur?
> 
> I think that waiting for the device to go down is out of the scope of
> WaitUntilFullyBooted. I think Reboot handles this the way I'd expect -- wait
for
> the device to fall offline, then call WaitUntilFullyBooted.

ok, I guess that's reasonable.

lgtm

[jbudorick, 2017-05-08 14:10:54]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2017-05-08 14:11:05]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-08 14:34:35]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1494252654211380, "parent_rev":
"643994eea8e5d5421441168190e8499c9cbf7ae8", "commit_rev":
"a71975b6f89dc0b61b7a12584a558572b90db174"}

[commit-bot: I haz the power, 2017-05-08 14:34:38]

Description was changed from

==========
[devil] Revise how EnableRoot waits for the device to come back.

BUG=chromium:713883
==========

to

==========
[devil] Revise how EnableRoot waits for the device to come back.

BUG=chromium:713883

Review-Url: https://codereview.chromium.org/2856253007
Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2017-05-08 14:34:39]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...