Extension profile generator + benchmark for startup with profile.

tools/perf/profile_creators/extension_profile_extender.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.
+
+import json
+import os
+import tempfile
+import zipfile
+
+import page_sets
+from profile_creators import fast_navigation_profile_extender
+from catapult_base import cloud_storage

[erikchen, 2015/07/14 23:36:14]

these imports should be alphabetically ordered.

[sydli, 2015/07/15 22:56:23]

Done.

+
+# Google Cloud Storage bucket and remote directory to download
+# extensions from

[erikchen, 2015/07/14 23:36:14]

missing a period.

[sydli, 2015/07/15 22:56:23]

Done.

+BUCKET_NAME = "chrome-partner-telemetry"

[erikchen, 2015/07/14 23:36:14]

you shouldn't need to define this variable. Use the variable from cloud_storage.

[sydli, 2015/07/15 22:56:23]

Done.

+DEFAULT_REMOTE_DIR = "extension_set"

[erikchen, 2015/07/14 23:36:14]

DEFAULT implies remote_dir can be changed by someone to be something else. Can
it?

[sydli, 2015/07/15 22:56:23]

No, it shouldn't be able to. Changed the name.

+
+class ExtensionProfileExtender(
+        fast_navigation_profile_extender.FastNavigationProfileExtender):
+  """Creates a small profile with many extensions """
+
+  def __init__(self, finder_options):
+    maximum_batch_size = 1
+    super(ExtensionProfileExtender, self).__init__(
+            finder_options, maximum_batch_size)
+    # Make browser navigate to blank page
+    self._page_set = page_sets.BlankPageSet()
+    urls = []
+    for story in self._page_set.stories:
+      urls.append(story.url)
+    self._navigation_urls = urls

[erikchen, 2015/07/14 23:36:14]

It might not be simpler, but it makes more sense to subclass ProfileExtender
directly instead of FastNavigationProfileExtender. I guess I don't really care
which way you choose.

[sydli, 2015/07/15 22:56:23]

Stuck with FastNav since it creates and destroys the browser nicely.

+
+    if not finder_options.browser_options.profile_dir:
+      finder_options.browser_options.profile_dir = tempfile.mkdtemp()

[erikchen, 2015/07/14 23:36:14]

How do you prevent tempfile.mkdtemp() from leaking? 

[sydli, 2015/07/15 22:56:23]

Didn't realize profile generator already creates such a directory.
Deleted.

+    finder_options.browser_options.disable_default_apps = False

[erikchen, 2015/07/14 23:36:14]

I seem to recall that you also set a similar flag in benchmark.py. The flag
should only be set in one location. (which is presumbably the benchmark, since
it needs to affect both times the browser is started.

[sydli, 2015/07/15 22:56:23]

I set this here as well so the profile generator works (and sets this flag) even
when run independently of the benchmark. Not sure how the browser_options object
is passed around-- Is there a way I can just set it in the profile gen so I
won't have to in benchmark?

[erikchen, 2015/07/16 00:33:28]

nope, you'll need it in both places.

+
+    # DL extensions from cloud & force-install extensions into profile.
+    profile_dir = finder_options.browser_options.profile_dir
+    local_extensions_dir = os.path.join(profile_dir, "External Extensions Crx")

[erikchen, 2015/07/14 23:36:14]

Please don't use spaces in folder names. 

[sydli, 2015/07/15 22:56:23]

Done.

+    self._DownloadRemoteExtensions(

[erikchen, 2015/07/14 23:36:14]

This logic should be called from ProfileExtender.Run(), not Init().

[sydli, 2015/07/15 22:56:24]

Done.

+            BUCKET_NAME,
+            DEFAULT_REMOTE_DIR,
+            local_extensions_dir)
+    self._LoadExtensions(local_extensions_dir, profile_dir)
+
+  def _DownloadRemoteExtensions(self, remote_bucket, remote_extensions_dir,
+          local_extensions_dir):
+    """ Downloads set of common extensions to disk.  """

[erikchen, 2015/07/14 23:36:14]

What exceptions can this function throw? Please document.

[sydli, 2015/07/15 22:56:24]

Should document errors that this function explicitly raises, or anything it
calls that can fault? on failure of cloud_storage.Get, I'll raise exception and
prompt user to run update_remote_extensions

+    remote_crx_files = cloud_storage.List(
+            os.path.join(remote_bucket, remote_extensions_dir))
+    for crx_file in remote_crx_files:
+      if not crx_file.endswith(".crx"):

[erikchen, 2015/07/14 23:36:14]

Should this ever happen? If it should never happen, maybe raise an exception?

[sydli, 2015/07/15 22:56:23]

Deleted. Check here is redundant with check in _Load

+          continue

[erikchen, 2015/07/14 23:36:14]

2 space indentation

[sydli, 2015/07/15 22:56:23]

Done.

+      remote_crx_path = os.path.join(remote_extensions_dir, crx_file)
+      local_crx_path = os.path.join(local_extensions_dir, crx_file)
+      cloud_storage.Get(remote_bucket, remote_crx_path, local_crx_path)
+
+  def _GetExtensionInfoFromCRX(self, crxfile): #TODO: integrity checks?

[erikchen, 2015/07/14 23:36:14]

I wouldn't be too worried about integrity checks. Let the exceptions propagate.

[sydli, 2015/07/15 22:56:23]

Acknowledged.

+    """ Format CRX as expected """
+    crx_zip = zipfile.ZipFile(crxfile)
+    manifest_contents = crx_zip.read('manifest.json')
+    decoded_manifest = json.loads(manifest_contents)
+    crx_version = decoded_manifest['version']
+    extension_name = decoded_manifest['name']
+    return (crx_version, extension_name)
+
+  def _LoadExtensions(self, local_extensions_dir, profile_dir):
+    """ Loads extensions in _local_extensions_dir into finder_options """

[erikchen, 2015/07/14 23:36:14]

what exceptions can this throw?

[sydli, 2015/07/15 22:56:23]

Any File IO or if extension directory was malformed (i.e. contained non-crx
file)

+    ext_files = os.listdir(local_extensions_dir)
+    # IF MAC

[erikchen, 2015/07/14 23:36:14]

It's okay to start off with this benchmark only running on Mac. Just disable the
benchmark on other platforms.

https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...

[sydli, 2015/07/15 22:56:23]

Disabled the benchmark on all platforms except mac. 
How should I disable the profile generator? Currently have a platform.system()
check.

+    external_ext_dir = os.path.join(profile_dir, "External Extensions")
+    os.makedirs(external_ext_dir)
+    for ext_file in ext_files:
+      if not ext_file.endswith('.crx'):

[erikchen, 2015/07/14 23:36:14]

ditto - should this ever happen? maybe this should be an assert

[sydli, 2015/07/15 22:56:23]

Should definitely never happen. I'll raise an exception and prompt the user to
run update_remote_extensions if there's a non-crx file.

+          continue
+      ext_path = os.path.join(local_extensions_dir, ext_file)
+      (version, name) = self._GetExtensionInfoFromCRX(ext_path)
+      extension_info = {'external_crx': ext_path,
+              'external_version': version,

[erikchen, 2015/07/14 23:36:14]

formatting of this dictionary - please see python style guide for a reference

[sydli, 2015/07/15 22:56:24]

Done.

+              '_comment': name}
+      ext_id = os.path.splitext(os.path.basename(ext_path))[0]
+      extension_json_path = os.path.join(external_ext_dir, "%s.json" % ext_id)
+      with open(extension_json_path, 'w') as f:
+        f.write(json.dumps(extension_info))
+
+  def GetUrlIterator(self):
+    """Superclass override."""
+    return iter(self._navigation_urls)
+
+  def ShouldExitAfterBatchNavigation(self):
+    """Superclass override."""
+    return False
+
+  def Run(self):
+    super(ExtensionProfileExtender, self).Run()
+