Better support for chrome for cros local builds.

Better support for chrome for cros local builds.

* Use ldd to detect chrome for cros local builds.
* Add browser types debug-cros and release-cros for chrome for cros on linux.
* Remove browser option --cros-desktop

BUG=229692
TEST=run_tests works with cros-debug going through the login flow.
NOTRY=True
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=194441

[achuithb, 2013-04-10 21:08:02]

Please take a look, Nat and Dave.

If you're swamped, please feel free to propose another reviewer for this and
other CLs. Thank you!

[nduca, 2013-04-11 03:54:58]

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/chrome/desktop_browser_backend.py (right):

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/desktop_browser_backend.py:19: def
__init__(self, options, executable, is_content_shell, is_cros):
can you pick something more descriptive? "needs_login"?

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py (right):

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:146: mac_canary,
False, False))
throughout, replace all implicit names with explcit.

False, False -> param1=False, param2=False

this is hard to understand otherwise

[nduca, 2013-04-11 03:57:32]

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py (right):

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:129: if
os.path.exists(app):
if not exists:
  return
unindented body

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:131: ldd =
subprocess.check_output(['/usr/bin/ldd', app])
what happens if they dont have ldd?

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:136:
AddCrOSIfFound('debug-cros', 'Debug')
can we make the linux-specific check outside of the AddCrOS?

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:138: 
is there another change coming that removes the old --use-cros-blah?

[achuithb, 2013-04-11 19:42:54]

PTAL, Nat.

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/chrome/desktop_browser_backend.py (right):

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/desktop_browser_backend.py:19: def
__init__(self, options, executable, is_content_shell, is_cros):
On 2013/04/11 03:54:58, nduca wrote:
> can you pick something more descriptive? "needs_login"?

I've changed this to use_login

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py (right):

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:129: if
os.path.exists(app):
On 2013/04/11 03:57:32, nduca wrote:
> if not exists:
>   return
> unindented body

Done.

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:131: ldd =
subprocess.check_output(['/usr/bin/ldd', app])
On 2013/04/11 03:57:32, nduca wrote:
> what happens if they dont have ldd?

Let's just skip the cros browser type in this case.

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:136:
AddCrOSIfFound('debug-cros', 'Debug')
On 2013/04/11 03:57:32, nduca wrote:
> can we make the linux-specific check outside of the AddCrOS?

Done.

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:138: 
On 2013/04/11 03:57:32, nduca wrote:
> is there another change coming that removes the old --use-cros-blah?

Sorry, forgot to include the file. Done.

https://codereview.chromium.org/14102002/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:146: mac_canary,
False, False))
On 2013/04/11 03:54:58, nduca wrote:
> throughout, replace all implicit names with explcit.
> 
> False, False -> param1=False, param2=False
> 
> this is hard to understand otherwise

I've made use_login an optional argument defaulting to False. I don't think we
want to make all the callsites of PossibleDesktopBrowser overly verbose with
is_content_shell=False, needs_login=False. Let me know if you think otherwise.

[achuithb, 2013-04-14 22:39:06]

Nat - ping?

[nduca, 2013-04-16 20:19:58]

lgtm

https://codereview.chromium.org/14102002/diff/12001/tools/telemetry/telemetry...
File tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py (right):

https://codereview.chromium.org/14102002/diff/12001/tools/telemetry/telemetry...
tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:132: app, False,
True))
you're not labeling the False, True stuff 

i mentioned that in my previous review

[achuithb, 2013-04-16 20:45:40]

Thanks for the review, Nat

https://codereview.chromium.org/14102002/diff/12001/tools/telemetry/telemetry...
File tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py (right):

https://codereview.chromium.org/14102002/diff/12001/tools/telemetry/telemetry...
tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:132: app, False,
True))
On 2013/04/16 20:19:58, nduca wrote:
> you're not labeling the False, True stuff 
> 
> i mentioned that in my previous review

Done.

[achuithb, 2013-04-16 20:47:39]

Committed patchset #4 manually as r194441 (presubmit successful).

[Julien - ping for review, 2013-04-16 22:21:00]

Reverted the change in r194466 as it was breaking the Linux Blink Perf tester.

See
http://build.chromium.org/p/chromium.webkit/builders/Linux%20Perf/builds/1989...
for the trace.

[tonyg, 2013-04-16 22:30:48]

On 2013/04/16 22:21:00, Julien Chaffraix wrote:
> Reverted the change in r194466 as it was breaking the Linux Blink Perf tester.
> 
> See
>
http://build.chromium.org/p/chromium.webkit/builders/Linux%2520Perf/builds/19...
> for the trace.

check_output() is new in python 2.7 and the bots use 2.6.