Commit 5f712b7b authored by dsinclair's avatar dsinclair Committed by Commit bot

Revert of Change PageTest.WillStartBrowser to depend on platform instead of...

Revert of Change PageTest.WillStartBrowser to depend on platform instead of browser object as it would not ha… (patchset #1 id:1 of https://codereview.chromium.org/554583003/)

Reason for revert:
It looks like the moto E perf bots are broken. This change looks like it makes sense so I'm reverting.

crbug.com/414267

Original issue's description:
> Change PageTest.WillStartBrowser to depend on platform instead of browser object as it would not have been created at this point.
>
> BUG=413481
>
> Committed: https://crrev.com/9bb65868ce3ac1e5aae3a844beacf62281c332b6
> Cr-Commit-Position: refs/heads/master@{#294703}

TBR=nduca@chromium.org,nednguyen@google.com,tonyg@chromium.org,chrishenry@google.com,wuhu@google.com
NOTREECHECKS=true
NOTRY=true
BUG=413481

Review URL: https://codereview.chromium.org/562243004

Cr-Commit-Position: refs/heads/master@{#294816}
parent f916f52c
...@@ -20,8 +20,8 @@ class _DromaeoMeasurement(page_test.PageTest): ...@@ -20,8 +20,8 @@ class _DromaeoMeasurement(page_test.PageTest):
def CustomizeBrowserOptions(self, options): def CustomizeBrowserOptions(self, options):
power.PowerMetric.CustomizeBrowserOptions(options) power.PowerMetric.CustomizeBrowserOptions(options)
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
self._power_metric = power.PowerMetric(platform) self._power_metric = power.PowerMetric(browser)
def DidNavigateToPage(self, page, tab): def DidNavigateToPage(self, page, tab):
self._power_metric.Start(page, tab) self._power_metric.Start(page, tab)
......
...@@ -42,9 +42,9 @@ class _IndexedDbMeasurement(page_test.PageTest): ...@@ -42,9 +42,9 @@ class _IndexedDbMeasurement(page_test.PageTest):
self._power_metric = None self._power_metric = None
self._v8_object_stats_metric = None self._v8_object_stats_metric = None
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
"""Initialize metrics once right before the browser has been launched.""" """Initialize metrics once right before the browser has been launched."""
self._power_metric = power.PowerMetric(platform) self._power_metric = power.PowerMetric(browser)
def DidStartBrowser(self, browser): def DidStartBrowser(self, browser):
"""Initialize metrics once right after the browser has been launched.""" """Initialize metrics once right after the browser has been launched."""
......
...@@ -73,8 +73,8 @@ class _KrakenMeasurement(page_test.PageTest): ...@@ -73,8 +73,8 @@ class _KrakenMeasurement(page_test.PageTest):
def CustomizeBrowserOptions(self, options): def CustomizeBrowserOptions(self, options):
power.PowerMetric.CustomizeBrowserOptions(options) power.PowerMetric.CustomizeBrowserOptions(options)
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
self._power_metric = power.PowerMetric(platform) self._power_metric = power.PowerMetric(browser)
def DidNavigateToPage(self, page, tab): def DidNavigateToPage(self, page, tab):
self._power_metric.Start(page, tab) self._power_metric.Start(page, tab)
......
...@@ -72,8 +72,8 @@ class _OctaneMeasurement(page_test.PageTest): ...@@ -72,8 +72,8 @@ class _OctaneMeasurement(page_test.PageTest):
def CustomizeBrowserOptions(self, options): def CustomizeBrowserOptions(self, options):
power.PowerMetric.CustomizeBrowserOptions(options) power.PowerMetric.CustomizeBrowserOptions(options)
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
self._power_metric = power.PowerMetric(platform) self._power_metric = power.PowerMetric(browser)
def WillNavigateToPage(self, page, tab): def WillNavigateToPage(self, page, tab):
memory_stats = tab.browser.memory_stats memory_stats = tab.browser.memory_stats
......
...@@ -21,8 +21,8 @@ class _RobohornetProMeasurement(page_test.PageTest): ...@@ -21,8 +21,8 @@ class _RobohornetProMeasurement(page_test.PageTest):
def CustomizeBrowserOptions(self, options): def CustomizeBrowserOptions(self, options):
power.PowerMetric.CustomizeBrowserOptions(options) power.PowerMetric.CustomizeBrowserOptions(options)
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
self._power_metric = power.PowerMetric(platform) self._power_metric = power.PowerMetric(browser)
def DidNavigateToPage(self, page, tab): def DidNavigateToPage(self, page, tab):
self._power_metric.Start(page, tab) self._power_metric.Start(page, tab)
......
...@@ -80,8 +80,8 @@ class _SunspiderMeasurement(page_test.PageTest): ...@@ -80,8 +80,8 @@ class _SunspiderMeasurement(page_test.PageTest):
def CustomizeBrowserOptions(self, options): def CustomizeBrowserOptions(self, options):
power.PowerMetric.CustomizeBrowserOptions(options) power.PowerMetric.CustomizeBrowserOptions(options)
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
self._power_metric = power.PowerMetric(platform) self._power_metric = power.PowerMetric(browser)
def DidNavigateToPage(self, page, tab): def DidNavigateToPage(self, page, tab):
self._power_metric.Start(page, tab) self._power_metric.Start(page, tab)
......
...@@ -20,8 +20,8 @@ class ImageDecoding(page_test.PageTest): ...@@ -20,8 +20,8 @@ class ImageDecoding(page_test.PageTest):
options.AppendExtraBrowserArgs('--enable-gpu-benchmarking') options.AppendExtraBrowserArgs('--enable-gpu-benchmarking')
power.PowerMetric.CustomizeBrowserOptions(options) power.PowerMetric.CustomizeBrowserOptions(options)
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
self._power_metric = power.PowerMetric(platform) self._power_metric = power.PowerMetric(browser)
def WillNavigateToPage(self, page, tab): def WillNavigateToPage(self, page, tab):
tab.ExecuteJavaScript(""" tab.ExecuteJavaScript("""
......
...@@ -25,8 +25,8 @@ class Media(page_test.PageTest): ...@@ -25,8 +25,8 @@ class Media(page_test.PageTest):
self._memory_metric = None self._memory_metric = None
self._power_metric = None self._power_metric = None
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
self._power_metric = power.PowerMetric(platform) self._power_metric = power.PowerMetric(browser)
def CustomizeBrowserOptions(self, options): def CustomizeBrowserOptions(self, options):
# Needed to run media actions in JS on touch-based devices as on Android. # Needed to run media actions in JS on touch-based devices as on Android.
......
...@@ -12,8 +12,8 @@ class Memory(page_test.PageTest): ...@@ -12,8 +12,8 @@ class Memory(page_test.PageTest):
self._memory_metric = None self._memory_metric = None
self._power_metric = None self._power_metric = None
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
self._power_metric = power.PowerMetric(platform) self._power_metric = power.PowerMetric(browser)
def DidStartBrowser(self, browser): def DidStartBrowser(self, browser):
self._memory_metric = memory.MemoryMetric(browser) self._memory_metric = memory.MemoryMetric(browser)
......
...@@ -84,9 +84,9 @@ class PageCycler(page_test.PageTest): ...@@ -84,9 +84,9 @@ class PageCycler(page_test.PageTest):
else: else:
cls._cold_run_start_index = args.pageset_repeat * args.page_repeat cls._cold_run_start_index = args.pageset_repeat * args.page_repeat
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
"""Initialize metrics once right before the browser has been launched.""" """Initialize metrics once right before the browser has been launched."""
self._power_metric = power.PowerMetric(platform) self._power_metric = power.PowerMetric(browser)
def DidStartBrowser(self, browser): def DidStartBrowser(self, browser):
"""Initialize metrics once right after the browser has been launched.""" """Initialize metrics once right after the browser has been launched."""
......
...@@ -120,7 +120,7 @@ class PageCyclerUnitTest(unittest.TestCase): ...@@ -120,7 +120,7 @@ class PageCyclerUnitTest(unittest.TestCase):
try: try:
page_cycler.memory = mock_memory_module page_cycler.memory = mock_memory_module
browser = FakeBrowser() browser = FakeBrowser()
cycler.WillStartBrowser(options.browser_options.platform) cycler.WillStartBrowser(browser)
cycler.DidStartBrowser(browser) cycler.DidStartBrowser(browser)
finally: finally:
page_cycler.memory = real_memory_module page_cycler.memory = real_memory_module
......
...@@ -20,8 +20,8 @@ class Smoothness(page_test.PageTest): ...@@ -20,8 +20,8 @@ class Smoothness(page_test.PageTest):
options.AppendExtraBrowserArgs('--running-performance-benchmark') options.AppendExtraBrowserArgs('--running-performance-benchmark')
power.PowerMetric.CustomizeBrowserOptions(options) power.PowerMetric.CustomizeBrowserOptions(options)
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
self._power_metric = power.PowerMetric(platform) self._power_metric = power.PowerMetric(browser)
def WillNavigateToPage(self, page, tab): def WillNavigateToPage(self, page, tab):
self._power_metric.Start(page, tab) self._power_metric.Start(page, tab)
......
...@@ -68,7 +68,7 @@ class SmoothnessUnitTest(page_test_test_case.PageTestTestCase): ...@@ -68,7 +68,7 @@ class SmoothnessUnitTest(page_test_test_case.PageTestTestCase):
tab = FakeTab() tab = FakeTab()
measurement = smoothness.Smoothness() measurement = smoothness.Smoothness()
measurement.WillStartBrowser(tab.browser.platform) measurement.WillStartBrowser(tab.browser)
measurement.WillNavigateToPage(test_page, tab) measurement.WillNavigateToPage(test_page, tab)
measurement.WillRunActions(test_page, tab) measurement.WillRunActions(test_page, tab)
...@@ -166,8 +166,8 @@ class SmoothnessUnitTest(page_test_test_case.PageTestTestCase): ...@@ -166,8 +166,8 @@ class SmoothnessUnitTest(page_test_test_case.PageTestTestCase):
class BuggyMeasurement(smoothness.Smoothness): class BuggyMeasurement(smoothness.Smoothness):
fake_power = None fake_power = None
# Inject fake power metric. # Inject fake power metric.
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
self.fake_power = self._power_metric = FakePowerMetric(platform) self.fake_power = self._power_metric = FakePowerMetric(browser)
measurement = BuggyMeasurement() measurement = BuggyMeasurement()
try: try:
......
...@@ -38,9 +38,9 @@ class TabSwitching(page_test.PageTest): ...@@ -38,9 +38,9 @@ class TabSwitching(page_test.PageTest):
options.disable_background_networking = False options.disable_background_networking = False
power.PowerMetric.CustomizeBrowserOptions(options) power.PowerMetric.CustomizeBrowserOptions(options)
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
self._first_page_in_pageset = True self._first_page_in_pageset = True
self._power_metric = power.PowerMetric(platform, TabSwitching.SAMPLE_TIME) self._power_metric = power.PowerMetric(browser, TabSwitching.SAMPLE_TIME)
def TabForPage(self, page, browser): def TabForPage(self, page, browser):
if self._first_page_in_pageset: if self._first_page_in_pageset:
......
...@@ -17,8 +17,8 @@ class WebRTC(page_test.PageTest): ...@@ -17,8 +17,8 @@ class WebRTC(page_test.PageTest):
self._memory_metric = None self._memory_metric = None
self._power_metric = None self._power_metric = None
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
self._power_metric = power.PowerMetric(platform) self._power_metric = power.PowerMetric(browser)
def DidStartBrowser(self, browser): def DidStartBrowser(self, browser):
self._cpu_metric = cpu.CpuMetric(browser) self._cpu_metric = cpu.CpuMetric(browser)
......
...@@ -15,16 +15,15 @@ class PowerMetric(Metric): ...@@ -15,16 +15,15 @@ class PowerMetric(Metric):
# System power draw while idle. # System power draw while idle.
_quiescent_power_draw_mwh = 0 _quiescent_power_draw_mwh = 0
def __init__(self, platform, quiescent_measurement_time_s=0): def __init__(self, browser, quiescent_measurement_time_s=0):
"""PowerMetric Constructor. """PowerMetric Constructor.
Args: Args:
platform: platform object to use. browser: browser object to use.
quiescent_measurement_time_s: time to measure quiescent power, quiescent_measurement_time_s: time to measure quiescent power,
in seconds. 0 means don't measure quiescent power.""" in seconds. 0 means don't measure quiescent power."""
super(PowerMetric, self).__init__() super(PowerMetric, self).__init__()
self._browser = None self._browser = browser
self._platform = platform
self._running = False self._running = False
self._starting_cpu_stats = None self._starting_cpu_stats = None
self._results = None self._results = None
...@@ -44,15 +43,16 @@ class PowerMetric(Metric): ...@@ -44,15 +43,16 @@ class PowerMetric(Metric):
if not self._running: if not self._running:
return return
self._running = False self._running = False
self._results = self._platform.StopMonitoringPower() self._results = self._browser.platform.StopMonitoringPower()
if self._results: # StopMonitoringPower() can return None. if self._results: # StopMonitoringPower() can return None.
self._results['cpu_stats'] = ( self._results['cpu_stats'] = (
_SubtractCpuStats(self._browser.cpu_stats, self._starting_cpu_stats)) _SubtractCpuStats(self._browser.cpu_stats, self._starting_cpu_stats))
def _MeasureQuiescentPower(self, measurement_time_s): def _MeasureQuiescentPower(self, measurement_time_s):
"""Measure quiescent power draw for the system.""" """Measure quiescent power draw for the system."""
if not self._platform.CanMonitorPower() or \ platform = self._browser.platform
self._platform.CanMeasurePerApplicationPower() or \ if not platform.CanMonitorPower() or \
platform.CanMeasurePerApplicationPower() or \
not measurement_time_s: not measurement_time_s:
return return
...@@ -60,16 +60,14 @@ class PowerMetric(Metric): ...@@ -60,16 +60,14 @@ class PowerMetric(Metric):
if PowerMetric._quiescent_power_draw_mwh: if PowerMetric._quiescent_power_draw_mwh:
return return
self._platform.StartMonitoringPower(self._browser) platform.StartMonitoringPower(self._browser)
time.sleep(measurement_time_s) time.sleep(measurement_time_s)
power_results = self._platform.StopMonitoringPower() power_results = platform.StopMonitoringPower()
PowerMetric._quiescent_power_draw_mwh = ( PowerMetric._quiescent_power_draw_mwh = (
power_results.get('energy_consumption_mwh', 0)) power_results.get('energy_consumption_mwh', 0))
def Start(self, _, tab): def Start(self, _, tab):
self._browser = tab.browser if not tab.browser.platform.CanMonitorPower():
if not self._platform.CanMonitorPower():
return return
self._results = None self._results = None
...@@ -77,11 +75,11 @@ class PowerMetric(Metric): ...@@ -77,11 +75,11 @@ class PowerMetric(Metric):
# This line invokes top a few times, call before starting power measurement. # This line invokes top a few times, call before starting power measurement.
self._starting_cpu_stats = self._browser.cpu_stats self._starting_cpu_stats = self._browser.cpu_stats
self._platform.StartMonitoringPower(self._browser) self._browser.platform.StartMonitoringPower(self._browser)
self._running = True self._running = True
def Stop(self, _, tab): def Stop(self, _, tab):
if not self._platform.CanMonitorPower(): if not tab.browser.platform.CanMonitorPower():
return return
self._StopInternal() self._StopInternal()
......
...@@ -54,7 +54,7 @@ class _RunState(object): ...@@ -54,7 +54,7 @@ class _RunState(object):
page_set.make_javascript_deterministic) page_set.make_javascript_deterministic)
self._last_archive_path = page.archive_path self._last_archive_path = page.archive_path
test.WillStartBrowser(possible_browser.platform) test.WillStartBrowser(self.browser)
self.browser.Start() self.browser.Start()
test.DidStartBrowser(self.browser) test.DidStartBrowser(self.browser)
......
...@@ -406,7 +406,7 @@ class PageRunnerTests(unittest.TestCase): ...@@ -406,7 +406,7 @@ class PageRunnerTests(unittest.TestCase):
self._did_call_will_start = False self._did_call_will_start = False
self._did_call_did_start = False self._did_call_did_start = False
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
self._did_call_will_start = True self._did_call_will_start = True
# TODO(simonjam): Test that the profile is available. # TODO(simonjam): Test that the profile is available.
......
...@@ -193,7 +193,7 @@ class PageTest(command_line.Command): ...@@ -193,7 +193,7 @@ class PageTest(command_line.Command):
if page.startup_url: if page.startup_url:
options.browser_options.startup_url = page.startup_url options.browser_options.startup_url = page.startup_url
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
"""Override to manipulate the browser environment before it launches.""" """Override to manipulate the browser environment before it launches."""
def DidStartBrowser(self, browser): def DidStartBrowser(self, browser):
......
...@@ -68,7 +68,7 @@ class MockPageTest(page_test.PageTest): ...@@ -68,7 +68,7 @@ class MockPageTest(page_test.PageTest):
def ValidatePage(self, page, tab, results): def ValidatePage(self, page, tab, results):
self.func_calls.append('ValidatePage') self.func_calls.append('ValidatePage')
def WillStartBrowser(self, platform): def WillStartBrowser(self, browser):
self.func_calls.append('WillStartBrowser') self.func_calls.append('WillStartBrowser')
def DidStartBrowser(self, browser): def DidStartBrowser(self, browser):
...@@ -201,7 +201,7 @@ class RecordWprUnitTests(tab_test_case.TabTestCase): ...@@ -201,7 +201,7 @@ class RecordWprUnitTests(tab_test_case.TabTestCase):
def testRecorderPageTest_BrowserMethods(self): def testRecorderPageTest_BrowserMethods(self):
record_page_test = record_wpr.RecorderPageTest([]) record_page_test = record_wpr.RecorderPageTest([])
record_page_test.page_test = MockBenchmark().test() record_page_test.page_test = MockBenchmark().test()
record_page_test.WillStartBrowser(self._tab.browser.platform) record_page_test.WillStartBrowser(self._tab.browser)
record_page_test.DidStartBrowser(self._tab.browser) record_page_test.DidStartBrowser(self._tab.browser)
self.assertTrue('WillStartBrowser' in record_page_test.page_test.func_calls) self.assertTrue('WillStartBrowser' in record_page_test.page_test.func_calls)
self.assertTrue('DidStartBrowser' in record_page_test.page_test.func_calls) self.assertTrue('DidStartBrowser' in record_page_test.page_test.func_calls)
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment