[Telemetry] Measure power in tab_switching measurement

[Telemetry] Measure power in tab_switching measurement

* Measure power instead of CPU usage in tab switching measurement.
* Fix a bug in said measurement where the initial open tab in the browser window was not navigated.
* Add a benchmark that switches between 5 blank tabs.

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

[jeremy, 2014-02-25 15:05:00]

https://codereview.chromium.org/178363005/diff/20001/tools/perf/measurements/...
File tools/perf/measurements/power_many_tabs.py (right):

https://codereview.chromium.org/178363005/diff/20001/tools/perf/measurements/...
tools/perf/measurements/power_many_tabs.py:39: return
self._speedindex_metric.IsFinished(tab)
I'm not comfortable using speedindex like this, do you think I should split the
IsFinished() code out of speed index? - if so where should I put it?

If not, what would you prefer I do here?

[qsr, 2014-02-25 15:11:14]

https://codereview.chromium.org/178363005/diff/20001/tools/perf/benchmarks/po...
File tools/perf/benchmarks/power.py (right):

https://codereview.chromium.org/178363005/diff/20001/tools/perf/benchmarks/po...
tools/perf/benchmarks/power.py:10: @test.Enabled('mavericks')
Should we have a new condition for power? Because this is only targeting mac,
while I guess it should also target android when power measurement is available.

https://codereview.chromium.org/178363005/diff/20001/tools/perf/measurements/...
File tools/perf/measurements/power_many_tabs.py (right):

https://codereview.chromium.org/178363005/diff/20001/tools/perf/measurements/...
tools/perf/measurements/power_many_tabs.py:39: return
self._speedindex_metric.IsFinished(tab)
On 2014/02/25 15:05:00, jeremy wrote:
> I'm not comfortable using speedindex like this, do you think I should split
the
> IsFinished() code out of speed index? - if so where should I put it?
> 
> If not, what would you prefer I do here?

Any reason to use speed index instead of usual page load? Or is it the same
thing in this context?

[tonyg, 2014-02-27 06:42:16]

https://codereview.chromium.org/178363005/diff/20001/tools/perf/benchmarks/po...
File tools/perf/benchmarks/power.py (right):

https://codereview.chromium.org/178363005/diff/20001/tools/perf/benchmarks/po...
tools/perf/benchmarks/power.py:10: @test.Enabled('mavericks')
On 2014/02/25 15:11:15, qsr wrote:
> Should we have a new condition for power? Because this is only targeting mac,
> while I guess it should also target android when power measurement is
available.
> 

I've wanted this feature too in many contexts. There are a whole range of
capabilities that would be nice to check. But I'd recommend we file a bug and
not hold up this CL for it.

https://codereview.chromium.org/178363005/diff/20001/tools/perf/measurements/...
File tools/perf/measurements/power_many_tabs.py (right):

https://codereview.chromium.org/178363005/diff/20001/tools/perf/measurements/...
tools/perf/measurements/power_many_tabs.py:14: """Open several blank tabs,
measure power after quiesencence."""
This shouldn't talk about blank tabs. The page set defines that.

https://codereview.chromium.org/178363005/diff/20001/tools/perf/measurements/...
tools/perf/measurements/power_many_tabs.py:16: NUM_TABS = 5
Should this be equal to the number of pages in the page set?

https://codereview.chromium.org/178363005/diff/20001/tools/perf/measurements/...
tools/perf/measurements/power_many_tabs.py:28: # load completion.
Like we talked about offline, I recommend factoring out a WaitForQuiescence()
method here:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...

And then both SpeedIndex and this would invoke it.

https://codereview.chromium.org/178363005/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/core/platform/__init__.py (left):

https://codereview.chromium.org/178363005/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/platform/__init__.py:164: def
CanMonitorPowerSync(self):
Let's move this to a separate CL and hold off on landing it for just a couple
more days. I'm confirming a couple more things.

[jeremy, 2014-02-27 13:41:11]

https://codereview.chromium.org/178363005/diff/20001/tools/perf/measurements/...
File tools/perf/measurements/power_many_tabs.py (right):

https://codereview.chromium.org/178363005/diff/20001/tools/perf/measurements/...
tools/perf/measurements/power_many_tabs.py:14: """Open several blank tabs,
measure power after quiesencence."""
On 2014/02/27 06:42:17, tonyg wrote:
> This shouldn't talk about blank tabs. The page set defines that.

Makes sense, added a five_blank_tabs pageset.

https://codereview.chromium.org/178363005/diff/20001/tools/perf/measurements/...
tools/perf/measurements/power_many_tabs.py:16: NUM_TABS = 5
On 2014/02/27 06:42:17, tonyg wrote:
> Should this be equal to the number of pages in the page set?

Done.

https://codereview.chromium.org/178363005/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/core/platform/__init__.py (left):

https://codereview.chromium.org/178363005/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/platform/__init__.py:164: def
CanMonitorPowerSync(self):
On 2014/02/27 06:42:17, tonyg wrote:
> Let's move this to a separate CL and hold off on landing it for just a couple
> more days. I'm confirming a couple more things.

Done.

[qsr, 2014-02-27 14:00:52]

https://codereview.chromium.org/178363005/diff/60001/tools/perf/measurements/...
File tools/perf/measurements/power_many_tabs.py (right):

https://codereview.chromium.org/178363005/diff/60001/tools/perf/measurements/...
tools/perf/measurements/power_many_tabs.py:37:
util.WaitFor(tab.HasReachedQuiescence, 60)
Should you wait for the quiescence of every tab, and not just the last one?
I guess you could iterate over all tabs, keeping track of the total time spent
so as not to wait for more than one minute.

[jeremy, 2014-02-27 14:43:43]

All fixed, ready for another look

[qsr, 2014-02-27 14:57:12]

LGTM with nit.

https://codereview.chromium.org/178363005/diff/80001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/core/web_contents.py (right):

https://codereview.chromium.org/178363005/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/web_contents.py:58: """Determine whether the page
has reached quiescence after loading.
Maybe add a TODO about looking into this in the context of evicted tabs?

[tonyg, 2014-02-27 19:58:18]

https://codereview.chromium.org/178363005/diff/80001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/core/time_since_last_response.js (right):

https://codereview.chromium.org/178363005/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/time_since_last_response.js:51:
window.timeSinceLastResponseAfterLoadMs = function() {
While we are making this more robust, let's name:

window.__telemetry_testSinceLastResponseAfterLoadMs.

We've collided with vars on the page before and have settled on using the
__telemetry_ prefix for anything we attach to window.

https://codereview.chromium.org/178363005/diff/80001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/core/web_contents.py (right):

https://codereview.chromium.org/178363005/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/web_contents.py:57: def
HasReachedQuiescence(self):
Can we split out just this refactor into a cl separate from the measurement
addition? With the comments here, it should be ready to go.

I mainly want to do that just so that if anything goes wrong, we'll have a clean
trail to know what caused it.

https://codereview.chromium.org/178363005/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/web_contents.py:61: True if 2 seconds have passed
since last resource received, false
Nit, indent the stuff below "Returns:"

Should look like this:
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Comments

https://codereview.chromium.org/178363005/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/web_contents.py:72: if
(self._has_reached_quiescence):
I don't see how we'd reach this or why it needs to be a member in the first
place.

[jeremy, 2014-03-02 13:52:28]

All done, split out quiescence-check refactor here:
https://codereview.chromium.org/185413005/

https://codereview.chromium.org/178363005/diff/80001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/core/time_since_last_response.js (right):

https://codereview.chromium.org/178363005/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/time_since_last_response.js:51:
window.timeSinceLastResponseAfterLoadMs = function() {
On 2014/02/27 19:58:19, tonyg wrote:
> While we are making this more robust, let's name:
> 
> window.__telemetry_testSinceLastResponseAfterLoadMs.
> 
> We've collided with vars on the page before and have settled on using the
> __telemetry_ prefix for anything we attach to window.

Done.

https://codereview.chromium.org/178363005/diff/80001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/core/web_contents.py (right):

https://codereview.chromium.org/178363005/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/web_contents.py:57: def
HasReachedQuiescence(self):
On 2014/02/27 19:58:19, tonyg wrote:
> Can we split out just this refactor into a cl separate from the measurement
> addition? With the comments here, it should be ready to go.
> 
> I mainly want to do that just so that if anything goes wrong, we'll have a
clean
> trail to know what caused it.

Done.

https://codereview.chromium.org/178363005/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/web_contents.py:58: """Determine whether the page
has reached quiescence after loading.
Will check with aberent@ and add a todo if further work is needed here.

https://codereview.chromium.org/178363005/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/web_contents.py:61: True if 2 seconds have passed
since last resource received, false
On 2014/02/27 19:58:19, tonyg wrote:
> Nit, indent the stuff below "Returns:"
> 
> Should look like this:
> http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Comments

Done.

https://codereview.chromium.org/178363005/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/web_contents.py:72: if
(self._has_reached_quiescence):
I added this to match behavior of the original code - the act of reaching
quiescence is permanent i.e. once HasReachedQuiescence() returns True it will
always return True for a given page.

If this isn't a desirable property then I can remove the variable.

[jeremy, 2014-03-05 09:33:42]

Ready for another look.

Per offline discussion with tonyg@, I've modified this CL to reuse the
tab_switching measurement instead of the creating a new measurement in the
interest of reducing bot cycle times.

[tonyg, 2014-03-05 15:50:26]

lgtm

https://codereview.chromium.org/178363005/diff/170001/tools/perf/measurements...
File tools/perf/measurements/tab_switching.py (right):

https://codereview.chromium.org/178363005/diff/170001/tools/perf/measurements...
tools/perf/measurements/tab_switching.py:38: if self._first_page_in_pageset:
Nothing for this CL, but just lamenting that we really need to make multitab
tests a first class citizen in pagesets and pagerunner.

[jeremy, 2014-03-10 09:12:12]

The CQ bit was checked by jeremy@chromium.org

[commit-bot: I haz the power, 2014-03-10 09:12:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeremy@chromium.org/178363005/190001

[commit-bot: I haz the power, 2014-03-10 09:19:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-10 09:19:52]

Try jobs failed on following builders: linux_chromium_clang_dbg

[jeremy, 2014-03-10 09:21:29]

The CQ bit was checked by jeremy@chromium.org

[commit-bot: I haz the power, 2014-03-10 09:22:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeremy@chromium.org/178363005/190001

[commit-bot: I haz the power, 2014-03-10 12:43:44]

Change committed as 255925