Fix SysfsPowerMonitor to let it work even if |browser| is None

Fix SysfsPowerMonitor to let it work even if |browser| is None

measurements/tab_switching.py creates power.PowerMetric (which calls StartMonitoringPower() in __init__) in WillStartBrowser() where |browser| is still None.
SysfsPowerMonitor should not expect |browser| to be always valid.

With this change, tab_switching.top_10 can output power related values as the result.

BUG=443077, 433442
TEST=tools/perf/run_benchmark --browser=cros-chrome --remote=172.23.69.61 tab_switching.top_10

Committed: https://crrev.com/9d1b7b54e996fba5b17d8f17769bebb69f2cc812
Cr-Commit-Position: refs/heads/master@{#309337}

[hashimoto, 2014-12-19 07:29:38]

hashimoto@chromium.org changed reviewers:
+ nednguyen@google.com

[nednguyen, 2014-12-19 14:53:48]

nednguyen@google.com changed reviewers:
+ tonyg@chromium.org

[nednguyen, 2014-12-19 14:53:49]

https://codereview.chromium.org/818653002/diff/1/tools/telemetry/telemetry/co...
File
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor.py
(right):

https://codereview.chromium.org/818653002/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor.py:51:
def StartMonitoringPower(self, browser):
can we remove the browser param?

[hashimoto, 2014-12-19 16:04:04]

https://codereview.chromium.org/818653002/diff/1/tools/telemetry/telemetry/co...
File
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor.py
(right):

https://codereview.chromium.org/818653002/diff/1/tools/telemetry/telemetry/co...
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor.py:51:
def StartMonitoringPower(self, browser):
On 2014/12/19 14:53:49, nednguyen wrote:
> can we remove the browser param?

I thought I could, but turned out that DumpsysPowerMonitor is using it. (they
are even disabling pylint warning for it!)
Fixed DumpsysPowerMonitor to remember |_browser| itself.

[nednguyen, 2014-12-19 16:17:09]

On 2014/12/19 16:04:04, hashimoto wrote:
>
https://codereview.chromium.org/818653002/diff/1/tools/telemetry/telemetry/co...
> File
> tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor.py
> (right):
> 
>
https://codereview.chromium.org/818653002/diff/1/tools/telemetry/telemetry/co...
>
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor.py:51:
> def StartMonitoringPower(self, browser):
> On 2014/12/19 14:53:49, nednguyen wrote:
> > can we remove the browser param?
> 
> I thought I could, but turned out that DumpsysPowerMonitor is using it. (they
> are even disabling pylint warning for it!)
> Fixed DumpsysPowerMonitor to remember |_browser| itself.

yeah, I look at it more carefully and turn out power_monitor.PowerMonitor API
require browser instance in StartMonitoringPower. So probably suppress pylint
with StartMonitoringPower(self, _browser) seems like the right thing. Looking at
the implementation of android_dumpsys_power_monitor though, it doesn't need the
browser instance until StopMonitoringPower. Wonder whether that's the cases for
other power_monitors.

[nednguyen, 2014-12-19 16:17:22]

nednguyen@google.com changed reviewers:
+ jeremy@chromium.org

[hashimoto, 2014-12-19 16:53:14]

Can we submit this change while leaving the browser argument?
This change is needed to run tab_switching.top_10.

We can clean up the browser argument later.

[nednguyen, 2014-12-20 01:06:55]

lgtm

Yeah, your patch is fine.

https://codereview.chromium.org/818653002/diff/20001/tools/telemetry/telemetr...
File
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor.py
(right):

https://codereview.chromium.org/818653002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor.py:51:
def StartMonitoringPower(self, browser):
s/browser/_browser so pylint won't complain.

[nednguyen, 2014-12-20 01:07:51]

https://codereview.chromium.org/818653002/diff/20001/tools/telemetry/telemetr...
File
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor.py
(right):

https://codereview.chromium.org/818653002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor.py:51:
def StartMonitoringPower(self, browser):
You may also add documentation to explain that this does not require browser and
the browser object can be None.

[hashimoto, 2014-12-20 04:01:59]

Patchset #3 (id:40001) has been deleted

[hashimoto, 2014-12-20 04:02:31]

https://codereview.chromium.org/818653002/diff/20001/tools/telemetry/telemetr...
File
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor.py
(right):

https://codereview.chromium.org/818653002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/platform/power_monitor/sysfs_power_monitor.py:51:
def StartMonitoringPower(self, browser):
On 2014/12/20 01:07:51, nednguyen wrote:
> You may also add documentation to explain that this does not require browser
and
> the browser object can be None.

Done.

[hashimoto, 2014-12-20 04:02:34]

The CQ bit was checked by hashimoto@chromium.org

[commit-bot: I haz the power, 2014-12-20 04:03:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/818653002/60001

[commit-bot: I haz the power, 2014-12-20 05:07:35]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2014-12-20 05:08:28]

Patchset 3 (id:??) landed as
https://crrev.com/9d1b7b54e996fba5b17d8f17769bebb69f2cc812
Cr-Commit-Position: refs/heads/master@{#309337}