[tools/perf] Keep both browsers open for the whole of the dual browser test

tools/perf/page_sets/dual_browser_story.py

 # Copyright 2016 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 collections
 import logging
 import re
 import sys
 import urllib
 
@@ skipping 46 matching lines @@
     The story_set is expected to include SinglePage instances (class defined
     below) mapping each page to a browser on which to run. The state
     requires at least one page to run on the 'default' browser, i.e. the
     browser selected from the command line by the user.
     """
     super(MultiBrowserSharedState, self).__init__(
         test, finder_options, story_set)
     self._platform = None
     self._story_set = story_set
     self._possible_browsers = {}
-    # We use an ordered dict to request memory dumps in a deterministic order.
-    self._browsers = collections.OrderedDict()
+    # We use an ordered dict to record the order in which browsers appear on
+    # the story set. However, browsers are not created yet.
+    self._browsers_created = False
+    self._browsers = collections.OrderedDict(
+        (s.browser_type, None) for s in story_set)
     self._current_story = None
     self._current_browser = None
     self._current_tab = None
 
     possible_browser = self._PrepareBrowser('default', 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)))
     if not finder_options.run_disabled_tests:
@@ skipping 52 matching lines @@
     if possible_browser is None:
       return None
 
     if self._platform is None:
       self._platform = possible_browser.platform
     else:
       assert self._platform is possible_browser.platform
     self._possible_browsers[browser_type] = (possible_browser, options)
     return possible_browser
 
-  def _SetCurrentBrowser(self, browser_type):
-    """Select a browser of the given type and bring it to the foreground.
+  def _CreateAllBrowsersIfNeeeded(self):
+    """Launch all browsers needed for the story set, if not already done.
 
-    This launches the browser if it does not exist already or, otherwise, moves
-    the existing browser to the foreground.
+    This ensures that all browsers are alive during the whole duration of the
+    benchmark and, therefore, e.g. memory dumps are always provided for all
+    of them.
     """
-    if not browser_type in self._browsers:
+    if self._browsers_created:

[nednguyen, 2016/06/23 13:13:26]

Are we calling _CreateAllBrowsersIfNeeeded twice anywhere? If not, maybe just
assert this is not called twice?

[nednguyen, 2016/06/23 13:14:06]

nvm, I forgot that WillRunStory is called in the loop

+      return
+    for browser_type in self._browsers:
       possible_browser, options = self._possible_browsers[browser_type]
-      self._current_browser = possible_browser.Create(options)
-      self._browsers[browser_type] = self._current_browser
-    else:
-      self._current_browser = self._browsers[browser_type]
-      self._current_browser.Foreground()
+      self._browsers[browser_type] = possible_browser.Create(options)
+    self._browsers_created = True
+
+  def _CloseAllBrowsers(self):
+    """Close all of the browsers that were launched for this benchmark."""
+    if not self._browsers_created:
+      return
+    for browser_type, browser in self._browsers.iteritems():
+      try:
+        browser.Close()
+      except Exception:
+        logging.exception('Error while closing %s browser', browser_type)
+      self._browsers[browser_type] = None
+    self._browsers_created = False
 
   def CanRunStory(self, _):
     return True
 
   def WillRunStory(self, story):
     self._current_story = story
 
     self.platform.network_controller.StartReplay(
         self._story_set.WprFilePathForStory(story),
         story.make_javascript_deterministic)
 
-    self._SetCurrentBrowser(self._current_story.browser_type)
+    # Note: browsers need to be created after replay has been started.
+    self._CreateAllBrowsersIfNeeeded()
+    self._current_browser = self._browsers[story.browser_type]
+    self._current_browser.Foreground()
     self._current_tab = self._current_browser.foreground_tab
 
   def RunStory(self, _):
     self._current_story.Run(self)
 
   def DidRunStory(self, _):
     self._current_story = None
 
   def TakeMemoryMeasurement(self):
     self.current_tab.action_runner.ForceGarbageCollection()
     self.platform.FlushEntireSystemCache()
     if not self.platform.tracing_controller.is_tracing_running:
       return  # Tracing is not running, e.g., when recording a WPR archive.
     for browser_type, browser in self._browsers.iteritems():
       if not browser.DumpMemory():
         logging.error('Unable to dump memory for %s', browser_type)
 
   def TearDownState(self):
     self.platform.network_controller.Close()
-
-    for browser_type, browser in self._browsers.iteritems():
-      try:
-        browser.Close()
-      except Exception:
-        logging.exception('Error while closing browser: %s', browser_type)
-    self._browsers = None  # Not usable after tearing down.
+    self._CloseAllBrowsers()
 
   def DumpStateUponFailure(self, unused_story, unused_results):
     if self._browsers:
       for browser_type, browser in self._browsers.iteritems():
         logging.info('vvvvv BROWSER STATE BELOW FOR \'%s\' vvvvv', browser_type)
         browser.DumpStateUponFailure()
     else:
       logging.warning('Cannot dump browser states: No browsers.')
 
 
@@ skipping 44 matching lines @@
           url=GOOGLE_SEARCH + urllib.urlencode({'q': query}),
           browser_type='android-webview',
           phase='on_webview'))
 
       # Stories that run on the browser selected by command line options.
       self.AddStory(SinglePage(
           name=re.sub('\W+', '_', url),
           url=url,
           browser_type='default',
           phase='on_chrome'))