[Telemetry] Add a page set option to use all cold page loads.

[Telemetry] Add a PageTest option to clear cache before each run.

This will be used for the network simulation page cycler where we need to
ensure that each page load requests the resources from WPR instead of from
the browser cache.

BUG=None
TEST=top_25 page cycler on linux
NOTRY=True

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

[nduca, 2013-05-31 20:42:41]

some thoughts

https://codereview.chromium.org/16154007/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/browser.py (right):

https://codereview.chromium.org/16154007/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/browser.py:232: def ClearCache(self):
what if the browser doesn't support these methods? Do we need supports_xxx
methods for both?

https://codereview.chromium.org/16154007/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/browser_options.py (right):

https://codereview.chromium.org/16154007/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/browser_options.py:130:
group.add_option('--cold-page-loads', action='store_true',
lets not expose this to the command line. in keeping with the new world order,
the benchmark object would set this i think?

[tonyg, 2013-06-03 22:27:58]

https://codereview.chromium.org/16154007/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/browser.py (right):

https://codereview.chromium.org/16154007/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/browser.py:232: def ClearCache(self):
On 2013/05/31 20:42:41, nduca wrote:
> what if the browser doesn't support these methods? Do we need supports_xxx
> methods for both?

My thought is that we consider them required and add supports_xxx methods if we
have to add a browser that doesn't support them.

https://codereview.chromium.org/16154007/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/browser_options.py (right):

https://codereview.chromium.org/16154007/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/browser_options.py:130:
group.add_option('--cold-page-loads', action='store_true',
On 2013/05/31 20:42:41, nduca wrote:
> lets not expose this to the command line. in keeping with the new world order,
> the benchmark object would set this i think?

It needs to be exposed on the run_measurement command line in order for the
Benchmark to set it.

[nduca, 2013-06-03 23:29:38]

You can expose an option as a field on the browser options object but not as an
add_option(...) method...

[tonyg, 2013-06-03 23:46:28]

On 2013/06/03 23:29:38, nduca wrote:
> You can expose an option as a field on the browser options object but not as
an
> add_option(...) method...

But dtu's run_benchmark patch calls run_benchmark with subprocess.popen so
there'd be no way of directly manipulating browser_options.

[dtu, 2013-06-03 23:57:17]

On 2013/06/03 23:46:28, tonyg wrote:
> On 2013/06/03 23:29:38, nduca wrote:
> > You can expose an option as a field on the browser options object but not as
> an
> > add_option(...) method...
> 
> But dtu's run_benchmark patch calls run_benchmark with subprocess.popen so
> there'd be no way of directly manipulating browser_options.

That's going to change. It'll probably end up calling PageRunner.Run() directly.

[nduca, 2013-06-04 00:42:29]

Yeplz.

[tonyg, 2013-06-04 18:29:06]

https://codereview.chromium.org/16154007/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/browser_options.py (right):

https://codereview.chromium.org/16154007/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/browser_options.py:130:
group.add_option('--cold-page-loads', action='store_true',
On 2013/06/03 22:27:58, tonyg wrote:
> On 2013/05/31 20:42:41, nduca wrote:
> > lets not expose this to the command line. in keeping with the new world
order,
> > the benchmark object would set this i think?
> 
> It needs to be exposed on the run_measurement command line in order for the
> Benchmark to set it.

OK, I removed this from browser_options. Temporarily I've added it to
page_cycler.py so that the netsim page cycler can move forward in parallel to
the run_benchmark work. Once we have a benchmark that sets the option, we can
remove the option from page_cycler too.

[tonyg, 2013-06-06 22:41:50]

On 2013/06/04 18:29:06, tonyg wrote:
>
https://codereview.chromium.org/16154007/diff/1/tools/telemetry/telemetry/cor...
> File tools/telemetry/telemetry/core/browser_options.py (right):
> 
>
https://codereview.chromium.org/16154007/diff/1/tools/telemetry/telemetry/cor...
> tools/telemetry/telemetry/core/browser_options.py:130:
> group.add_option('--cold-page-loads', action='store_true',
> On 2013/06/03 22:27:58, tonyg wrote:
> > On 2013/05/31 20:42:41, nduca wrote:
> > > lets not expose this to the command line. in keeping with the new world
> order,
> > > the benchmark object would set this i think?
> > 
> > It needs to be exposed on the run_measurement command line in order for the
> > Benchmark to set it.
> 
> OK, I removed this from browser_options. Temporarily I've added it to
> page_cycler.py so that the netsim page cycler can move forward in parallel to
> the run_benchmark work. Once we have a benchmark that sets the option, we can
> remove the option from page_cycler too.

Dave, Nat, what do you guys think now?

[nduca, 2013-06-06 22:49:04]

some concerns, i'm afraid

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
File tools/telemetry/telemetry/core/browser.py (right):

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
tools/telemetry/telemetry/core/browser.py:228: def CloseConnections(self):
i recall asking if these were supported on all browsers. are they? and will
these work if you dont pass the right arguments to the browser? how do you
guarantee it..?

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
File tools/telemetry/telemetry/core/browser_options.py (right):

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
tools/telemetry/telemetry/core/browser_options.py:36: self.cold_page_loads =
False
this doesn't sound like a browser option to me... it sounds like a page test
preference  a la page_test.requiresBrowserRestartAfterEachRun

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
File tools/telemetry/telemetry/core/chrome/browser_backend.py (right):

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
tools/telemetry/telemetry/core/chrome/browser_backend.py:247: except
exceptions.EvaluateException:
this seems pretty funky, how do you know that this is supported? You really
should be thinking through this either being on all browsers, or doing is an
is_clear_cache_supported in the style of, for instance, screenshot.

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
File tools/telemetry/telemetry/page/page_runner.py (right):

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
tools/telemetry/telemetry/page/page_runner.py:145: if options.cold_page_loads:
i'm a little confused about how you're saying clearing cache implies a close of
connections. That doesn't feel orthogonal to me. Why not close connections
always and cache if requested by the page?

[tonyg, 2013-06-20 02:42:29]

This uses devtools now for cache clearing. Should be ready for another look.

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
File tools/telemetry/telemetry/core/browser.py (right):

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
tools/telemetry/telemetry/core/browser.py:228: def CloseConnections(self):
On 2013/06/06 22:49:04, nduca wrote:
> i recall asking if these were supported on all browsers. are they? and will
> these work if you dont pass the right arguments to the browser? how do you
> guarantee it..?

I switched this to use devtools' cache clearing method. If devtools tells us it
isn't supported, I raise an exception.

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
File tools/telemetry/telemetry/core/browser_options.py (right):

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
tools/telemetry/telemetry/core/browser_options.py:36: self.cold_page_loads =
False
On 2013/06/06 22:49:04, nduca wrote:
> this doesn't sound like a browser option to me... it sounds like a page test
> preference  a la page_test.requiresBrowserRestartAfterEachRun

To me it seems to fit in with the page set options here. Like pageset-repeat
page-filter page-repeat, etc. We also can only customize BrowserOptions from the
Measurement and we only have access to the BrowserOptions in the place in the
PageRunner where we clear the cache. Is this okay to leave here or do you feel
strongly about moving it out?

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
File tools/telemetry/telemetry/core/chrome/browser_backend.py (right):

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
tools/telemetry/telemetry/core/chrome/browser_backend.py:247: except
exceptions.EvaluateException:
On 2013/06/06 22:49:04, nduca wrote:
> this seems pretty funky, how do you know that this is supported? You really
> should be thinking through this either being on all browsers, or doing is an
> is_clear_cache_supported in the style of, for instance, screenshot.

This is gone now.

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
File tools/telemetry/telemetry/page/page_runner.py (right):

https://codereview.chromium.org/16154007/diff/10001/tools/telemetry/telemetry...
tools/telemetry/telemetry/page/page_runner.py:145: if options.cold_page_loads:
On 2013/06/06 22:49:04, nduca wrote:
> i'm a little confused about how you're saying clearing cache implies a close
of
> connections. That doesn't feel orthogonal to me. Why not close connections
> always and cache if requested by the page?

Good point. Fixed.

[nduca, 2013-06-20 19:54:15]

im sorry

https://codereview.chromium.org/16154007/diff/19001/tools/perf/perf_tools/pag...
File tools/perf/perf_tools/page_cycler.py (right):

https://codereview.chromium.org/16154007/diff/19001/tools/perf/perf_tools/pag...
tools/perf/perf_tools/page_cycler.py:39: parser.add_option('--cold-page-loads',
action='store_true',
rather you not expose it as a commandline. just block your bot hookup of this on
dtus benchmark objects.

https://codereview.chromium.org/16154007/diff/19001/tools/telemetry/telemetry...
File tools/telemetry/telemetry/core/browser_options.py (right):

https://codereview.chromium.org/16154007/diff/19001/tools/telemetry/telemetry...
tools/telemetry/telemetry/core/browser_options.py:36: self.cold_page_loads =
False
we have access to the test object in page_runner.

browser_options should only have options that are used by the backends. this
isn't used by them, so shouldn't be there. i'm sorry to be cantankerous.

https://codereview.chromium.org/16154007/diff/19001/tools/telemetry/telemetry...
File tools/telemetry/telemetry/page/page_runner.py (right):

https://codereview.chromium.org/16154007/diff/19001/tools/telemetry/telemetry...
tools/telemetry/telemetry/page/page_runner.py:236: util.CloseConnections(tab)
and this for that matter shoudl move into page state or clean up page

https://codereview.chromium.org/16154007/diff/19001/tools/telemetry/telemetry...
tools/telemetry/telemetry/page/page_runner.py:237: if options.cold_page_loads:
lets make this part of clean up page

also, to ask a question.... what semantic of cold_page_loads is implied if we
load a site in two tabs? Should thiss be part of prepare, rather than after? In
the world of multi tab, you wouldn't clean up page until after the page was
completely gone -- meaning going to google.com in two tabs would hit the cache
on the second tab, in the case that you clear cache after the test runs....

[tonyg, 2013-06-21 22:36:18]

All comments addressed. This does look a lot cleaner now.

https://codereview.chromium.org/16154007/diff/19001/tools/perf/perf_tools/pag...
File tools/perf/perf_tools/page_cycler.py (right):

https://codereview.chromium.org/16154007/diff/19001/tools/perf/perf_tools/pag...
tools/perf/perf_tools/page_cycler.py:39: parser.add_option('--cold-page-loads',
action='store_true',
On 2013/06/20 19:54:15, nduca wrote:
> rather you not expose it as a commandline. just block your bot hookup of this
on
> dtus benchmark objects.

Done (reluctantly, as I still think this is a useful option).

https://codereview.chromium.org/16154007/diff/19001/tools/telemetry/telemetry...
File tools/telemetry/telemetry/core/browser_options.py (right):

https://codereview.chromium.org/16154007/diff/19001/tools/telemetry/telemetry...
tools/telemetry/telemetry/core/browser_options.py:36: self.cold_page_loads =
False
On 2013/06/20 19:54:15, nduca wrote:
> we have access to the test object in page_runner.
> 
> browser_options should only have options that are used by the backends. this
> isn't used by them, so shouldn't be there. i'm sorry to be cantankerous.

Done. We should really move everything in the "Page set options" section below.

https://codereview.chromium.org/16154007/diff/19001/tools/telemetry/telemetry...
File tools/telemetry/telemetry/page/page_runner.py (right):

https://codereview.chromium.org/16154007/diff/19001/tools/telemetry/telemetry...
tools/telemetry/telemetry/page/page_runner.py:236: util.CloseConnections(tab)
On 2013/06/20 19:54:15, nduca wrote:
> and this for that matter shoudl move into page state or clean up page

Agreed. I decided not to touch CloseConnections in this patch though. I want to
keep it self-contained.

https://codereview.chromium.org/16154007/diff/19001/tools/telemetry/telemetry...
tools/telemetry/telemetry/page/page_runner.py:237: if options.cold_page_loads:
On 2013/06/20 19:54:15, nduca wrote:
> lets make this part of clean up page
> 
> also, to ask a question.... what semantic of cold_page_loads is implied if we
> load a site in two tabs? Should thiss be part of prepare, rather than after?
In
> the world of multi tab, you wouldn't clean up page until after the page was
> completely gone -- meaning going to http://google.com in two tabs would hit
the cache
> on the second tab, in the case that you clear cache after the test runs....

Really good point. I put this into PreparePage.

[nduca, 2013-06-21 22:49:49]

LGTM

[commit-bot: I haz the power, 2013-06-21 22:50:25]

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

[commit-bot: I haz the power, 2013-06-21 23:13:08]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-06-22 00:40:47]

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

[commit-bot: I haz the power, 2013-06-22 00:45:07]

Change committed as 207979