telemetry: Add a page set for blink's memory usage measurement

tools/perf/page_sets/blink_memory_mobile.py

+# Copyright 2015 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.
+from telemetry.page import page as page_module

[petrcermak, 2015/08/26 09:33:51]

nit: Please put a blank line above this one.

[bashi, 2015/08/28 01:49:07]

Done.

+from telemetry.page import shared_page_state
+from telemetry import story
+
+from page_sets import key_mobile_sites_pages
+from page_sets.login_helpers import google_login
+
+
+class BlinkMemoryMobilePage(page_module.Page):
+  def __init__(self, url, page_set, name):
+    super(BlinkMemoryMobilePage, self).__init__(
+      url=url, page_set=page_set, name=name,

[petrcermak, 2015/08/26 09:33:50]

Lines 15-17 should be indented 4 spaces (+2)

[bashi, 2015/08/28 01:49:07]

Done.

+      shared_page_state_class=shared_page_state.SharedMobilePageState,
+      credentials_path='data/credentials.json')
+    self.arcive_data_file = 'data/blink_memory_mobile.json'

[Sami, 2015/08/26 10:03:25]

typo: archive_data_file

[bashi, 2015/08/28 01:49:07]

Done.

+
+  def RunPageInteractions(self, action_runner):
+    action_runner.ScrollPage()
+
+
+class TheVergePage(BlinkMemoryMobilePage):

[Sami, 2015/08/26 10:03:26]

Since this isn't enabled yet could we leave it out of this patch?

[bashi, 2015/08/28 01:49:07]

I would like to keep this so that I can keep the TODO line. I'm worrying about
record_wpr never get fixed. Also, I'd like to use this with --use-live-sites
option. How about commenting this out?

[Sami, 2015/08/28 11:32:22]

I see. Keeping this here for the TODO sounds fine.

+  def __init__(self, page_set):
+    super(TheVergePage, self).__init__(
+      # pylint: disable=C0301

[petrcermak, 2015/08/26 09:33:50]

lines 27-30 should be indented 4 spaces (+2)

[bashi, 2015/08/28 01:49:07]

Done.

+      url='http://www.theverge.com/2015/8/11/9133883/taylor-swift-spotify-discover-weekly-what-is-going-on',

[Sami, 2015/08/26 10:03:25]

Looks like this line is too long. Tip: in python you can split strings across
lines like this:

long_string = ('foo'
               'bar')

[bashi, 2015/08/28 01:49:08]

Hmm, I don't think splitting URLs is a good idea. It would be difficult to read
and copy-and-paste to omnibox. Other page sets don't split URLs.

[nednguyen, 2015/08/28 03:58:47]

I think Bashi's point is valid.
You can wait for this to land: https://codereview.chromium.org/1306083005/

+      page_set=page_set,
+      name='TheVerge')
+
+  def RunPageInteractions(self, action_runner):
+    comment_link_selector = '.show_comments_link'

[petrcermak, 2015/08/26 09:33:50]

This should probably be a class constant

[bashi, 2015/08/28 01:49:08]

Done.

+    action_runner.WaitForElement(selector=comment_link_selector)
+    action_runner.ExecuteJavaScript(
+      'window.location.hash = "comments"')

[petrcermak, 2015/08/26 09:33:50]

This line should be indented 4 spaces (+2)

[bashi, 2015/08/28 01:49:07]

Done.

+    action_runner.TapElement(selector=comment_link_selector)
+    action_runner.WaitForJavaScriptCondition(
+      'window.Chorus.Comments.collection.length > 0')

[petrcermak, 2015/08/26 09:33:50]

This line should be indented 4 spaces (+2)

[bashi, 2015/08/28 01:49:07]

Done.

+
+
+class GmailPage(BlinkMemoryMobilePage):
+  def __init__(self, page_set):
+    super(GmailPage, self).__init__(
+      url='https://mail.google.com/mail/',

[petrcermak, 2015/08/26 09:33:50]

Lines 45-47 should be indented 4 spaces (+2)

[bashi, 2015/08/28 01:49:07]

Done.

+      page_set=page_set,
+      name='Gmail')
+
+  def RunNavigateSteps(self, action_runner):
+    google_login.LoginGoogleAccount(action_runner, 'google',
+                                    self.credentials_path)
+    super(GmailPage, self).RunNavigateSteps(action_runner)
+    action_runner.WaitForNavigate()

[Sami, 2015/08/26 10:03:26]

RunNavigateSteps already waits for the navigation to complete so there's no need
to do it again here.

[bashi, 2015/08/28 01:49:07]

Gmail login page redirects multiple times so I needed to add this. Maybe there
is a way to handle such situation in a better way?

+    action_runner.WaitForElement(selector='#apploadingdiv')
+    action_runner.WaitForJavaScriptCondition(
+      'document.querySelector("#apploadingdiv").style.opacity == "0"')

[petrcermak, 2015/08/26 09:33:50]

This line should be indented 4 spaces (+2)

[bashi, 2015/08/28 01:49:08]

Done.

+
+
+class BlinkMemoryMobilePageSet(story.StorySet):
+  """Key mobile sites for Blink memory reduction."""
+
+  def __init__(self):
+    super(BlinkMemoryMobilePageSet, self).__init__(
+      archive_data_file='data/blink_memory_mobile.json',

[petrcermak, 2015/08/26 09:33:50]

Lines 64-65 should be indented 4 spaces (+2)

[bashi, 2015/08/28 01:49:07]

Done.

+      cloud_storage_bucket=story.PARTNER_BUCKET)
+
+    # Why: High rate of Blink's memory consumption rate.

[petrcermak, 2015/08/26 09:33:50]

I don't understand what's the difference between "High rate of Blink's memory
consumption rate" and "Renderer memory usage is high". Is the former just more
specific? Why is it not "Blink memory usage is high" (consistency)?

[bashi, 2015/08/28 01:49:07]

There are some components in renderer process; blink, content, gpu, v8, and so
on. In the former, blink's memory usage is relativity high compared with other
components, and in the latter, renderer memory usage itself is high, according
to the memory-infra.

+    self.AddStory(BlinkMemoryMobilePage(
+      url='https://www.pinterest.com',

[petrcermak, 2015/08/26 09:33:50]

Lines 69-71 should be indented 4 spaces (+2)

[bashi, 2015/08/28 01:49:07]

Done.

+      page_set=self,
+      name='Pinterest'))
+    self.AddStory(key_mobile_sites_pages.FacebookPage(self))
+    # TODO(bashi): Enable TheVergePage. http://crbug.com/522381
+    # self.AddStory(TheVergePage(self))
+
+    # Why: High rate of Blink's memory comsumption rate on low-RAM devices.
+    self.AddStory(BlinkMemoryMobilePage(
+      url='http://en.m.wikipedia.org/wiki/Wikipedia',

[petrcermak, 2015/08/26 09:33:51]

ditto (spaces)

[bashi, 2015/08/28 01:49:07]

Done.

+      page_set=self,
+      name='Wikipedia (1 tab) - delayed scroll start',))
+    self.AddStory(BlinkMemoryMobilePage(
+      url='http://www.reddit.com/r/programming/comments/1g96ve',

[petrcermak, 2015/08/26 09:33:50]

ditto (spaces)

[bashi, 2015/08/28 01:49:08]

Done.

+      page_set=self,
+      name='Reddit'))
+    self.AddStory(BlinkMemoryMobilePage(
+      # pylint: disable=line-too-long

[petrcermak, 2015/08/26 09:33:50]

ditto (spaces)

[Sami, 2015/08/26 10:03:25]

Also please break the long line instead of turning off the linter.

[bashi, 2015/08/28 01:49:08]

I prefer not splitting as described above.

[bashi, 2015/08/28 01:49:08]

Done.

+      url='https://en.blog.wordpress.com/2012/09/04/freshly-pressed-editors-picks-for-august-2012/',
+      page_set=self,
+      name='Wordpress'))
+
+    # Why: Renderer memory usage is high.
+    self.AddStory(BlinkMemoryMobilePage(
+      url='http://worldjournal.com/',

[petrcermak, 2015/08/26 09:33:50]

ditto (spaces)

[bashi, 2015/08/28 01:49:07]

Done.

+      page_set=self,
+      name='Worldjournal'))
+
+    # Why: Key products.
+    self.AddStory(GmailPage(page_set=self))
+    self.AddStory(BlinkMemoryMobilePage(
+      # pylint: disable=line-too-long
+      url='http://googlewebmastercentral.blogspot.com/2015/04/rolling-out-mobile-friendly-update.html?m=1',

[petrcermak, 2015/08/26 09:33:50]

ditto (spaces)

[bashi, 2015/08/28 01:49:07]

Done.

+      page_set=self,
+      name='Blogger'))
+    self.AddStory(BlinkMemoryMobilePage(
+      # pylint: disable=line-too-long

[petrcermak, 2015/08/26 09:33:50]

ditto (spaces)

[bashi, 2015/08/28 01:49:07]

Done.

+      url='https://plus.google.com/app/basic/110031535020051778989/posts?source=apppromo',
+      page_set=self,
+      name='GooglePlus'))