re-enable startup benchmarks on Android

re-enable startup benchmarks on Android

(trybots are probably useless anyway)

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

[pasko, 2014-07-17 12:39:02]

Tony, it seems that these tests were disabled by accident in your change
(https://codereview.chromium.org/335923007)

would it be enough of a change to make the tests enabled? how do I locally check
that it works?

[tonyg, 2014-07-17 14:29:22]

https://codereview.chromium.org/399053004/diff/1/tools/perf/benchmarks/startu...
File tools/perf/benchmarks/startup.py (left):

https://codereview.chromium.org/399053004/diff/1/tools/perf/benchmarks/startu...
tools/perf/benchmarks/startup.py:28: @benchmark.Disabled('android',
'snowleopard') # crbug.com/336913
Chrome on android doesn't support themes or extensions. Why would it run the
below 4?

https://codereview.chromium.org/399053004/diff/1/tools/perf/benchmarks/startu...
File tools/perf/benchmarks/startup.py (right):

https://codereview.chromium.org/399053004/diff/1/tools/perf/benchmarks/startu...
tools/perf/benchmarks/startup.py:10: @benchmark.Disabled('snowleopard') #
crbug.com/336913
Does this benchmark open multiple tabs? If so, it needs to retain an
@benchmark.Enabled('has tabs').

[pasko, 2014-07-18 12:37:02]

PTAL

https://codereview.chromium.org/399053004/diff/1/tools/perf/benchmarks/startu...
File tools/perf/benchmarks/startup.py (left):

https://codereview.chromium.org/399053004/diff/1/tools/perf/benchmarks/startu...
tools/perf/benchmarks/startup.py:28: @benchmark.Disabled('android',
'snowleopard') # crbug.com/336913
On 2014/07/17 14:29:22, tonyg wrote:
> Chrome on android doesn't support themes or extensions. Why would it run the
> below 4?

sorry, that was mechanical, Undone.

https://codereview.chromium.org/399053004/diff/1/tools/perf/benchmarks/startu...
File tools/perf/benchmarks/startup.py (right):

https://codereview.chromium.org/399053004/diff/1/tools/perf/benchmarks/startu...
tools/perf/benchmarks/startup.py:10: @benchmark.Disabled('snowleopard') #
crbug.com/336913
On 2014/07/17 14:29:22, tonyg wrote:
> Does this benchmark open multiple tabs? If so, it needs to retain an
> @benchmark.Enabled('has tabs').

I think it does not open multiple tabs, it's a blank page startup after all.

It's hard to me to read the magic in the decorator code, is there any
document/comment/helpmessage explaining what arguments for @benchmark.Enabled
are supported and what effect they cause? can I have bothe @benchmark.Enable and
@benchmark.Disable decorators for one method? Is a native New-Tab-Page on
Android considered a tab in the sense of Enabled('has tabs')?

[tonyg, 2014-07-18 14:58:08]

lgtm

https://codereview.chromium.org/399053004/diff/20001/tools/perf/benchmarks/st...
File tools/perf/benchmarks/startup.py (right):

https://codereview.chromium.org/399053004/diff/20001/tools/perf/benchmarks/st...
tools/perf/benchmarks/startup.py:10: @benchmark.Disabled('snowleopard') #
crbug.com/336913
Actually, thinking about this more, let's add the @benchmark.Enabled('has tabs')
above this annotation. That's because in practice full Chrome has tabs and we
care about startup there. Chrome Shell doesn't have tabs and I don't think we
want to run startup benchmarks on it because they'd differ too far from reality.

If you agree, please add it. If not, then please also remove it from
StartupWarmBlankPage as well as that should be consistent with this.

[pasko, 2014-07-18 15:28:35]

https://codereview.chromium.org/399053004/diff/20001/tools/perf/benchmarks/st...
File tools/perf/benchmarks/startup.py (right):

https://codereview.chromium.org/399053004/diff/20001/tools/perf/benchmarks/st...
tools/perf/benchmarks/startup.py:10: @benchmark.Disabled('snowleopard') #
crbug.com/336913
On 2014/07/18 14:58:07, tonyg wrote:
> Actually, thinking about this more, let's add the @benchmark.Enabled('has
tabs')
> above this annotation. That's because in practice full Chrome has tabs and we
> care about startup there. Chrome Shell doesn't have tabs and I don't think we
> want to run startup benchmarks on it because they'd differ too far from
reality.

Sure, adding Enabled('has tabs') since this test is interesting for the full
chrome only. Done.

Is the absence of 'has tabs' a synonym of 'run only in testshell/contentshell'
or there is more to 'has tabs'?

[pasko, 2014-07-18 15:28:43]

The CQ bit was checked by pasko@chromium.org

[commit-bot: I haz the power, 2014-07-18 15:29:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pasko@chromium.org/399053004/40001

[tonyg, 2014-07-18 15:33:28]

> Is the absence of 'has tabs' a synonym of 'run only in testshell/contentshell'
> or there is more to 'has tabs'?

Currently, !'has tabs' is equivalent to testshell/contentshell/webview/etc. and
'has tabs' to chrome. So it isn't exactly the right semantics, but likely to be
good enough for the foreseeable future.

[commit-bot: I haz the power, 2014-07-18 17:56:00]

Change committed as 284151