Change profile_creator to not subclass page_test

tools/perf/profile_creators/extensions_profile_creator.py

Index: tools/perf/profile_creators/extensions_profile_creator.py
diff --git a/tools/perf/profile_creators/extensions_profile_creator.py b/tools/perf/profile_creators/extensions_profile_creator.py
index 027439b7a8726011b18857d2b7c6682f6e31bb0c..e65e72d5e2330db2b7b29cae1b9faacf680b69ce 100644
--- a/tools/perf/profile_creators/extensions_profile_creator.py
+++ b/tools/perf/profile_creators/extensions_profile_creator.py
@@ -18,6 +18,11 @@ from telemetry.page import profile_creator
 
 import page_sets
 
+from telemetry import benchmark
+from telemetry.page import page_test
+from telemetry.page import test_expectations
+from telemetry.results import results_options
+from telemetry.user_story import user_story_runner
 
 def _ExternalExtensionsPath():
   """Returns the OS-dependent path at which to install the extension deployment
@@ -75,20 +80,69 @@ def _GetExtensionInfoFromCRX(crx_path):
   return (crx_version, extension_name)
 
 class ExtensionsProfileCreator(profile_creator.ProfileCreator):
-  """Virtual base class for profile creators that install extensions.
+  """Abstract base class for profile creators that install extensions.
 
   Extensions are installed using the mechanism described in
   https://developer.chrome.com/extensions/external_extensions.html .
 
   Subclasses are meant to be run interactively.
   """
+  class PageTest(page_test.PageTest):

[dtu, 2015/01/09 01:23:58]

I'm finding the nested classes hard to read. Can you move it to the top-level
and precede the name with an underscore?

[erikchen, 2015/01/09 03:12:42]

Done. I renamed it to _ExtensionPageTest just to make it a little more
differentiable from PageTest.

+    def __init__(self):
+      super(ExtensionsProfileCreator.PageTest, self).__init__()
+      self._page_set = page_sets.Typical25PageSet()
+
+      # Have the extensions been installed yet?
+      self._extensions_installed = False
+
+      # Expected
+      self._expected_extension_count = 0
+
+    def CanRunForPage(self, page):
+      # Superclass override.
+      # No matter how many pages in the pageset, just perform two test
+      # iterations.
+      return page.page_set.pages.index(page) < 2
+
+    def ValidateAndMeasurePage(self, _, tab, results):
+      # Superclass override.
+      # Profile setup works in 2 phases:
+      # Phase 1: When the first page is loaded: we wait for a timeout to allow
+      #     all extensions to install and to prime safe browsing and other
+      #     caches.  Extensions may open tabs as part of the install process.
+      # Phase 2: When the second page loads, user_story_runner closes all tabs -
+      #     we are left with one open tab, wait for that to finish loading.
+
+      # Sleep for a bit to allow safe browsing and other data to load +
+      # extensions to install.
+      if not self._extensions_installed:
+        sleep_seconds = 5 * 60
+        logging.info("Sleeping for %d seconds." % sleep_seconds)
+        time.sleep(sleep_seconds)
+        self._extensions_installed = True
+      else:
+        # Phase 2: Wait for tab to finish loading.
+        for i in xrange(len(tab.browser.tabs)):
+          t = tab.browser.tabs[i]
+          t.WaitForDocumentReadyStateToBeComplete()
+
+    def DidRunTest(self, browser, results):
+      """Superclass override."""
+      super(ExtensionsProfileCreator.PageTest, self).DidRunTest(browser,
+          results)
+      # Do some basic sanity checks to make sure the profile is complete.
+      installed_extensions = browser.extensions.keys()
+      if not len(installed_extensions) == self._expected_extension_count:
+        # Diagnosing errors:
+        # Too many extensions: Managed environment may be installing additional
+        # extensions.
+        raise Exception("Unexpected number of extensions installed in browser",
+            installed_extensions)
+
 
   def __init__(self):
     super(ExtensionsProfileCreator, self).__init__()
-    self._page_set = page_sets.Typical25()
-
-    # Directory into which the output profile is written.
-    self._output_profile_path = None
+    self._page_test = ExtensionsProfileCreator.PageTest()

[dtu, 2015/01/09 01:23:58]

Why create a side-effect on the class if you're using this entirely within
Run()?

[erikchen, 2015/01/09 03:12:42]

I removed the member, and made it a local variable.

 
     # List of extensions to install.
     self._extensions_to_install = []
@@ -99,12 +153,36 @@ class ExtensionsProfileCreator(profile_creator.ProfileCreator):
     # Directory to download extension files into.
     self._extension_download_dir = None
 
-    # Have the extensions been installed yet?
-    self._extensions_installed = False
-
     # List of files to delete after run.
     self._files_to_cleanup = []
 
+  def Run(self, options):
+    self._PrepareExtensionInstallFiles()
+
+    expectations = test_expectations.TestExpectations()
+    results = results_options.CreateResults(
+        benchmark.BenchmarkMetadata(profile_creator.__class__.__name__),
+        options)
+    self._page_test._expected_extension_count = len(self._extensions_to_install)
+    user_story_runner.Run(self._page_test, self._page_test._page_set,
+        expectations, options, results)
+
+    self._CleanupExtensionInstallFiles()
+
+    # Check that files on this list exist and have content.
+    expected_files = [
+        os.path.join('Default', 'Network Action Predictor')]
+    for filename in expected_files:
+      filename = os.path.join(options.output_profile_path, filename)
+      if not os.path.getsize(filename) > 0:
+        raise Exception("Profile not complete: %s is zero length." % filename)
+
+    if results.failures:
+      logging.warning('Some pages failed.')
+      logging.warning('Failed pages:\n%s',
+                      '\n'.join(map(str, results.pages_that_failed)))
+      raise Exception('ExtensionsProfileCreator failed.')
+
   def _PrepareExtensionInstallFiles(self):
     """Download extension archives and create extension install files."""
     extensions_to_install = self._extensions_to_install
@@ -151,73 +229,3 @@ class ExtensionsProfileCreator(profile_creator.ProfileCreator):
       shutil.rmtree(self._extension_download_dir)
       self._extension_download_dir = None
 
-  def CustomizeBrowserOptions(self, options):
-    self._output_profile_path = options.output_profile_path
-
-  def WillRunTest(self, options):
-    """Run before browser starts.
-
-    Download extensions and write installation files."""
-    super(ExtensionsProfileCreator, self).WillRunTest(options)
-
-    # Running this script on a corporate network or other managed environment
-    # could potentially alter the profile contents.
-    hostname = socket.gethostname()
-    if hostname.endswith('corp.google.com'):
-      raise Exception("It appears you are connected to a corporate network "
-          "(hostname=%s).  This script needs to be run off the corp "
-          "network." % hostname)
-
-    prompt = ("\n!!!This script must be run on a fresh OS installation, "
-        "disconnected from any corporate network. Are you sure you want to "
-        "continue? (y/N) ")
-    if (raw_input(prompt).lower() != 'y'):
-      sys.exit(-1)
-    self._PrepareExtensionInstallFiles()
-
-  def DidRunTest(self, browser, results):
-    """Run before exit."""
-    super(ExtensionsProfileCreator, self).DidRunTest()
-    # Do some basic sanity checks to make sure the profile is complete.
-    installed_extensions = browser.extensions.keys()
-    if not len(installed_extensions) == len(self._extensions_to_install):
-      # Diagnosing errors:
-      # Too many extensions: Managed environment may be installing additional
-      # extensions.
-      raise Exception("Unexpected number of extensions installed in browser",
-          installed_extensions)
-
-    # Check that files on this list exist and have content.
-    expected_files = [
-        os.path.join('Default', 'Network Action Predictor')]
-    for filename in expected_files:
-      filename = os.path.join(self._output_profile_path, filename)
-      if not os.path.getsize(filename) > 0:
-        raise Exception("Profile not complete: %s is zero length." % filename)
-
-    self._CleanupExtensionInstallFiles()
-
-  def CanRunForPage(self, page):
-    # No matter how many pages in the pageset, just perform two test iterations.
-    return page.page_set.pages.index(page) < 2
-
-  def ValidateAndMeasurePage(self, _, tab, results):
-    # Profile setup works in 2 phases:
-    # Phase 1: When the first page is loaded: we wait for a timeout to allow
-    #     all extensions to install and to prime safe browsing and other
-    #     caches.  Extensions may open tabs as part of the install process.
-    # Phase 2: When the second page loads, user_story_runner closes all tabs -
-    #     we are left with one open tab, wait for that to finish loading.
-
-    # Sleep for a bit to allow safe browsing and other data to load +
-    # extensions to install.
-    if not self._extensions_installed:
-      sleep_seconds = 5 * 60
-      logging.info("Sleeping for %d seconds." % sleep_seconds)
-      time.sleep(sleep_seconds)
-      self._extensions_installed = True
-    else:
-      # Phase 2: Wait for tab to finish loading.
-      for i in xrange(len(tab.browser.tabs)):
-        t = tab.browser.tabs[i]
-        t.WaitForDocumentReadyStateToBeComplete()