[system-health] Generalize loading stories to single-page stories

tools/perf/page_sets/system_health/single_page_stories.py

Index: tools/perf/page_sets/system_health/single_page_stories.py
diff --git a/tools/perf/page_sets/system_health/loading_stories.py b/tools/perf/page_sets/system_health/single_page_stories.py
similarity index 83%
rename from tools/perf/page_sets/system_health/loading_stories.py
rename to tools/perf/page_sets/system_health/single_page_stories.py
index d382bbc737dad46a2d237c450ccd1f6433072d35..b10b3f3cbf704bbbd65205247be86499192a8a2d 100644
--- a/tools/perf/page_sets/system_health/loading_stories.py
+++ b/tools/perf/page_sets/system_health/single_page_stories.py
@@ -33,36 +33,37 @@ def _LogIntoDropboxAccount(action_runner, credentials_path):
   dropbox_login.LoginAccount(action_runner, 'dropbox', credentials_path)
 
 
-def _CloseInboxInterstitialAndWaitUntilGmailReady(platform, action_runner):
-  if platform == 'desktop':
-    # Wait until the UI loads.
-    action_runner.WaitForJavaScriptCondition(
-        'document.getElementById("loading").style.display === "none"')
-  elif platform == 'mobile':
-    # Close the "Get Inbox by Gmail" interstitial.
-    action_runner.WaitForJavaScriptCondition(
-        'document.querySelector("#isppromo a") !== null')
-    action_runner.ExecuteJavaScript(
-        'document.querySelector("#isppromo a").click()')
-    # Wait until the UI loads.
-    action_runner.WaitForJavaScriptCondition(
-        'document.getElementById("apploadingdiv").style.height === "0px"')
-
-
-def _WaitUntilBubblesReady(_, action_runner):
+def _WaitUntilDesktopGmailReady(action_runner):
+  # Wait until the UI loads.
+  action_runner.WaitForJavaScriptCondition(
+      'document.getElementById("loading").style.display === "none"')
+
+
+def _CloseInboxInterstitialAndWaitUntilMobileGmailReady(action_runner):
+  # Close the "Get Inbox by Gmail" interstitial.
+  action_runner.WaitForJavaScriptCondition(
+      'document.querySelector("#isppromo a") !== null')
+  action_runner.ExecuteJavaScript(
+      'document.querySelector("#isppromo a").click()')
+  # Wait until the UI loads.
+  action_runner.WaitForJavaScriptCondition(
+      'document.getElementById("apploadingdiv").style.height === "0px"')
+
+
+def _WaitUntilBubblesReady(action_runner):
   # The #logo element is removed right before the main menu is displayed.
   action_runner.WaitForJavaScriptCondition(
       'document.getElementById("logo") === null')
 
 
-def _WaitUntilSpyChaseReady(_, action_runner):
+def _WaitUntilSpyChaseReady(action_runner):
   # The background of the game canvas is set when the "Tap screen to play"
   # caption is displayed.
   action_runner.WaitForJavaScriptCondition(
       'document.querySelector("#game canvas").style.background !== ""')
 
 
-def _WaitUntilFlickrReady(_, action_runner):
+def _WaitUntilFlickrReady(action_runner):
   # Wait until the 'Recently tagged' view loads.
   action_runner.WaitForJavaScriptCondition('''
       document.querySelector(
@@ -72,7 +73,6 @@ def _WaitUntilFlickrReady(_, action_runner):
 class _PageSpec(object):
 
   def __init__(self, name, url, login_hook=None, post_load_hook=None):
-    assert isinstance(url, (dict, str))
     self._name = name
     self._url = url
     self._login_hook = login_hook
@@ -94,6 +94,17 @@ class _PageSpec(object):
   def post_load_hook(self):
     return self._post_load_hook
 
+  def Resolve(self, platform_name):

[nednguyen, 2016/06/24 17:08:26]

It seems to me that we are going down to the path of declarative json page set
(https://bugs.chromium.org/p/chromium/issues/detail?id=239179)

How is this more simpler than just create a bunch of page classes? My worry with
this kind of strategy is it makes it harder for new comers to know how to
fix/tweaks the page behaviors when they need to.

[petrcermak, 2016/06/27 19:31:21]

You're right. I rewrote the file completely ;-)

+    return _PageSpec(self.name,
+                     self._ResolveField(self._url, platform_name),
+                     self._ResolveField(self._login_hook, platform_name),
+                     self._ResolveField(self._post_load_hook, platform_name))
+
+  def _ResolveField(self, field, platform_name):
+    if isinstance(field, dict):
+      return field.get(platform_name)
+    return field
+
 
 _SINGLE_PAGE_SPECS = {
     # Search and e-commerce.
@@ -225,7 +236,9 @@ _SINGLE_PAGE_SPECS = {
             name='gmail',
             url='https://mail.google.com/mail/',
             login_hook=_LogIntoGoogleAccountAndSetUpGmailSession,
-            post_load_hook=_CloseInboxInterstitialAndWaitUntilGmailReady),
+            post_load_hook={
+                'desktop': _WaitUntilDesktopGmailReady,
+                'mobile': _CloseInboxInterstitialAndWaitUntilMobileGmailReady}),
         _PageSpec(
             name='maps',
             url='https://www.google.com/maps/place/London,+UK/'),
@@ -278,17 +291,18 @@ _SINGLE_PAGE_SPECS = {
 DUMP_WAIT_TIME = 3
 
 
-class _LoadCase(page_module.Page):
-  """Generic System Health user story that loads a page and measures memory."""
+class _SinglePageCase(page_module.Page):
+  """Abstract base class for single-page System Health user stories."""
+  CASE = NotImplemented
 
-  def __init__(self, story_set, group, url, page_spec):
-    # For example, the name of the story for the |page_spec| with name
+  def __init__(self, story_set, group, page_spec):
+    # For example, the name of the load story for the |page_spec| with name
     # 'example' from the 'sample' URL |group| will be 'load:sample:example'.
-    name = 'load:%s:%s' % (group, page_spec.name)
-    super(_LoadCase, self).__init__(
-        page_set=story_set, name=name, url=url,
+    story_name = ':'.join((self.CASE, group, page_spec.name))
+    super(_SinglePageCase, self).__init__(
+        page_set=story_set, name=story_name, url=page_spec.url,
         credentials_path='../data/credentials.json',
-        grouping_keys={'case': 'load', 'group': group})
+        grouping_keys={'case': self.CASE, 'group': group})
     self._page_spec = page_spec
 
   @property
@@ -311,12 +325,20 @@ class _LoadCase(page_module.Page):
   def RunNavigateSteps(self, action_runner):
     if self._page_spec.login_hook:
       self._page_spec.login_hook(action_runner, self.credentials_path)
-    super(_LoadCase, self).RunNavigateSteps(action_runner)
+    super(_SinglePageCase, self).RunNavigateSteps(action_runner)
 
   def RunPageInteractions(self, action_runner):
     action_runner.tab.WaitForDocumentReadyStateToBeComplete()
     if self._page_spec.post_load_hook:
-      self._page_spec.post_load_hook(self.platform, action_runner)
+      self._page_spec.post_load_hook(action_runner)
+
+
+class _LoadCase(_SinglePageCase):
+  """Generic System Health user story that loads a page and measures memory."""
+  CASE = 'load'
+
+  def RunPageInteractions(self, action_runner):
+    super(_LoadCase, self).RunPageInteractions(action_runner)
     self._TakeMemoryMeasurement(action_runner)
 
 
@@ -335,12 +357,10 @@ class _MemorySystemHealthStorySet(story.StorySet):
 
     for group_name, page_specs in _SINGLE_PAGE_SPECS.iteritems():
       for page_spec in page_specs:
-        url = page_spec.url
-        if isinstance(url, dict):
-          url = url.get(self.PLATFORM)
-          if url is None:
-            continue  # URL not supported on the platform.
-        self.AddStory(_LoadCase(self, group_name, url, page_spec))
+        resolved_page_spec = page_spec.Resolve(self.PLATFORM)
+        if not resolved_page_spec.url:
+          continue  # URL not supported on the platform.
+        self.AddStory(_LoadCase(self, group_name, resolved_page_spec))
 
 
 class DesktopMemorySystemHealthStorySet(_MemorySystemHealthStorySet):