[Telemetry] Default to most recent local build if --browser is omitted.

[Telemetry] Add support for picking a default browser if --browser is omitted.
    
On desktop we default to the most recent local build (if one exists).
On CrOS we default to system (if running on device).

Also:
- Remove --browser=any, I don't think that was ever documented or used.
- Clean up user-error messages so they don't dump messy stacks.

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

[achuithb, 2013-07-23 23:58:39]

https://codereview.chromium.org/19972003/diff/2001/tools/telemetry/telemetry/...
File tools/telemetry/telemetry/core/browser_finder.py (right):

https://codereview.chromium.org/19972003/diff/2001/tools/telemetry/telemetry/...
tools/telemetry/telemetry/core/browser_finder.py:24: def
_FindMostRecentLocalBrowser(possible_browsers):
This function seems like it should be in desktop_browser_finder, since the
concept of local builds is a desktop thing?

Why not rename this to FindDefaultBrowser or something, and allow all
browser_finders to implement it as they wish?

cros_browser_finder will want a default too ('system')

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

https://codereview.chromium.org/19972003/diff/2001/tools/telemetry/telemetry/...
tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:158: False,
False))
We might be at a point where being explicit about the arguments may be a good
idea. Alternatively default to False?

[tonyg, 2013-07-24 00:46:11]

Excellent code review comments! All done.

https://codereview.chromium.org/19972003/diff/2001/tools/telemetry/telemetry/...
File tools/telemetry/telemetry/core/browser_finder.py (right):

https://codereview.chromium.org/19972003/diff/2001/tools/telemetry/telemetry/...
tools/telemetry/telemetry/core/browser_finder.py:24: def
_FindMostRecentLocalBrowser(possible_browsers):
On 2013/07/23 23:58:39, achuith.bhandarkar wrote:
> This function seems like it should be in desktop_browser_finder, since the
> concept of local builds is a desktop thing?
> 
> Why not rename this to FindDefaultBrowser or something, and allow all
> browser_finders to implement it as they wish?
> 
> cros_browser_finder will want a default too ('system')
> 

Done.

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

https://codereview.chromium.org/19972003/diff/2001/tools/telemetry/telemetry/...
tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:158: False,
False))
On 2013/07/23 23:58:39, achuith.bhandarkar wrote:
> We might be at a point where being explicit about the arguments may be a good
> idea. Alternatively default to False?

Done.

[dtu, 2013-07-24 01:13:36]

lgtm

[achuithb, 2013-07-24 01:14:07]

You may need to re-upload the patch. I'm having difficulty viewing the diffs.

[tonyg, 2013-07-24 01:19:57]

On 2013/07/24 01:14:07, achuith.bhandarkar wrote:
> You may need to re-upload the patch. I'm having difficulty viewing the diffs.

Should be better now. PTAL

[achuithb, 2013-07-24 01:20:07]

I might be going overboard with trying to find a good default in all cases.
Ignore if these suggestions don't seem worthwhile.

lgtm otherwise.

https://codereview.chromium.org/19972003/diff/8001/tools/telemetry/telemetry/...
File tools/telemetry/telemetry/core/browser_finder.py (right):

https://codereview.chromium.org/19972003/diff/8001/tools/telemetry/telemetry/...
tools/telemetry/telemetry/core/browser_finder.py:15: cros_browser_finder
Why not reverse this? If the user specifies --remote but no browser, I think
cros-chrome should be the default, no?

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

https://codereview.chromium.org/19972003/diff/8001/tools/telemetry/telemetry/...
tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:108: if
b.is_local_build]
Why not default to the system browser if there are no local builds?

[tonyg, 2013-07-24 01:35:21]

On 2013/07/24 01:20:07, achuith.bhandarkar wrote:
> I might be going overboard with trying to find a good default in all cases.
> Ignore if these suggestions don't seem worthwhile.
> 
> lgtm otherwise.
> 
>
https://codereview.chromium.org/19972003/diff/8001/tools/telemetry/telemetry/...
> File tools/telemetry/telemetry/core/browser_finder.py (right):
> 
>
https://codereview.chromium.org/19972003/diff/8001/tools/telemetry/telemetry/...
> tools/telemetry/telemetry/core/browser_finder.py:15: cros_browser_finder
> Why not reverse this? If the user specifies --remote but no browser, I think
> cros-chrome should be the default, no?

Do you mean also default to cros-chrome or only default to cros-chrome?
> 
>
https://codereview.chromium.org/19972003/diff/8001/tools/telemetry/telemetry/...
> File tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py (right):
> 
>
https://codereview.chromium.org/19972003/diff/8001/tools/telemetry/telemetry/...
> tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:108: if
> b.is_local_build]
> Why not default to the system browser if there are no local builds?

We talked about this in the bug. Blink developers are used to have their perf
scripts choose the most recently built browsers and that makes a lot of sense to
me for chromium in general. However, we didn't want to go picking a system
browser by default because we don't want to unexpectedly run on the wrong
browser.

[achuithb, 2013-07-24 02:36:57]

On 2013/07/24 01:35:21, tonyg wrote:
> On 2013/07/24 01:20:07, achuith.bhandarkar wrote:
> > I might be going overboard with trying to find a good default in all cases.
> > Ignore if these suggestions don't seem worthwhile.
> > 
> > lgtm otherwise.
> > 
> >
>
https://codereview.chromium.org/19972003/diff/8001/tools/telemetry/telemetry/...
> > File tools/telemetry/telemetry/core/browser_finder.py (right):
> > 
> >
>
https://codereview.chromium.org/19972003/diff/8001/tools/telemetry/telemetry/...
> > tools/telemetry/telemetry/core/browser_finder.py:15: cros_browser_finder
> > Why not reverse this? If the user specifies --remote but no browser, I think
> > cros-chrome should be the default, no?
> 
> Do you mean also default to cros-chrome or only default to cros-chrome?

Let's not worry about this. 

>
https://codereview.chromium.org/19972003/diff/8001/tools/telemetry/telemetry/...
> > File tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py
(right):
> > 
> >
>
https://codereview.chromium.org/19972003/diff/8001/tools/telemetry/telemetry/...
> > tools/telemetry/telemetry/core/chrome/desktop_browser_finder.py:108: if
> > b.is_local_build]
> > Why not default to the system browser if there are no local builds?
> 
> We talked about this in the bug. Blink developers are used to have their perf
> scripts choose the most recently built browsers and that makes a lot of sense
to
> me for chromium in general. However, we didn't want to go picking a system
> browser by default because we don't want to unexpectedly run on the wrong
> browser.

Sure.

lgtm

[commit-bot: I haz the power, 2013-07-24 02:42:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tonyg@chromium.org/19972003/23001

[commit-bot: I haz the power, 2013-07-24 10:54:24]

Change committed as 213401