Commit fd51b8b0 authored by qsr@chromium.org's avatar qsr@chromium.org

[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. we 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 is a reland of https://codereview.chromium.org/103373003 fixing failures
on the bots.

BUG=253604
R=pdr@chromium.org
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/112413006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243533 0039d316-1c4b-4281-b951-d872f2087c98
parent ed1cca4f
...@@ -91,8 +91,6 @@ class PageCyclerNetsimTop10(test.Test): ...@@ -91,8 +91,6 @@ class PageCyclerNetsimTop10(test.Test):
class PageCyclerTop10Mobile(test.Test): class PageCyclerTop10Mobile(test.Test):
enabled = False # Fails on Android.
test = page_cycler.PageCycler test = page_cycler.PageCycler
page_set = 'page_sets/top_10_mobile.json' page_set = 'page_sets/top_10_mobile.json'
options = {'pageset_repeat_iters': 10} options = {'pageset_repeat_iters': 10}
......
...@@ -15,8 +15,12 @@ class InspectorPage(object): ...@@ -15,8 +15,12 @@ class InspectorPage(object):
'Page', 'Page',
self._OnNotification, self._OnNotification,
self._OnClose) self._OnClose)
self._navigation_pending = False self._navigation_pending = False
self._navigation_url = "" self._navigation_url = ""
self._script_to_evaluate_on_commit = None
# Turn on notifications. We need them to get the Page.frameNavigated event.
self._EnablePageNotifications()
def _OnNotification(self, msg): def _OnNotification(self, msg):
logging.debug('Notification: %s', json.dumps(msg, indent=2)) logging.debug('Notification: %s', json.dumps(msg, indent=2))
...@@ -32,16 +36,36 @@ class InspectorPage(object): ...@@ -32,16 +36,36 @@ class InspectorPage(object):
def _OnClose(self): def _OnClose(self):
pass pass
def _EnablePageNotifications(self, timeout): def _SetScriptToEvaluateOnCommit(self, source):
request = { existing_source = (self._script_to_evaluate_on_commit and
'method': 'Page.enable' self._script_to_evaluate_on_commit['source'])
} if source == existing_source:
res = self._inspector_backend.SyncRequest(request, timeout) return
assert len(res['result'].keys()) == 0 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 = { request = {
'method': 'Page.disable' 'method': 'Page.enable'
} }
res = self._inspector_backend.SyncRequest(request, timeout) res = self._inspector_backend.SyncRequest(request, timeout)
assert len(res['result'].keys()) == 0 assert len(res['result'].keys()) == 0
...@@ -55,19 +79,15 @@ class InspectorPage(object): ...@@ -55,19 +79,15 @@ class InspectorPage(object):
start_time = time.time() start_time = time.time()
remaining_time = timeout remaining_time = timeout
action_function()
self._navigation_pending = True
try: try:
self._EnablePageNotifications(remaining_time) while self._navigation_pending and remaining_time > 0:
try: remaining_time = max(timeout - (time.time() - start_time), 0.0)
action_function() self._inspector_backend.DispatchNotifications(remaining_time)
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)
except util.TimeoutException: except util.TimeoutException:
# Since we pass remaining_time as timeout to all of the calls in this, # Since we pass remaining_time to DispatchNotifications, we need to
# method, we need to list the full timeout time in this message. # list the full timeout time in this message.
raise util.TimeoutException('Timed out while waiting %ds for navigation. ' raise util.TimeoutException('Timed out while waiting %ds for navigation. '
'Error=%s' % (timeout, sys.exc_info()[1])) 'Error=%s' % (timeout, sys.exc_info()[1]))
...@@ -80,14 +100,7 @@ class InspectorPage(object): ...@@ -80,14 +100,7 @@ class InspectorPage(object):
""" """
def DoNavigate(): def DoNavigate():
if script_to_evaluate_on_commit: self._SetScriptToEvaluateOnCommit(script_to_evaluate_on_commit)
request = {
'method': 'Page.addScriptToEvaluateOnLoad',
'params': {
'scriptSource': script_to_evaluate_on_commit,
}
}
self._inspector_backend.SyncRequest(request)
# Navigate the page. However, there seems to be a bug in chrome devtools # 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 # protocol where the request id for this event gets held on the browser
# side pretty much indefinitely. # side pretty much indefinitely.
......
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