Try to enable root when starting adb, as root is needed for a full coverage.

Try to enable root when starting adb, as root is needed for a full coverage.

BUG=375178
R=tonyg@chromium.org

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

[tonyg, 2014-05-20 11:53:37]

lgtm

https://codereview.chromium.org/294943002/diff/1/tools/telemetry/telemetry/co...
File tools/telemetry/telemetry/core/backends/chrome/android_browser_finder.py
(right):

https://codereview.chromium.org/294943002/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/backends/chrome/android_browser_finder.py:203: if
not adb.IsRootEnabled():
Is this check even necessary? I think the below will just be a no-op if it
already has root, right?

[qsr, 2014-05-20 11:58:31]

https://chromiumcodereview.appspot.com/294943002/diff/1/tools/telemetry/telem...
File tools/telemetry/telemetry/core/backends/chrome/android_browser_finder.py
(right):

https://chromiumcodereview.appspot.com/294943002/diff/1/tools/telemetry/telem...
tools/telemetry/telemetry/core/backends/chrome/android_browser_finder.py:203: if
not adb.IsRootEnabled():
On 2014/05/20 11:53:38, tonyg wrote:
> Is this check even necessary? I think the below will just be a no-op if it
> already has root, right?

 Yes, but it does a few SendCommand, and maybe a shell restart, so cutting that
seemed a good idea. If you prefer I remove it, I can.

[tonyg, 2014-05-20 11:59:51]

On 2014/05/20 11:58:31, qsr wrote:
>
https://chromiumcodereview.appspot.com/294943002/diff/1/tools/telemetry/telem...
> File tools/telemetry/telemetry/core/backends/chrome/android_browser_finder.py
> (right):
> 
>
https://chromiumcodereview.appspot.com/294943002/diff/1/tools/telemetry/telem...
> tools/telemetry/telemetry/core/backends/chrome/android_browser_finder.py:203:
if
> not adb.IsRootEnabled():
> On 2014/05/20 11:53:38, tonyg wrote:
> > Is this check even necessary? I think the below will just be a no-op if it
> > already has root, right?
> 
>  Yes, but it does a few SendCommand, and maybe a shell restart, so cutting
that
> seemed a good idea. If you prefer I remove it, I can.

Up to you. My preference is for whichever is the fastest in the case where root
is already enabled (which will always be the case on the bots).

[qsr, 2014-05-20 12:01:14]

On 2014/05/20 11:59:51, tonyg wrote:
> On 2014/05/20 11:58:31, qsr wrote:
> >
>
https://chromiumcodereview.appspot.com/294943002/diff/1/tools/telemetry/telem...
> > File
tools/telemetry/telemetry/core/backends/chrome/android_browser_finder.py
> > (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/294943002/diff/1/tools/telemetry/telem...
> >
tools/telemetry/telemetry/core/backends/chrome/android_browser_finder.py:203:
> if
> > not adb.IsRootEnabled():
> > On 2014/05/20 11:53:38, tonyg wrote:
> > > Is this check even necessary? I think the below will just be a no-op if it
> > > already has root, right?
> > 
> >  Yes, but it does a few SendCommand, and maybe a shell restart, so cutting
> that
> > seemed a good idea. If you prefer I remove it, I can.
> 
> Up to you. My preference is for whichever is the fastest in the case where
root
> is already enabled (which will always be the case on the bots).

 Then I'm pretty sure that checking is quicker. We can even makes it a little
bit quicker by memorizing the result, as this might be used again later on...

[qsr, 2014-05-20 12:01:17]

The CQ bit was checked by qsr@chromium.org

[commit-bot: I haz the power, 2014-05-20 12:01:42]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/qsr@chromium.org/294943002/1

[commit-bot: I haz the power, 2014-05-20 17:23:43]

Change committed as 271680