Telemetry: Fix several small problems in the profile creator.

tools/perf/profile_creators/fast_navigation_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 time
 
 from telemetry.core import browser_finder
 from telemetry.core import browser_finder_exceptions
 from telemetry.core import exceptions
 from telemetry.core import platform
 from telemetry.core import util
+from telemetry.core.backends.chrome_inspector import devtools_http
 
 
 class FastNavigationProfileExtender(object):
   """Extends a Chrome profile.
 
   This class creates or extends an existing profile by performing a set of tab
   navigations in large batches. This is accomplished by opening a large number
   of tabs, simultaneously navigating all the tabs, and then waiting for all the
   tabs to load. This provides two benefits:
     - Takes advantage of the high number of logical cores on modern CPUs.
@@ skipping 12 matching lines @@
 
     # The path of the profile that the browser will use while it's running.
     # This member is initialized during SetUp().
     self._profile_path = None
 
     # A reference to the browser that will be performing all of the tab
     # navigations.
     # This member is initialized during SetUp().
     self._browser = None
 
+    # The list of tabs to use for url navigations. This list may not contain
+    # all tabs in the browser.
+    self._navigation_tabs = []
+
     # The number of tabs to use.
     self._NUM_TABS = maximum_batch_size
 
     # The amount of time to wait for a batch of pages to finish loading.
     self._BATCH_PAGE_LOAD_TIMEOUT_IN_SECONDS = 10
 
     # The default amount of time to wait for the retrieval of the URL of a tab.
     self._TAB_URL_RETRIEVAL_TIMEOUT_IN_SECONDS = 1
 
   def Run(self, finder_options):
@@ skipping 35 matching lines @@
     implementation.
     """
     self._profile_path = finder_options.output_profile_path
     possible_browser = self._GetPossibleBrowser(finder_options)
 
     assert possible_browser.supports_tab_control
     assert (platform.GetHostPlatform().GetOSName() in
         ["win", "mac", "linux"])
     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
 
   @property
   def profile_path(self):
     return self._profile_path
 
+  def _RefreshNavigationTabs(self):
+    """Updates the member self._navigation_tabs to contain self._NUM_TABS
+    elements, each of which is not crashed. The crashed tabs are intentionally
+    leaked, since Telemetry doesn't have a good way of killing crashed tabs."""
+    live_tabs = []
+    for tab in self._navigation_tabs:
+      try:
+        live_tab = self._browser.tabs.GetTabById(tab.id)

[nednguyen, 2015/02/19 20:05:04]

If you have to do this just to get the "live tab", I feel like we are doing
something wrong here with the tab_list API.

[erikchen, 2015/02/19 21:55:12]

I've added comments - this method is doing more than just getting "live tabs".
It's intentionally avoiding stalled tabs.

+        live_tabs.append(live_tab)
+      except KeyError:
+        pass
+
+    self._navigation_tabs = live_tabs
+
+    while len(self._navigation_tabs) < self._NUM_TABS:
+      self._navigation_tabs.append(self._browser.tabs.New())
+
+  def _RemoveNavigationTab(self, tab):
+    self._navigation_tabs.remove(tab)
+
   def _GetPossibleBrowser(self, finder_options):
     """Return a possible_browser with the given options."""
     possible_browser = browser_finder.FindBrowser(finder_options)
     if not possible_browser:
       raise browser_finder_exceptions.BrowserFinderException(
           'No browser found.\n\nAvailable browsers:\n%s\n' %
           '\n'.join(browser_finder.GetAllAvailableBrowserTypes(finder_options)))
     finder_options.browser_options.browser_type = (
         possible_browser.browser_type)
 
     return possible_browser
 
   def _RetrieveTabUrl(self, tab, timeout):
     """Retrives the URL of the tab."""
     try:
       return tab.EvaluateJavaScript('document.URL', timeout)
-    except exceptions.DevtoolsTargetCrashException:
+    except (exceptions.DevtoolsTargetCrashException,
+        devtools_http.DevToolsClientConnectionError,
+        devtools_http.DevToolsClientUrlError):
       return None
 
   def _WaitForUrlToChange(self, tab, initial_url, timeout):
     """Waits for the tab to navigate away from its initial url."""
     end_time = time.time() + timeout
     while True:
       seconds_to_wait = end_time - time.time()
       seconds_to_wait = max(0, seconds_to_wait)
 
       if seconds_to_wait == 0:
@@ skipping 20 matching lines @@
     """
     timeout_in_seconds = 0
 
     queued_tabs = []
     for tab, url in batch:
       initial_url = self._RetrieveTabUrl(tab,
           self._TAB_URL_RETRIEVAL_TIMEOUT_IN_SECONDS)
 
       try:
         tab.Navigate(url, None, timeout_in_seconds)
-      except exceptions.DevtoolsTargetCrashException:
-        # We expect a time out, and don't mind if the webpage crashes. Ignore
-        # both exceptions.
+      except (exceptions.DevtoolsTargetCrashException,
+          devtools_http.DevToolsClientConnectionError,
+          devtools_http.DevToolsClientUrlError):
+        # We expect a time out. It's possible for other problems to arise, but
+        # this method is not responsible for dealing with them. Ignore all
+        # exceptions.
         pass
 
       queued_tabs.append((tab, initial_url))
     return queued_tabs
 
   def _WaitForQueuedTabsToLoad(self, queued_tabs):
     """Waits for all the batch navigated tabs to finish loading.
 
     Args:
       queued_tabs: A list of tuples (tab, initial_url). Each tab is guaranteed
       to have already been sent a navigation command.
     """
     end_time = time.time() + self._BATCH_PAGE_LOAD_TIMEOUT_IN_SECONDS
     for tab, initial_url in queued_tabs:
       seconds_to_wait = end_time - time.time()
       seconds_to_wait = max(0, seconds_to_wait)
 
       if seconds_to_wait == 0:
         break
 
       # Since we don't wait any time for the tab url navigation to commit, it's
       # possible that the tab hasn't started navigating yet.
       self._WaitForUrlToChange(tab, initial_url, seconds_to_wait)
 
       seconds_to_wait = end_time - time.time()
       seconds_to_wait = max(0, seconds_to_wait)
 
       try:
         tab.WaitForDocumentReadyStateToBeComplete(seconds_to_wait)
-      except (util.TimeoutException, exceptions.DevtoolsTargetCrashException):
-        # Ignore time outs and web page crashes.
+      except util.TimeoutException:
+        # Ignore time outs.
         pass
+      except (exceptions.DevtoolsTargetCrashException,
+          devtools_http.DevToolsClientConnectionError,
+          devtools_http.DevToolsClientUrlError):
+        # If any error occurs, remove the tab. it's probably in an
+        # unrecoverable state.
+        self._RemoveNavigationTab(tab)
 
   def _GetUrlsToNavigate(self, url_iterator):
     """Returns an array of urls to navigate to, given a url_iterator."""
     urls = []
     for _ in xrange(self._NUM_TABS):
       try:
         urls.append(url_iterator.next())
       except StopIteration:
         break
     return urls
 
   def _PerformNavigations(self):
     """Repeatedly fetches a batch of urls, and navigates to those urls. This
     will run until an empty batch is returned, or
     ShouldExitAfterBatchNavigation() returns True.
     """
     url_iterator = self.GetUrlIterator()
     while True:
+      self._RefreshNavigationTabs()
       urls = self._GetUrlsToNavigate(url_iterator)
 
       if len(urls) == 0:
         break
 
       batch = []
       for i in range(len(urls)):
         url = urls[i]
-        tab = self._browser.tabs[i]
+        tab = self._navigation_tabs[i]
         batch.append((tab, url))
 
       queued_tabs = self._BatchNavigateTabs(batch)
       self._WaitForQueuedTabsToLoad(queued_tabs)
 
       self.CleanUpAfterBatchNavigation()
 
       if self.ShouldExitAfterBatchNavigation():
         break