[Telemetry] Remove test references from user_story_runner.

Move is_multi_tab_test down to shared_page_state.
Remove test.RequestExit and test.IsExiting.

BUG=440101

Review URL: https://codereview.chromium.org/801773004

Cr-Commit-Position: refs/heads/master@{#308964}

5 files changed, 146 insertions, 47 deletions

diff --git a/tools/telemetry/telemetry/page/page_run_end_to_end_unittest.py b/tools/telemetry/telemetry/page/page_run_end_to_end_unittest.py
index 2861d31..2d84d86 100644
--- a/tools/telemetry/telemetry/page/page_run_end_to_end_unittest.py
+++ b/tools/telemetry/telemetry/page/page_run_end_to_end_unittest.py
@@ -249,16 +249,11 @@ class PageRunEndToEndTests(unittest.TestCase):
     ps.pages.append(page)
 
     class TestOneTab(page_test.PageTest):
-      def __init__(self):
-        super(TestOneTab, self).__init__()
-        self._browser = None
-
       def DidStartBrowser(self, browser):
-        self._browser = browser
-        self._browser.tabs.New()
+        browser.tabs.New()
 
-      def ValidateAndMeasurePage(self, *_):
-        assert len(self._browser.tabs) == 1
+      def ValidateAndMeasurePage(self, _, tab, __):
+        assert len(tab.browser.tabs) == 1
 
     test = TestOneTab()
     options = options_for_unittests.GetCopy()
@@ -268,6 +263,30 @@ class PageRunEndToEndTests(unittest.TestCase):
     results = results_options.CreateResults(EmptyMetadataForTest(), options)
     user_story_runner.Run(test, ps, expectations, options, results)
 
+  # Ensure that user_story_runner allows >1 tab for multi-tab test.
+  @decorators.Enabled('has tabs')
+  def testMultipleTabsOkayForMultiTabTest(self):
+    ps = page_set.PageSet()
+    expectations = test_expectations.TestExpectations()
+    page = page_module.Page(
+        'file://blank.html', ps, base_dir=util.GetUnittestDataDir())
+    ps.AddUserStory(page)
+
+    class TestMultiTabs(page_test.PageTest):
+      def TabForPage(self, _, browser):
+        return browser.tabs.New()
+
+      def ValidateAndMeasurePage(self, _, tab, __):
+        assert len(tab.browser.tabs) == 2
+
+    test = TestMultiTabs()
+    options = options_for_unittests.GetCopy()
+    options.output_formats = ['none']
+    options.suppress_gtest_report = True
+    SetUpUserStoryRunnerArguments(options)
+    results = results_options.CreateResults(EmptyMetadataForTest(), options)
+    user_story_runner.Run(test, ps, expectations, options, results)
+
   # Ensure that user_story_runner allows the test to customize the browser
   # before it launches.
   def testBrowserBeforeLaunch(self):
@@ -434,3 +453,48 @@ class PageRunEndToEndTests(unittest.TestCase):
           os.path.join(options.output_dir, 'blank_html.json')))
     finally:
       shutil.rmtree(options.output_dir)
+
+  def _RunPageTestThatRaisesAppCrashException(self, test, max_failures):
+    self.SuppressExceptionFormatting()
+    class TestPage(page_module.Page):
+      def RunNavigateSteps(self, _):
+        raise exceptions.AppCrashException
+
+    ps = page_set.PageSet()
+    for _ in range(5):
+      ps.AddUserStory(
+          TestPage('file://blank.html', ps, base_dir=util.GetUnittestDataDir()))
+    expectations = test_expectations.TestExpectations()
+    options = options_for_unittests.GetCopy()
+    options.output_formats = ['none']
+    options.suppress_gtest_report = True
+    SetUpUserStoryRunnerArguments(options)
+    results = results_options.CreateResults(EmptyMetadataForTest(), options)
+    user_story_runner.Run(test, ps, expectations, options, results,
+                          max_failures=max_failures)
+    return results
+
+  def testSingleTabMeansCrashWillCauseFailureValue(self):
+    class SingleTabTest(page_test.PageTest):
+      # Test is not multi-tab because it does not override TabForPage.
+      def ValidateAndMeasurePage(self, *_):
+        pass
+
+    test = SingleTabTest()
+    results = self._RunPageTestThatRaisesAppCrashException(test, max_failures=1)
+    self.assertEquals([], GetSuccessfulPageRuns(results))
+    self.assertEquals(2, len(results.failures))  # max_failures + 1
+
+  @decorators.Enabled('has tabs')
+  def testMultipleTabsMeansCrashRaises(self):
+    class MultipleTabsTest(page_test.PageTest):
+      # Test *is* multi-tab because it overrides TabForPage.
+      def TabForPage(self, page, browser):
+        return browser.tabs.New()
+      def ValidateAndMeasurePage(self, *_):
+        pass
+
+    test = MultipleTabsTest()
+    with self.assertRaises(page_test.MultiTabTestAppCrashError):
+      self._RunPageTestThatRaisesAppCrashException(test, max_failures=1)
+
diff --git a/tools/telemetry/telemetry/page/page_test.py b/tools/telemetry/telemetry/page/page_test.py
index 6bb111e..2e8f398 100644
--- a/tools/telemetry/telemetry/page/page_test.py
+++ b/tools/telemetry/telemetry/page/page_test.py
@@ -14,6 +14,13 @@ class TestNotSupportedOnPlatformError(Exception):
   """
 
 
+class MultiTabTestAppCrashError(Exception):
+  """PageTest Exception raised after browser or tab crash for multi-tab tests.
+
+  Used to abort the test rather than try to recover from an unknown state.
+  """
+
+
 class Failure(Exception):
   """PageTest Exception raised when an undesired but designed-for problem."""
 
@@ -68,15 +75,17 @@ class PageTest(object):
     self._clear_cache_before_each_run = clear_cache_before_each_run
     self._close_tabs_before_run = True
     self._is_action_name_to_run_optional = is_action_name_to_run_optional
-    # If the test overrides the TabForPage method, it is considered a multi-tab
-    # test.  The main difference between this and a single-tab test is that we
-    # do not attempt recovery for the former if a tab or the browser crashes,
-    # because we don't know the current state of tabs (how many are open, etc.)
-    self.is_multi_tab_test = (self.__class__ is not PageTest and
-                              self.TabForPage.__func__ is not
-                              self.__class__.__bases__[0].TabForPage.__func__)
-    # _exit_requested is set to true when the test requests an early exit.
-    self._exit_requested = False
+
+  @property
+  def is_multi_tab_test(self):
+    """Returns True if the test opens multiple tabs.
+
+    If the test overrides TabForPage, it is deemed a multi-tab test.
+    Multi-tab tests do not retry after tab or browser crashes, whereas,
+    single-tab tests too. That is because the state of multi-tab tests
+    (e.g., how many tabs are open, etc.) is unknown after crashes.
+    """
+    return self.TabForPage.__func__ is not PageTest.TabForPage.__func__
 
   @property
   def discard_first_result(self):
@@ -250,12 +259,6 @@ class PageTest(object):
         tab, skip_waits=page.skip_waits)
     page.RunNavigateSteps(action_runner)
 
-  def IsExiting(self):
-    return self._exit_requested
-
-  def RequestExit(self):
-    self._exit_requested = True
-
   @property
   def action_name_to_run(self):
     return self._action_name_to_run
diff --git a/tools/telemetry/telemetry/page/page_test_unittest.py b/tools/telemetry/telemetry/page/page_test_unittest.py
index 4bad7b0..f190289 100644
--- a/tools/telemetry/telemetry/page/page_test_unittest.py
+++ b/tools/telemetry/telemetry/page/page_test_unittest.py
@@ -4,9 +4,9 @@
 
 import json
 import os
+import unittest
 
 from telemetry import decorators
-from telemetry.core import exceptions
 from telemetry.core import wpr_modes
 from telemetry.page import page as page_module
 from telemetry.page import page_set
@@ -148,3 +148,32 @@ class PageTestUnitTest(page_test_test_case.PageTestTestCase):
     measurement = PageTestWithAction()
     self.RunMeasurement(measurement, ps, options=self._options)
     self.assertTrue(page.run_test_action_called)
+
+
+class MultiTabPageTestUnitTest(unittest.TestCase):
+  def testNoTabForPageReturnsFalse(self):
+    class PageTestWithoutTabForPage(page_test.PageTest):
+      def ValidateAndMeasurePage(self, *_):
+        pass
+    test = PageTestWithoutTabForPage()
+    self.assertFalse(test.is_multi_tab_test)
+
+  def testHasTabForPageReturnsTrue(self):
+    class PageTestWithTabForPage(page_test.PageTest):
+      def ValidateAndMeasurePage(self, *_):
+        pass
+      def TabForPage(self, *_):
+        pass
+    test = PageTestWithTabForPage()
+    self.assertTrue(test.is_multi_tab_test)
+
+  def testHasTabForPageInAncestor(self):
+    class PageTestWithTabForPage(page_test.PageTest):
+      def ValidateAndMeasurePage(self, *_):
+        pass
+      def TabForPage(self, *_):
+        pass
+    class PageTestWithTabForPageInParent(PageTestWithTabForPage):
+      pass
+    test = PageTestWithTabForPageInParent()
+    self.assertTrue(test.is_multi_tab_test)
diff --git a/tools/telemetry/telemetry/page/shared_page_state.py b/tools/telemetry/telemetry/page/shared_page_state.py
index 5968e32..6e6ca1f 100644
--- a/tools/telemetry/telemetry/page/shared_page_state.py
+++ b/tools/telemetry/telemetry/page/shared_page_state.py
@@ -9,11 +9,13 @@ from telemetry import decorators
 from telemetry.core import browser_finder
 from telemetry.core import browser_finder_exceptions
 from telemetry.core import browser_info
+from telemetry.core import exceptions
 from telemetry.core import util
 from telemetry.core import wpr_modes
 from telemetry.core.platform.profiler import profiler_finder
 from telemetry.page import page_test
 from telemetry.user_story import shared_user_story_state
+from telemetry.util import exception_formatter
 from telemetry.util import file_handle
 from telemetry.value import skip
 
@@ -172,7 +174,7 @@ class SharedPageState(shared_user_story_state.SharedUserStoryState):
       # Must wait for tab to commit otherwise it can commit after the next
       # navigation has begun and RenderFrameHostManager::DidNavigateMainFrame()
       # will cancel the next navigation because it's pending. This manifests as
-      # the first navigation in a PageSet freezing indefinitly because the
+      # the first navigation in a PageSet freezing indefinitely because the
       # navigation was silently cancelled when |self.browser.tabs[0]| was
       # committed. Only do this when we just started the browser, otherwise
       # there are cases where previous pages in a PageSet never complete
@@ -227,13 +229,21 @@ class SharedPageState(shared_user_story_state.SharedUserStoryState):
     self._test.DidNavigateToPage(self._current_page, self._current_tab)
 
   def RunUserStory(self, results):
-    self._PreparePage()
-    self._ImplicitPageNavigation()
-    self._test.RunPage(self._current_page, self._current_tab, results)
+    try:
+      self._PreparePage()
+      self._ImplicitPageNavigation()
+      self._test.RunPage(self._current_page, self._current_tab, results)
+    except exceptions.AppCrashException:
+      if self._test.is_multi_tab_test:
+        # Avoid trying to recover from an unknown multi-tab state.
+        exception_formatter.PrintFormattedException(
+            msg='AppCrashException during multi tab test:')
+        raise page_test.MultiTabTestAppCrashError
+      raise
 
   def TearDownState(self, results):
     # NOTE: this is a HACK to get user_story_runner to be generic enough for any
-    # user_story while maintaing existing usecases of page tests. Other
+    # user_story while maintaining existing use cases of page tests. Other
     # SharedUserStory should not call DidRunTest this way.
     self._test.DidRunTest(self.browser, results)
     self._StopBrowser()
diff --git a/tools/telemetry/telemetry/user_story/user_story_runner.py b/tools/telemetry/telemetry/user_story/user_story_runner.py
index 57b217e..a04b899 100644
--- a/tools/telemetry/telemetry/user_story/user_story_runner.py
+++ b/tools/telemetry/telemetry/user_story/user_story_runner.py
@@ -77,8 +77,8 @@ def ProcessCommandLineArgs(parser, args):
     parser.error('--pageset-repeat must be a positive integer.')
 
 
-def _RunUserStoryAndProcessErrorIfNeeded(
-    test, expectations, user_story, results, state):
+def _RunUserStoryAndProcessErrorIfNeeded(expectations, user_story, results,
+                                         state):
   def ProcessError():
     if expectation == 'fail':
       msg = 'Expected exception while running %s' % user_story.display_name
@@ -100,10 +100,6 @@ def _RunUserStoryAndProcessErrorIfNeeded(
     ProcessError()
   except exceptions.AppCrashException:
     ProcessError()
-    if test.is_multi_tab_test:
-      logging.error('Aborting multi-tab test after browser or tab crashed at '
-                    'user story %s' % user_story.display_name)
-      test.RequestExit()
     raise
   except page_action.PageActionNotSupported as e:
     results.AddValue(
@@ -241,8 +237,6 @@ def Run(test, user_story_set, expectations, finder_options, results,
       for _ in xrange(finder_options.pageset_repeat):
         for user_story in group.user_stories:
           for _ in xrange(finder_options.page_repeat):
-            if test.IsExiting():
-              break
             if not state:
               state = group.shared_user_story_state_class(
                   test, finder_options, user_story_set)
@@ -250,19 +244,18 @@ def Run(test, user_story_set, expectations, finder_options, results,
             try:
               _WaitForThermalThrottlingIfNeeded(state.platform)
               _RunUserStoryAndProcessErrorIfNeeded(
-                  test, expectations, user_story, results, state)
+                  expectations, user_story, results, state)
             except exceptions.AppCrashException:
               # Catch AppCrashException to give the story a chance to retry.
               # The retry is enabled by tearing down the state and creating
               # a new state instance in the next iteration.
-              if not test.IsExiting():
-                try:
-                  # If TearDownState raises, do not catch the exception.
-                  # (The AppCrashException was saved as a failure value.)
-                  state.TearDownState(results)
-                finally:
-                  # Later finally-blocks use state, so ensure it is cleared.
-                  state = None
+              try:
+                # If TearDownState raises, do not catch the exception.
+                # (The AppCrashException was saved as a failure value.)
+                state.TearDownState(results)
+              finally:
+                # Later finally-blocks use state, so ensure it is cleared.
+                state = None
             finally:
               has_existing_exception = sys.exc_info() is not None
               try:
@@ -283,7 +276,7 @@ def Run(test, user_story_set, expectations, finder_options, results,
           if (effective_max_failures is not None and
               len(results.failures) > effective_max_failures):
             logging.error('Too many failures. Aborting.')
-            test.RequestExit()
+            return
     finally:
       if state:
         has_existing_exception = sys.exc_info() is not None