diff options
author | pdr@chromium.org <pdr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-12-09 19:39:54 +0000 |
---|---|---|
committer | pdr@chromium.org <pdr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-12-09 19:39:54 +0000 |
commit | 31e81c4edae80e91b9904e79cce4364f33ac7272 (patch) | |
tree | fa46c15bed1f8c0aadf07758e61b3733c9f56d48 /tools/telemetry | |
parent | 8b57e94ec5ce31479af6d04b79c59a652ddecf86 (diff) | |
download | chromium_src-31e81c4edae80e91b9904e79cce4364f33ac7272.zip chromium_src-31e81c4edae80e91b9904e79cce4364f33ac7272.tar.gz chromium_src-31e81c4edae80e91b9904e79cce4364f33ac7272.tar.bz2 |
[Telemetry] Fix page_cycler for pages with client side redirects.
Page.disable clears all scripts to evaluate on commit. There is no way
to detect client side redirects because linkedin, for example, performs
its client side redirect after the page has fully loaded.
In order for the page_cyclers to work on pages with client side
redirects, the script to evaluate on commit must execute on the
redirected page.
So to workaround this, we avoid ever calling Page.disable. I don't
expect continuing to listen for notifications here to impact
performance, but if we do see any performance reduction on the bots we
should revert and search for another approach.
This was originally landed as r209505 but had to be rolled out. This
patch simply updates the original patch since the regression was
unrelated and has been fixed.
BUG=253604
NOTRY=true
TEST=tools/perf/run_measurement --browser=canary page_cycler tools/perf/page_sets/top_10_mobile.json --pageset-repeat=2
Review URL: https://codereview.chromium.org/103373003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239532 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'tools/telemetry')
-rw-r--r-- | tools/telemetry/telemetry/core/backends/chrome/inspector_page.py | 67 |
1 files changed, 40 insertions, 27 deletions
diff --git a/tools/telemetry/telemetry/core/backends/chrome/inspector_page.py b/tools/telemetry/telemetry/core/backends/chrome/inspector_page.py index 8c4c6fe..e107127 100644 --- a/tools/telemetry/telemetry/core/backends/chrome/inspector_page.py +++ b/tools/telemetry/telemetry/core/backends/chrome/inspector_page.py @@ -15,8 +15,12 @@ class InspectorPage(object): 'Page', self._OnNotification, self._OnClose) + + # Turn on notifications. We need them to get the Page.frameNavigated event. + self._EnablePageNotifications() self._navigation_pending = False self._navigation_url = "" + self._script_to_evaluate_on_commit = None def _OnNotification(self, msg): logging.debug('Notification: %s', json.dumps(msg, indent=2)) @@ -32,16 +36,36 @@ class InspectorPage(object): def _OnClose(self): pass - def _EnablePageNotifications(self, timeout): - request = { - 'method': 'Page.enable' - } - res = self._inspector_backend.SyncRequest(request, timeout) - assert len(res['result'].keys()) == 0 + def _SetScriptToEvaluateOnCommit(self, source): + existing_source = (self._script_to_evaluate_on_commit and + self._script_to_evaluate_on_commit['source']) + if source == existing_source: + return + if existing_source: + request = { + 'method': 'Page.removeScriptToEvaluateOnLoad', + 'params': { + 'identifier': self._script_to_evaluate_on_commit['id'], + } + } + self._inspector_backend.SyncRequest(request) + self._script_to_evaluate_on_commit = None + if source: + request = { + 'method': 'Page.addScriptToEvaluateOnLoad', + 'params': { + 'scriptSource': source, + } + } + res = self._inspector_backend.SyncRequest(request) + self._script_to_evaluate_on_commit = { + 'id': res['result']['identifier'], + 'source': source + } - def _DisablePageNotifications(self, timeout): + def _EnablePageNotifications(self, timeout=60): request = { - 'method': 'Page.disable' + 'method': 'Page.enable' } res = self._inspector_backend.SyncRequest(request, timeout) assert len(res['result'].keys()) == 0 @@ -55,19 +79,15 @@ class InspectorPage(object): start_time = time.time() remaining_time = timeout + action_function() + self._navigation_pending = True try: - self._EnablePageNotifications(remaining_time) - try: - action_function() - self._navigation_pending = True - while self._navigation_pending and remaining_time > 0: - remaining_time = max(timeout - (time.time() - start_time), 0.0) - self._inspector_backend.DispatchNotifications(remaining_time) - finally: - self._DisablePageNotifications(remaining_time) + while self._navigation_pending and remaining_time > 0: + remaining_time = max(timeout - (time.time() - start_time), 0.0) + self._inspector_backend.DispatchNotifications(remaining_time) except util.TimeoutException: - # Since we pass remaining_time as timeout to all of the calls in this, - # method, we need to list the full timeout time in this message. + # Since we pass remaining_time to DispatchNotifications, we need to + # list the full timeout time in this message. raise util.TimeoutException('Timed out while waiting %ds for navigation. ' 'Error=%s' % (timeout, sys.exc_info()[1])) @@ -80,14 +100,7 @@ class InspectorPage(object): """ def DoNavigate(): - if script_to_evaluate_on_commit: - request = { - 'method': 'Page.addScriptToEvaluateOnLoad', - 'params': { - 'scriptSource': script_to_evaluate_on_commit, - } - } - self._inspector_backend.SyncRequest(request) + self._SetScriptToEvaluateOnCommit(script_to_evaluate_on_commit) # Navigate the page. However, there seems to be a bug in chrome devtools # protocol where the request id for this event gets held on the browser # side pretty much indefinitely. |