Telemetry: Create new profile creator large_profile_creator.

tools/perf/profile_creators/fast_navigation_profile_extender.py

Index: tools/perf/profile_creators/fast_navigation_profile_extender.py
diff --git a/tools/perf/profile_creators/fast_navigation_profile_extender.py b/tools/perf/profile_creators/fast_navigation_profile_extender.py
index 397002856d105f0541ee75549e37ebc30e772aae..1d25af4d0d6f378485dc1ed639cd0d581cf8bf25 100644
--- a/tools/perf/profile_creators/fast_navigation_profile_extender.py
+++ b/tools/perf/profile_creators/fast_navigation_profile_extender.py
@@ -21,24 +21,26 @@ class FastNavigationProfileExtender(object):
       with the number of batches, but does not scale with the size of the
       batch.
   """
-  def __init__(self):
+  def __init__(self, batch_size):
+    """Initializer.
+
+    Args:
+      batch_size: A positive integer indicating the number of tabs to
+      simultaneously perform navigations.
+    """
     super(FastNavigationProfileExtender, self).__init__()
 
+    # The path of the profile that the browser will use while it's running.
+    # This member is initialized during SetUp().
+    self.profile_path = None

[nednguyen, 2015/02/19 00:33:32]

If this doesn't need to be accessed from outside, then make it _profile_path.
If this could be accessed from outside, but shouldn't be mutated, please add
@property def profile_path(self):...

[erikchen, 2015/02/19 01:56:03]

It needs to be accessed from subclasses. I added a property, and prefixed the
member name with an underscore.

+
     # A reference to the browser that will be performing all of the tab
     # navigations.
+    # This member is initialized during SetUp().
     self._browser = None
 
-    # A static copy of the urls that this class is going to navigate to.
-    self._navigation_urls = None
-
     # The number of tabs to use.
-    self._NUM_TABS = 15
-
-    # The number of pages to load in parallel.
-    self._NUM_PARALLEL_PAGES = 15
-
-    assert self._NUM_PARALLEL_PAGES <= self._NUM_TABS, (' the batch size can\'t'
-        ' be larger than the number of available tabs')
+    self._NUM_TABS = batch_size
 
     # The amount of time to wait for a batch of pages to finish loading.
     self._BATCH_PAGE_LOAD_TIMEOUT_IN_SECONDS = 10
@@ -55,19 +57,66 @@ class FastNavigationProfileExtender(object):
       profile, and sufficient information to choose a specific browser binary.
     """
     try:
-      self._navigation_urls = self.GetUrlsToNavigate()
-      self._SetUpBrowser(finder_options)
+      self.SetUp(finder_options)
       self._PerformNavigations()
     finally:
-      self._TearDownBrowser()
+      self.TearDown()
+
+  def GetUrlsToNavigate(self, maximum_batch_size):
+    """Gets URLs for the browser to navigate to.
+
+    Intended for subclass override.
+
+    Args:
+      maximum_batch_size: A positive integer indicating the maximum allowed
+      length of the returned list.
+    Returns:
+      A list of urls to be navigated to.
+    """
+    raise NotImplementedError()
 
-  def GetUrlsToNavigate(self):
-    """Returns a list of urls to be navigated to.
+  def ShouldExitAfterBatchNavigation(self):
+    """Returns a boolean indicating whether profile extension is finished.
 
     Intended for subclass override.
     """
     raise NotImplementedError()
 
+  def SetUp(self, finder_options):
+    """Finds the browser, starts the browser, and opens the requisite number of
+    tabs.
+
+    Can be overridden by subclasses. Subclasses must call the super class
+    implementation.
+    """
+    self.profile_path = finder_options.output_profile_path
+    possible_browser = self._GetPossibleBrowser(finder_options)
+
+    assert possible_browser.supports_tab_control
+    assert possible_browser.platform.GetOSName() in ["win", "mac", "linux"]

[nednguyen, 2015/02/19 00:33:32]

Is the what we want here "possible_browser.platform is
platform.GetHostPlatform()"?

Do you intend for this to be run on ChromeOs?

[erikchen, 2015/02/19 01:56:03]

I replaced possible_browser.platform with platform.GetHostPlatform(). 

I'm not familiar with ChromeOS, and I haven't tried running this script there.
The goal is to be able to run on all platforms, eventually.

+    self._browser = possible_browser.Create(finder_options)
+
+    while(len(self._browser.tabs) < self._NUM_TABS):
+      self._browser.tabs.New()
+
+  def TearDown(self):
+    """Teardown that is guaranteed to be executed before the instance is
+    destroyed.
+
+    Can be overridden by subclasses. Subclasses must call the super class
+    implementation.
+    """
+    if self._browser:
+      self._browser.Close()
+      self._browser = None
+
+  def CleanUpAfterBatchNavigation(self):
+    """A hook for subclasses to perform cleanup after each batch of
+    navigations.
+
+    Can be overridden by subclasses.
+    """
+    pass
 
   def _GetPossibleBrowser(self, finder_options):
     """Return a possible_browser with the given options."""
@@ -162,40 +211,27 @@ class FastNavigationProfileExtender(object):
         # Ignore time outs and web page crashes.
         pass
 
-  def _SetUpBrowser(self, finder_options):
-    """Finds the browser, starts the browser, and opens the requisite number of
-    tabs."""
-    possible_browser = self._GetPossibleBrowser(finder_options)
-    self._browser = possible_browser.Create(finder_options)
-
-    for _ in range(self._NUM_TABS):
-      self._browser.tabs.New()
-
   def _PerformNavigations(self):
-    """Performs the navigations specified by |_navigation_urls| in large
-    batches."""
-    # The index of the first url that has not yet been navigated to.
-    navigation_url_index = 0
+    """Repeatedly fetches a batch of urls, and navigates to those urls. This
+    will run until an empty batch is returned, or
+    ShouldExitAfterBatchNavigation() returns True.
+    """
     while True:
       # Generate the next batch of navigations.
+      urls = self.GetUrlsToNavigate(self._NUM_TABS)

[nednguyen, 2015/02/19 00:33:32]

I don't quite like the NUM_TABS param because it requires the subclass to handle
the "batch logic". 

I think the code here could be written:

urls_iter = iter(self.GetUrlsToNavigate()) # <- or this can return an iterator
directly

while True:
  for _ in xrange(self._NUM_TABS):
     urls.append(self.urls_iter.next())

[erikchen, 2015/02/19 01:56:03]

I like your suggestion. The result is cleaner. Done.

+      if len(urls) == 0:
+        break
+
       batch = []
-      max_index = min(navigation_url_index + self._NUM_PARALLEL_PAGES,
-          len(self._navigation_urls))
-      for i in range(navigation_url_index, max_index):
-        url = self._navigation_urls[i]
-        tab = self._browser.tabs[i % self._NUM_TABS]
+      for i in range(len(urls)):
+        url = urls[i]
+        tab = self._browser.tabs[i]
         batch.append((tab, url))
-      navigation_url_index = max_index
 
       queued_tabs = self._BatchNavigateTabs(batch)
       self._WaitForQueuedTabsToLoad(queued_tabs)
 
-      if navigation_url_index == len(self._navigation_urls):
-        break
+      self.CleanUpAfterBatchNavigation()
 
-  def _TearDownBrowser(self):
-    """Teardown that is guaranteed to be executed before the instance is
-    destroyed."""
-    if self._browser:
-      self._browser.Close()
-      self._browser = None
+      if self.ShouldExitAfterBatchNavigation():
+        break