Add benchmark that imitates news reading.

tools/perf/page_sets/system_health/browsing_stories.py

+# Copyright 2016 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import sys
+
+from page_sets.system_health import platforms
+from page_sets.system_health import system_health_story
+
+from telemetry import story
+
+
+class _BrowsingStory(system_health_story.SystemHealthStory):
+  """Abstract base class for browsing stories.
+
+  A browsing story visits items on the main page. Subclasses provide
+  CSS selector to identify the items and implement interaction using
+  the helper methods of this class.
+  """
+
+  IS_SINGLE_PAGE_APP = False
+  ITEM_SELECTOR = NotImplemented
+  ITEMS_TO_VISIT = 4
+
+  def __init__(self, story_set):
+    super(_BrowsingStory, self).__init__(
+        story_set, take_memory_measurement=False)
+
+  def _Item(self, index):

[petrcermak, 2016/07/08 12:46:03]

nit: I think that this should be called something like _BuildItemSelector

[ulan, 2016/07/08 13:57:56]

I inlined this function in NavigateToItem.

+    return 'document.querySelectorAll(\'%s\')[%d]' % (

[petrcermak, 2016/07/08 12:46:03]

nit: I suggest you use " inside the string to avoid the backslashes ;-)

[ulan, 2016/07/08 13:57:56]

Done.

+        self.ITEM_SELECTOR, index)
+
+  def _WaitForItem(self, action_runner, index):

[petrcermak, 2016/07/08 12:46:03]

Do you expect any subclass to override _WaitForItem, _ClickItem or
_WaitForNavigation? If not, then I suggest you just inline them into
_NavigateToNewsItem (which I suggested you move to this class).

[ulan, 2016/07/08 13:57:57]

Done. I thought that the media browsing benchmark needs WaitForItem and
ClickItem. But now I think that media browsing benchmark could also use higher
level NavigateToItem method.

+    action_runner.WaitForElement(element_function=self._Item(index))
+
+  def _ClickItem(self, action_runner, index):
+    action_runner.ClickElement(element_function=self._Item(index))
+
+  def _WaitForNavigation(self, action_runner):
+    if not self.IS_SINGLE_PAGE_APP:
+      action_runner.WaitForNavigate()
+
+  def _NavigateBack(self, action_runner):
+    action_runner.ExecuteJavaScript('window.history.back()')
+    self._WaitForNavigation(action_runner)
+
+
+class _NewsBrowsingStory(_BrowsingStory):
+  """Abstract base class for news user stories.
+
+  A news story imitates browsing a news website:
+  1. Load the main page.
+  2. Open and scroll the first news item.
+  3. Go back to the main page and scroll it.
+  4. Open and scroll the second news item.
+  5. Go back to the main page and scroll it.
+  6. etc.
+  """
+
+  ITEM_READ_TIME_IN_SECONDS = 3
+  ITEM_SCROLL_REPEAT = 2
+  MAIN_PAGE_SCROLL_REPEAT = 0
+
+  def RunPageInteractions(self, action_runner):

[ulan, 2016/07/08 10:50:55]

This one is not calling the SystemHealthStory.RunPageInteraction, which is not
nice.

I could introduce _RunPageInteractions, which is called by
SystemHealthStory.RunPageInteraction() after loading the document and override 
_RunPageInteractions here. wdyt?

[petrcermak, 2016/07/08 12:46:03]

I think that the best solution would be to move the for loop into
_DidLoadDocument and not override RunPageInteractions at all. How does that
sound?

[ulan, 2016/07/08 13:57:57]

I am using _DidLoadDocument as a hook to do special actions like closing pop-up
(see WashingtonPostMobileStory) before the actual interactions.

If I move everything to _DidLoadDocument, I would need another hook that runs
before _DidLoadDocument but after loading the document.

[petrcermak, 2016/07/08 15:06:28]

I don't think that's true. You get
action_runner.tab.WaitForDocumentReadyStateToBeComplete() for free as it's in
SystemHealthStory.RunPageInteractions. All you'd have to do is:

class _NewsBrowsingStory:

  def _DidLoadDocument(self, action_runner):
    for i in xrange(self.ITEMS_TO_VISIT):
      ...

class WashingtonPostMobileStory:

  def _DidLoadDocument(self, action_runner):
    action_runner.ClickElement(selector='.close')
    super(WashingtonPostMobileStory, self)._DidLoadDocument(action_runner)

[ulan, 2016/07/08 15:40:47]

Very nice, thank you! I didn't think about that. Done.

+    action_runner.tab.WaitForDocumentReadyStateToBeComplete()
+    self._DidLoadDocument(action_runner)
+    for i in xrange(self.ITEMS_TO_VISIT):
+      self._NavigateToNewsItem(action_runner, i)
+      self._ReadNewsItem(action_runner)
+      self._NavigateBack(action_runner)
+      self._ScrollMainPage(action_runner)
+
+  def _NavigateToNewsItem(self, action_runner, index):

[petrcermak, 2016/07/08 12:46:03]

Why is this method not defined on _BrowsingStory? It doesn't seem to be news
related.

[ulan, 2016/07/08 13:57:57]

Done.

+    self._WaitForItem(action_runner, index)

[petrcermak, 2016/07/08 12:46:03]

These methods build the item selector twice unnecessarily. Plus they both have
to know how selectors are created. I suggest you build the selector first and
then pass it to them:

item_selector = 'document.querySelectorAll("%s")[%d]' % (self.ITEM_SELECTOR,
index)
action_runner.WaitForElement(element_function=item_selector)
action_runner.ClickElement(element_function=item_selector)
if not self.IS_SINGLE_PAGE_APP:
  action_runner.WaitForNavigate()

[ulan, 2016/07/08 13:57:56]

Done, but I left WaitForNavigation in separate function because it is used in
NavigateBack().

[petrcermak, 2016/07/12 12:53:19]

Acknowledged.

+    self._ClickItem(action_runner, index)
+    self._WaitForNavigation(action_runner)
+
+  def _ReadNewsItem(self, action_runner):
+    action_runner.tab.WaitForDocumentReadyStateToBeComplete()
+    action_runner.Wait(self.ITEM_READ_TIME_IN_SECONDS)
+    action_runner.RepeatableBrowserDrivenScroll(
+        repeat_count=self.ITEM_SCROLL_REPEAT)
+
+  def _ScrollMainPage(self, action_runner):
+    action_runner.tab.WaitForDocumentReadyStateToBeComplete()
+    action_runner.RepeatableBrowserDrivenScroll(
+        repeat_count=self.MAIN_PAGE_SCROLL_REPEAT)
+
+
+##############################################################################
+# News browsing stories.
+##############################################################################
+

[nednguyen, 2016/07/08 12:54:42]

Looks like we don't cover any non-US news pages here. I think we can punt on
reddit & hacknews for some international news pages?

[Hannes Payer (out of office), 2016/07/08 13:52:54]

Hacker news is nice because it goes to other webpages when you click on a link.
Kind of nice to have this behavior as well. 

[ulan, 2016/07/08 13:57:57]

Petr suggested to sync load:news and browse:news.

Our motivation for browse:news pages that are not in load:news:
- Nytimes and hackernews have the largest V8 heaps in mobile browse:news
workload (except CNN which crashes with OOM).
- Washingtonpost is WPA.
- Facebook and twitter are super-popular pages.

Any strong opinion for removing one of these pages? Otherwise, I'll add these
pages to load:news.

Now pages in load:news that are not in browse:new: bbc, qq, sohu, wikipedia.

Should I move qq and sohu to browse:news?

[nednguyen, 2016/07/08 14:01:35]

Ah, that's a good point. Those I think we should use Google news search for this
behavior because google news's page content has a lot more things than
hackernews.

[nednguyen, 2016/07/08 14:17:14]

Keeping reddit is fine.
I suggest we replace hacker news with google news search for the reason stated. 
For qq & sohu, you can just copy them here instead of removing them from
load:news. I can buy that we may be move all the loading stories to browsing
stories one day, but not now :-)

[petrcermak, 2016/07/08 15:06:29]

Facebook and Twitter are in load:social. I suggest you put your stirues under
browse:social:facebook and browse:social:twitter.
Not really. If you have a strong opinion for adding a browse:X:Y, I suggest you
just make sure that there's load:X:Y as well. If you think that some of the
existing loading stories doesn't make sense (because it's superseded by you new
load:X:Y story), then you can remove it.
That would be great as it would address the problem with non-US pages.

Note that load:X:Y should load the same URL as the initial url of the
corresponding browse:X:Y (so that the two can be "subtracted"). We can do that
in a follow up patch though if you want ;-)

[ulan, 2016/07/08 15:40:47]

Changed facebook and twitter to browse:social.

I'll discuss the pages with Hannes and update the patch.

Let's sync the url in a separate change.

[petrcermak, 2016/07/08 15:58:39]

SGTM. Thanks :-)

+
+class RedditDesktopStory(_NewsBrowsingStory):
+  """The top website in http://www.alexa.com/topsites/category/News"""
+  NAME = 'browse:news:reddit'
+  URL = 'https://www.reddit.com/top/?sort=top&t=week'
+  SUPPORTED_PLATFORMS = platforms.DESKTOP_ONLY
+  ITEM_SELECTOR = '.thing .title > a'
+
+
+class RedditMobileStory(_NewsBrowsingStory):
+  """The top website in http://www.alexa.com/topsites/category/News"""
+  NAME = 'browse:news:reddit'
+  URL = 'https://m.reddit.com/?sort=top&time=week'
+  SUPPORTED_PLATFORMS = platforms.MOBILE_ONLY
+  ITEM_SELECTOR = '.PostHeader__post-title-line'
+  IS_SINGLE_PAGE_APP = True
+
+
+class CnnStory(_NewsBrowsingStory):
+  """The second top website in http://www.alexa.com/topsites/category/News"""
+  NAME = 'browse:news:cnn'
+  URL = 'http://edition.cnn.com/'
+  ITEM_SELECTOR = '.cd__content > h3 > a'
+  # TODO(ulan): Enable this story on mobile once it uses less memory and
+  # does not crash with OOM.
+  SUPPORTED_PLATFORMS = platforms.DESKTOP_ONLY
+
+
+class NytimesMobileStory(_NewsBrowsingStory):
+  """The third top website in http://www.alexa.com/topsites/category/News"""
+  NAME = 'browse:news:nytimes'
+  URL = 'http://mobile.nytimes.com'
+  SUPPORTED_PLATFORMS = platforms.MOBILE_ONLY
+  ITEM_SELECTOR = '.sfgAsset-link'
+  # Visiting more items causes OOM.
+  ITEMS_TO_VISIT = 2
+
+
+class NytimesDesktopStory(_NewsBrowsingStory):
+  """The third top website in http://www.alexa.com/topsites/category/News"""
+  NAME = 'browse:news:nytimes'
+  URL = 'http://www.nytimes.com'
+  SUPPORTED_PLATFORMS = platforms.DESKTOP_ONLY
+  ITEM_SELECTOR = '.story-heading > a'
+
+
+class HackerNewsStory(_NewsBrowsingStory):
+  NAME = 'browse:news:hackernews'
+  URL = 'https://news.ycombinator.com'
+  ITEM_SELECTOR = '.athing .title > a'
+
+
+# Facebook on desktop is not interesting because it embeds post comments
+# directly in the main timeline.
+class FacebookMobileStory(_NewsBrowsingStory):
+  NAME = 'browse:news:facebook'
+  URL = 'https://facebook.com/natgeo'
+  ITEM_SELECTOR = 'article ._5msj'
+  SUPPORTED_PLATFORMS = platforms.MOBILE_ONLY
+
+
+class TwitterMobileStory(_NewsBrowsingStory):
+  NAME = 'browse:news:twitter'
+  URL = 'https://mobile.twitter.com/cnnbrk'
+  ITEM_SELECTOR = '.Tweet-text'
+  SUPPORTED_PLATFORMS = platforms.MOBILE_ONLY
+
+
+class TwitterDesktopStory(_NewsBrowsingStory):
+  NAME = 'browse:news:twitter'
+  URL = 'https://twitter.com/cnnbrk'
+  ITEM_SELECTOR = '.tweet-text'
+  SUPPORTED_PLATFORMS = platforms.DESKTOP_ONLY
+  IS_SINGLE_PAGE_APP = True
+
+
+class WashingtonPostMobileStory(_NewsBrowsingStory):
+  """Progressive website"""
+  NAME = 'browse:news:washingtonpost'
+  URL = 'https://www.washingtonpost.com/pwa'
+  SUPPORTED_PLATFORMS = platforms.MOBILE_ONLY
+  ITEM_SELECTOR = '.hed > a'
+  IS_SINGLE_PAGE_APP = True
+
+  def _DidLoadDocument(self, action_runner):
+    # Close the popup window.
+    action_runner.ClickElement(selector='.close')
+
+
+class WashingtonPostDesktopStory(_NewsBrowsingStory):
+  NAME = 'browse:news:washingtonpost'
+  URL = 'https://www.washingtonpost.com'
+  SUPPORTED_PLATFORMS = platforms.DESKTOP_ONLY
+  ITEM_SELECTOR = '.headline > a'
+
+
+##############################################################################
+# Browsing story sets.
+##############################################################################
+

[petrcermak, 2016/07/08 12:46:03]

nit: please add a second blank line here (there should be two blank lines
between top-level statements).

[ulan, 2016/07/08 13:57:57]

Done.

+def IterAllStoryClasses():

[petrcermak, 2016/07/08 12:46:03]

This will also return the _NewsBrowsingStory abstract class, which is
undesirable. We'll need to figure out a more scalable approach so that we don't
need to define this kind of function in every single file in system_health/.
Therefore, I suggest you remove this method now and we add it later when we
actually add the stories to the SH set.

[ulan, 2016/07/08 13:57:57]

Done.

+  return system_health_story.IterAllStoryClasses(
+      sys.modules[__name__], _BrowsingStory)
+
+
+def IterAllNewsBrowsingStoryClasses():

[petrcermak, 2016/07/08 12:46:03]

I'd make this private for the time being (leading underscore).

[ulan, 2016/07/08 13:57:56]

Done.

+  return system_health_story.IterAllStoryClasses(
+      sys.modules[__name__], _NewsBrowsingStory)
+
+
+class _NewsBrowsingStorySet(story.StorySet):
+  PLATFORM = NotImplemented
+
+  def __init__(self):
+    super(_NewsBrowsingStorySet, self).__init__(
+        archive_data_file=('../data/news_%s.json' % self.PLATFORM),
+        cloud_storage_bucket=story.PARTNER_BUCKET)
+    for story_class in IterAllNewsBrowsingStoryClasses():
+      if self.PLATFORM not in story_class.SUPPORTED_PLATFORMS:
+        continue
+      self.AddStory(story_class(self))
+
+
+class DesktopNewsStorySet(_NewsBrowsingStorySet):
+  PLATFORM = platforms.DESKTOP
+
+
+class MobileNewsStorySet(_NewsBrowsingStorySet):
+  PLATFORM = platforms.MOBILE