Fix ineffective --disable-gpu-driver-bug-workarounds

content/test/gpu/page_sets/gpu_process_tests.py

 # Copyright 2014 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 sys
 from telemetry.story import story_set as story_set_module
 from telemetry.page import page_test
 
 from gpu_tests import gpu_test_base
 
 class GpuProcessSharedPageState(gpu_test_base.GpuSharedPageState):
@@ skipping 102 matching lines @@
       return
 
     if self.expected_workaround:
       self._Validate(tab, "browser_process", True, self.expected_workaround)
       self._Validate(tab, "gpu_process", True, self.expected_workaround)
 
     if self.unexpected_workaround:
       self._Validate(tab, "browser_process", False, self.unexpected_workaround)
       self._Validate(tab, "gpu_process", False, self.unexpected_workaround)
 
+
+class EqualBugWorkaroundsBasePage(gpu_test_base.PageBase):
+  def __init__(self, name=None, page_set=None, shared_page_state_class=None,
+               expectations=None):
+    super(EqualBugWorkaroundsBasePage, self).__init__(
+      url='chrome:gpu',
+      name=name,
+      page_set=page_set,
+      shared_page_state_class=shared_page_state_class,
+      expectations=expectations)
+
+  def Validate(self, tab, results):
+    browser_list = tab.EvaluateJavaScript('GetDriverBugWorkarounds()')
+    gpu_list = tab.EvaluateJavaScript( \
+      'chrome.gpuBenchmarking.getGpuDriverBugWorkarounds()')
+
+    diff = set(browser_list).symmetric_difference(set(gpu_list))
+    if len(diff) > 0:
+      print 'Test failed. Printing page contents:'
+      print tab.EvaluateJavaScript('document.body.innerHTML')
+      raise page_test.Failure('Browser and GPU process list of driver bug' \
+        'workarounds are not equal: %s != %s, diff: %s' % \
+        (browser_list, gpu_list, list(diff)))
+
+    return gpu_list
+
+
 class GpuProcessTestsPage(gpu_test_base.PageBase):
   def __init__(self, url, name, story_set, expectations):
     super(GpuProcessTestsPage, self).__init__(url=url,
         shared_page_state_class=gpu_test_base.GpuSharedPageState,
         page_set=story_set,
         name=name,
         expectations=expectations)
 
 
 class FunctionalVideoPage(GpuProcessTestsPage):
@@ skipping 358 matching lines @@
           result = result and status == 'disabled_software'
         elif name == 'webgl':
           result = result and status == 'enabled_readback'
         else:
           result = result and status == 'unavailable_software'
       if not result:
         raise page_test.Failure('WebGL readback setup failed: %s' \
           % feature_status_list)
 
 
-class EqualBugWorkaroundsInBrowserAndGpuProcessPage(gpu_test_base.PageBase):
-  def __init__(self, story_set, expectations):
-    super(EqualBugWorkaroundsInBrowserAndGpuProcessPage, self).__init__(
-      url='chrome:gpu',
-      name='GpuProcess.equal_bug_workarounds_in_browser_and_gpu_process',
-      page_set=story_set,
-      shared_page_state_class=GpuProcessSharedPageState,
-      expectations=expectations)
-
-  def Validate(self, tab, results):
-    browser_list = tab.EvaluateJavaScript('GetDriverBugWorkarounds()')
-    gpu_list = tab.EvaluateJavaScript( \
-      'chrome.gpuBenchmarking.getGpuDriverBugWorkarounds()')
-
-    diff = set(browser_list).symmetric_difference(set(gpu_list))
-    if len(diff) > 0:
-      print 'Test failed. Printing page contents:'
-      print tab.EvaluateJavaScript('document.body.innerHTML')
-      raise page_test.Failure('Browser and GPU process list of driver bug' \
-        'workarounds are not equal: %s != %s, diff: %s' % \
-        (browser_list, gpu_list, list(diff)))
-
-
 class HasTransparentVisualsShared(GpuProcessSharedPageState):
   def __init__(self, test, finder_options, story_set):
     super(HasTransparentVisualsShared, self).__init__(
       test, finder_options, story_set)
     options = finder_options.browser_options
     if sys.platform.startswith('linux'):
       # Hit id 173 from kGpuDriverBugListJson.
       options.AppendExtraBrowserArgs('--gpu-testing-gl-version=3.0 Mesa ' \
                                      '12.1')
 
@@ skipping 28 matching lines @@
       shared_page_state_class=NoTransparentVisualsShared,
       expectations=expectations,
       expected_workaround='disable_transparent_visuals',
       unexpected_workaround=None)
 
   def Validate(self, tab, results):
     if sys.platform.startswith('linux'):
       super(NoTransparentVisualsGpuProcessPage, self).Validate(tab, results)
 
 
+class TransferWorkaroundSharedPageState(GpuProcessSharedPageState):
+  def __init__(self, test, finder_options, story_set):
+    super(TransferWorkaroundSharedPageState, self).__init__(
+      test, finder_options, story_set)
+
+  # Extra browser args need to be added here. Adding them in RunStory would be
+  # too late.
+  def WillRunStory(self, results):
+    if self.WarmUpDone():
+      options = self._finder_options.browser_options
+      options.AppendExtraBrowserArgs('--use_gpu_driver_workaround_for_testing')
+      options.AppendExtraBrowserArgs('--disable-gpu-driver-bug-workarounds')
+
+      # Inject some info to make sure the flag above is effective.
+      if sys.platform == 'darwin':
+        # Hit id 33 from kGpuDriverBugListJson.
+        options.AppendExtraBrowserArgs('--gpu-testing-gl-vendor=Imagination')
+      else:
+        # Hit id 5 from kGpuDriverBugListJson.
+        options.AppendExtraBrowserArgs('--gpu-testing-vendor-id=0x10dee')

[Ken Russell (switch to Gerrit), 2016/08/17 06:52:22]

This looks like a typo, and that it should be 0x10de. If that's true -- how did
this pass the new test?

[Julien Isorce Samsung, 2016/08/17 07:49:13]

Oh sorry it is a left over of some local testing. Locally I also changed the
value in entry 5 in gpu_driver_bug_list_json.cc.
The reason why it passes the CQ dry dun is that it does not trigger any
additional workaround since it does not match any entry. But even though this
bit is to trigger the bug in gpu_process_host.cc that this CL also fixes. So to
make the test fail you need to remove the fix in gpu_process_host.cc and make it
0x10de. Then it will trigger the entry 5 and fails the test because it will be
different than the recorded list.
Of course this part is only useful on non nvidia hardware but I guess we can
assume there will be always tests on other hardwares.

Thx for catching that!. About simplifying the test using class attributes, what
do you think ?

[Ken Russell (switch to Gerrit), 2016/08/17 18:48:42]

It's concerning to me that it would have been possible for that to slip in and
not fail any test. Please give this more thought.
Why don't you do that in a follow-on patch. This one has been dragging on for
some time.

+        options.AppendExtraBrowserArgs('--gpu-testing-device-id=')
+        options.AppendExtraBrowserArgs('--gpu-testing-secondary-vendor-ids=')
+        options.AppendExtraBrowserArgs('--gpu-testing-secondary-device-ids=')
+
+      for workaround in self.RecordedWorkarounds():
+        options.AppendExtraBrowserArgs('--' + workaround)
+    super(TransferWorkaroundSharedPageState, self).WillRunStory(results)
+
+  # self._current_page is None in WillRunStory so do the transfer here.
+  def RunStory(self, results):
+    if self.WarmUpDone():
+      self._current_page.expected_workarounds = self.RecordedWorkarounds()
+      self.AddTestingWorkaround(self._current_page.expected_workarounds)
+    super(TransferWorkaroundSharedPageState, self).RunStory(results)
+
+  def WarmUpDone(self):
+    return self._previous_page is not None and \
+           self.RecordedWorkarounds() is not None
+
+  def RecordedWorkarounds(self):
+    return self._previous_page.recorded_workarounds
+
+  def AddTestingWorkaround(self, workarounds):
+    if workarounds is not None:
+      workarounds.append('use_gpu_driver_workaround_for_testing')
+
+
+class EqualBugWorkaroundsPage(EqualBugWorkaroundsBasePage):
+  def __init__(self, story_set, expectations):
+    super(EqualBugWorkaroundsPage, self).__init__(
+      name='GpuProcess.equal_bug_workarounds_in_browser_and_gpu_process',
+      page_set=story_set,
+      shared_page_state_class=TransferWorkaroundSharedPageState,
+      expectations=expectations)
+    self.recorded_workarounds = None
+
+  def Validate(self, tab, results):
+    gpu_list = super(EqualBugWorkaroundsPage, self).Validate(tab, results)
+    self.recorded_workarounds = gpu_list
+
+
+class OnlyOneWorkaroundPage(EqualBugWorkaroundsBasePage):
+  def __init__(self, story_set, expectations):
+    super(OnlyOneWorkaroundPage, self).__init__(
+      name='GpuProcess.only_one_workaround',
+      page_set=story_set,
+      shared_page_state_class=TransferWorkaroundSharedPageState,
+      expectations=expectations)
+    self.expected_workarounds = None
+
+  def Validate(self, tab, results):
+    # Requires EqualBugWorkaroundsPage to succeed. If it has failed then just
+    # pass to not overload the logs.
+    if self.expected_workarounds is None:
+      return
+
+    gpu_list = super(OnlyOneWorkaroundPage, self).Validate(tab, results)
+
+    diff = set(self.expected_workarounds).symmetric_difference(set(gpu_list))
+    if len(diff) > 0:
+      print 'Test failed. Printing page contents:'
+      print tab.EvaluateJavaScript('document.body.innerHTML')
+      raise page_test.Failure('GPU process and expected list of driver bug' \
+        'workarounds are not equal: %s != %s, diff: %s' % \
+        (self.expected_workarounds, gpu_list, list(diff)))
+
+
 class GpuProcessTestsStorySet(story_set_module.StorySet):
 
   """ Tests that accelerated content triggers the creation of a GPU process """
 
   def __init__(self, expectations, is_platform_android):
     super(GpuProcessTestsStorySet, self).__init__(
       serving_dirs=set(['../../../../content/test/data']))
 
     urls_and_names_list = [
       ('file://../../data/gpu/functional_canvas_demo.html',
@@ skipping 11 matching lines @@
     self.AddStory(GpuInfoCompletePage(self, expectations))
     self.AddStory(NoGpuProcessPage(self, expectations))
     self.AddStory(SoftwareGpuProcessPage(self, expectations))
     self.AddStory(DriverBugWorkaroundsInGpuProcessPage(self, expectations))
     self.AddStory(IdentifyActiveGpuPage1(self, expectations))
     self.AddStory(IdentifyActiveGpuPage2(self, expectations))
     self.AddStory(IdentifyActiveGpuPage3(self, expectations))
     self.AddStory(IdentifyActiveGpuPage4(self, expectations))
     self.AddStory(ReadbackWebGLGpuProcessPage(self, expectations))
     self.AddStory(DriverBugWorkaroundsUponGLRendererPage(self, expectations))
-    self.AddStory(EqualBugWorkaroundsInBrowserAndGpuProcessPage(self,
-                                                                expectations))
+    self.AddStory(EqualBugWorkaroundsPage(self, expectations))
+    self.AddStory(OnlyOneWorkaroundPage(self, expectations))
+
     if not is_platform_android:
       self.AddStory(SkipGpuProcessPage(self, expectations))
       self.AddStory(HasTransparentVisualsGpuProcessPage(self, expectations))
       self.AddStory(NoTransparentVisualsGpuProcessPage(self, expectations))
 
   @property
   def allow_mixed_story_states(self):
     # Return True here in order to be able to run pages with different browser
     # command line arguments.
     return True