Remove expected leftover minidumps

Removes expected leftover minidumps from several tests that
intentionally crash a Chrome process. This is so that these
intentional crashes are not caught as failures once pre/post test
minidump checking is enabled with crrev.com/c/2090590.

Bug: 1056235
Change-Id: Iad026563b3a7acfc6e2078ffec36a000ed4b2dec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2080863
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748433}

content/test/gpu/gpu_tests/gpu_integration_test.py

@@ -172,9 +172,13 @@ class GpuIntegrationTest(
       # generator?
       if len(args) == 1 and isinstance(args[0], tuple):
         args = args[0]
+      expected_crashes = self.GetExpectedCrashes(args)
       self.RunActualGpuTest(url, *args)
     except Exception:
       if ResultType.Failure in expected_results or should_retry_on_failure:
+        # We don't check the return value here since we'll be raising the
+        # caught exception already.
+        self._ClearExpectedCrashes(expected_crashes)
         if should_retry_on_failure:
           logging.exception('Exception while running flaky test %s', test_name)
           # For robustness, shut down the browser and restart it
@@ -206,9 +210,16 @@ class GpuIntegrationTest(
         self._RestartBrowser('unexpected test failure')
       raise
     else:
+      # We always want to clear any expected crashes, but we don't bother
+      # failing the test if it's expected to fail.
+      actual_and_expected_crashes_match = self._ClearExpectedCrashes(
+          expected_crashes)
       if ResultType.Failure in expected_results:
         logging.warning(
           '%s was expected to fail, but passed.\n', test_name)
+      else:
+        if not actual_and_expected_crashes_match:
+          raise RuntimeError('Actual and expected crashes did not match')
 
   def _IsIntel(self, vendor_id):
     return vendor_id == 0x8086
@@ -238,6 +249,59 @@ class GpuIntegrationTest(
       return True
     return False
 
+  def _ClearExpectedCrashes(self, expected_crashes):
+    """Clears any expected crash minidumps so they're not caught later.
+
+    Args:
+      expected_crashes: A dictionary mapping crash types as strings to the
+          number of expected crashes of that type.
+
+    Returns:
+      True if the actual number of crashes matched the expected number,
+      otherwise False.
+    """
+    # We can't get crashes if we don't have a browser.
+    if self.browser is None:
+      return True
+    # TODO(crbug.com/1006331): Properly match type once we have a way of
+    # checking the crashed process type without symbolizing the minidump.
+    total_expected_crashes = sum(expected_crashes.values())
+    # The Telemetry-wide cleanup will handle any remaining minidumps, so early
+    # return here since we don't expect any, which saves us a bit of work.
+    if total_expected_crashes == 0:
+      return True
+    unsymbolized_minidumps = self.browser.GetAllUnsymbolizedMinidumpPaths()
+    total_unsymbolized_minidumps = len(unsymbolized_minidumps)
+
+    if total_expected_crashes == total_unsymbolized_minidumps:
+      for path in unsymbolized_minidumps:
+        self.browser.IgnoreMinidump(path)
+      return True
+
+    logging.error(
+        'Found %d unsymbolized minidumps when we expected %d. Expected '
+        'crash breakdown: %s', total_unsymbolized_minidumps,
+        total_expected_crashes, expected_crashes)
+    return False
+
+  def GetExpectedCrashes(self, args):
+    """Returns which crashes, per process type, to expect for the current test.
+
+    Should be overridden by child classes to actually return valid data if
+    available.
+
+    Args:
+      args: The list passed to _RunGpuTest()
+
+    Returns:
+      A dictionary mapping crash types as strings to the number of expected
+      crashes of that type. Examples include 'gpu' for the GPU process,
+      'renderer' for the renderer process, and 'browser' for the browser
+      process.
+    """
+    del args
+    return {}
+
   @classmethod
   def GenerateGpuTests(cls, options):
     """Subclasses must implement this to yield (test_name, url, args)

content/test/gpu/gpu_tests/pixel_integration_test.py

@@ -116,6 +116,21 @@ class PixelIntegrationTest(
           # test makes assertions about the active GPU.
           time.sleep(4)
 
+  def GetExpectedCrashes(self, args):
+    """Returns which crashes, per process type, to expect for the current test.
+
+    Args:
+      args: The list passed to _RunGpuTest()
+
+    Returns:
+      A dictionary mapping crash types as strings to the number of expected
+      crashes of that type. Examples include 'gpu' for the GPU process,
+      'renderer' for the renderer process, and 'browser' for the browser
+      process.
+    """
+    # args[0] is the PixelTestPage for the current test.
+    return args[0].expected_per_process_crashes
+
   def _RunSkiaGoldBasedPixelTest(self, do_page_action, page):
     """Captures and compares a test image using Skia Gold.
 

content/test/gpu/gpu_tests/pixel_test_pages.py

@@ -30,6 +30,9 @@ _FOUR_COLOR_VIDEO_240x135_EXPECTED_COLORS = [
 ]
 
 
+CRASH_TYPE_GPU = 'gpu'
+
+
 class PixelTestPage(object):
   """A wrapper class mimicking the functionality of the PixelTestsStorySet
   from the old-style GPU tests.
@@ -37,7 +40,8 @@ class PixelTestPage(object):
   def __init__(self, url, name, test_rect, tolerance=2, browser_args=None,
                expected_colors=None, gpu_process_disabled=False,
                optional_action=None, restart_browser_after_test=False,
-               other_args=None, grace_period_end=None):
+               other_args=None, grace_period_end=None,
+               expected_per_process_crashes=None):
     super(PixelTestPage, self).__init__()
     self.url = url
     self.name = name
@@ -75,6 +79,10 @@ class PixelTestPage(object):
     # be triaged without turning the bots red.
     # This should be a datetime.date object.
     self.grace_period_end = grace_period_end
+    # This lets the test runner know that one or more crashes are expected as
+    # part of the test. Should be a map of process type (str) to expected number
+    # of crashes (int).
+    self.expected_per_process_crashes = expected_per_process_crashes or {}
 
   def CopyWithNewBrowserArgsAndSuffix(self, browser_args, suffix):
     return PixelTestPage(
@@ -468,7 +476,10 @@ class PixelTestPages(object):
         base_name + '_Video_Context_Loss_MP4',
         test_rect=[0, 0, 240, 135],
         tolerance=tolerance,
-        expected_colors=_FOUR_COLOR_VIDEO_240x135_EXPECTED_COLORS),
+        expected_colors=_FOUR_COLOR_VIDEO_240x135_EXPECTED_COLORS,
+        expected_per_process_crashes={
+          CRASH_TYPE_GPU: 1,
+        }),
 
       # The VP9 test clip is primarily software decoded on bots.
       PixelTestPage(
@@ -477,7 +488,10 @@ class PixelTestPages(object):
         base_name + '_Video_Context_Loss_VP9',
         test_rect=[0, 0, 240, 135],
         tolerance=tolerance_vp9,
-        expected_colors=_FOUR_COLOR_VIDEO_240x135_EXPECTED_COLORS),
+        expected_colors=_FOUR_COLOR_VIDEO_240x135_EXPECTED_COLORS,
+        expected_per_process_crashes={
+          CRASH_TYPE_GPU: 1,
+        }),
 
       PixelTestPage(
         'pixel_video_backdrop_filter.html',

tools/perf/core/minidump_unittest.py

@@ -147,6 +147,10 @@ class BrowserMinidumpTest(tab_test_case.TabTestCase):
     self.assertEquals(after_symbolize_all_unsymbolized_paths,
         [first_crash_path])
 
+    # Explicitly ignore the remaining minidump so that it isn't detected during
+    # teardown by the test runner.
+    self._browser.IgnoreMinidump(first_crash_path)
+
   def _LoadPageThenWait(self, script, value):
     # We are occasionally seeing these tests fail on the first load and
     # call to GetMostRecentMinidumpPath, where the directory is coming up empty.