Append (instead of replace) gpu driver bug workarounds from commandline.

Before if we specify a workaround through commandline, then
we bypass loading them from the built-in json list.

After this, we append it to the workarounds from the
built-in json list, not overwriting them.

If we don't want the workarounds from the json list, we can
specify "--disable-gpu-driver-bug-workarounds", and on top
of that, adding any workarounds we want.

This provides most flexibility for testing purpose.

BUG=359348
TEST=gpu_unittests,manual
R=kbr@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@261232 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/gpu/gpu_data_manager_impl_private.cc

@@ -304,21 +304,6 @@ void ApplyAndroidWorkarounds(const gpu::GPUInfo& gpu_info,
 }
 #endif  // OS_ANDROID
 
-// Overwrite force gpu workaround if a commandline switch exists.
-void AdjustGpuSwitchingOption(std::set<int>* workarounds) {
-  DCHECK(workarounds);
-  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
-  std::string option = command_line.GetSwitchValueASCII(
-      switches::kGpuSwitching);
-  if (option == switches::kGpuSwitchingOptionNameForceDiscrete) {
-    workarounds->erase(gpu::FORCE_INTEGRATED_GPU);
-    workarounds->insert(gpu::FORCE_DISCRETE_GPU);
-  } else if (option == switches::kGpuSwitchingOptionNameForceIntegrated) {
-    workarounds->erase(gpu::FORCE_DISCRETE_GPU);
-    workarounds->insert(gpu::FORCE_INTEGRATED_GPU);
-  }
-}
-
 // Block all domains' use of 3D APIs for this many milliseconds if
 // approaching a threshold where system stability might be compromised.
 const int64 kBlockAllDomainsMs = 10000;
@@ -614,13 +599,12 @@ void GpuDataManagerImplPrivate::UpdateGpuInfoHelper() {
 
     UpdateBlacklistedFeatures(features);
   }
-  gpu_driver_bugs_ =
-      gpu::WorkaroundsFromCommandLine(CommandLine::ForCurrentProcess());
-  if (gpu_driver_bugs_.empty() && gpu_driver_bug_list_) {
+  if (gpu_driver_bug_list_) {
     gpu_driver_bugs_ = gpu_driver_bug_list_->MakeDecision(
         gpu::GpuControlList::kOsAny, std::string(), gpu_info_);
   }
-  AdjustGpuSwitchingOption(&gpu_driver_bugs_);
+  gpu::GpuDriverBugList::AppendWorkaroundsFromCommandLine(
+      &gpu_driver_bugs_, *CommandLine::ForCurrentProcess());
 
   // We have to update GpuFeatureType before notify all the observers.
   NotifyGpuInfoUpdate();

gpu/config/gpu_driver_bug_list.cc

@@ -12,7 +12,12 @@ namespace gpu {
 
 namespace {
 
-const DriverBugInfo kFeatureList[] = {
+struct GpuDriverBugWorkaroundInfo {
+  GpuDriverBugWorkaroundType type;
+  const char* name;
+};
+
+const GpuDriverBugWorkaroundInfo kFeatureList[] = {
 #define GPU_OP(type, name) { type, #name },
   GPU_DRIVER_BUG_WORKAROUNDS(GPU_OP)
 #undef GPU_OP
@@ -34,22 +39,50 @@ GpuDriverBugList* GpuDriverBugList::Create() {
   DCHECK_EQ(static_cast<int>(arraysize(kFeatureList)),
             NUMBER_OF_GPU_DRIVER_BUG_WORKAROUND_TYPES);
   for (int i = 0; i < NUMBER_OF_GPU_DRIVER_BUG_WORKAROUND_TYPES; ++i) {
-    list->AddSupportedFeature(kFeatureList[i].feature_name,
-                              kFeatureList[i].feature_type);
+    list->AddSupportedFeature(kFeatureList[i].name,
+                              kFeatureList[i].type);
   }
   return list;
 }
 
 std::string GpuDriverBugWorkaroundTypeToString(
     GpuDriverBugWorkaroundType type) {
   if (type < NUMBER_OF_GPU_DRIVER_BUG_WORKAROUND_TYPES)
-    return kFeatureList[type].feature_name;
+    return kFeatureList[type].name;
   else
     return "unknown";
 }
 
-const struct DriverBugInfo* GetDriverBugWorkarounds() {
-  return kFeatureList;
+// static
+void GpuDriverBugList::AppendWorkaroundsFromCommandLine(
+    std::set<int>* workarounds, const CommandLine& command_line) {
+  DCHECK(workarounds);
+  for (int i = 0; i < NUMBER_OF_GPU_DRIVER_BUG_WORKAROUND_TYPES; i++) {
+    if (!command_line.HasSwitch(kFeatureList[i].name))
+      continue;
+    // Removing conflicting workarounds.
+    switch (kFeatureList[i].type) {
+      case FORCE_DISCRETE_GPU:
+        workarounds->erase(FORCE_INTEGRATED_GPU);
+        workarounds->insert(FORCE_DISCRETE_GPU);
+        break;
+      case FORCE_INTEGRATED_GPU:
+        workarounds->erase(FORCE_DISCRETE_GPU);
+        workarounds->insert(FORCE_INTEGRATED_GPU);
+        break;
+      case MAX_CUBE_MAP_TEXTURE_SIZE_LIMIT_512:
+      case MAX_CUBE_MAP_TEXTURE_SIZE_LIMIT_1024:
+      case MAX_CUBE_MAP_TEXTURE_SIZE_LIMIT_4096:
+        workarounds->erase(MAX_CUBE_MAP_TEXTURE_SIZE_LIMIT_512);
+        workarounds->erase(MAX_CUBE_MAP_TEXTURE_SIZE_LIMIT_1024);
+        workarounds->erase(MAX_CUBE_MAP_TEXTURE_SIZE_LIMIT_4096);
+        workarounds->insert(kFeatureList[i].type);
+        break;
+      default:
+        workarounds->insert(kFeatureList[i].type);
+        break;
+    }
+  }
 }
 
 }  // namespace gpu

gpu/config/gpu_driver_bug_list.h

@@ -5,8 +5,10 @@
 #ifndef GPU_CONFIG_GPU_DRIVER_BUG_LIST_H_
 #define GPU_CONFIG_GPU_DRIVER_BUG_LIST_H_
 
+#include <set>
 #include <string>
 
+#include "base/command_line.h"
 #include "gpu/config/gpu_control_list.h"
 #include "gpu/config/gpu_driver_bug_workaround_type.h"
 #include "gpu/gpu_export.h"
@@ -19,19 +21,17 @@ class GPU_EXPORT GpuDriverBugList : public GpuControlList {
 
   static GpuDriverBugList* Create();
 
+  // Append |workarounds| with these passed in through the
+  // |command_line|.
+  static void AppendWorkaroundsFromCommandLine(
+      std::set<int>* workarounds, const CommandLine& command_line);
+
  private:
   GpuDriverBugList();
 
   DISALLOW_COPY_AND_ASSIGN(GpuDriverBugList);
 };
 
-struct DriverBugInfo {
-  GpuDriverBugWorkaroundType feature_type;
-  const char* feature_name;
-};
-
-const struct DriverBugInfo* GetDriverBugWorkarounds();
-
 }  // namespace gpu
 
 #endif  // GPU_CONFIG_GPU_DRIVER_BUG_LIST_H_

gpu/config/gpu_driver_bug_list_unittest.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/command_line.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "gpu/config/gpu_control_list_jsons.h"
@@ -125,5 +126,35 @@ TEST_F(GpuDriverBugListTest, GpuSwitching) {
   EXPECT_EQ(2u, entries[0]);
 }
 
+TEST_F(GpuDriverBugListTest, AppendSingleWorkaround) {
+  base::CommandLine command_line(0, NULL);
+  command_line.AppendSwitch(
+      GpuDriverBugWorkaroundTypeToString(DISABLE_MULTISAMPLING));
+  std::set<int> workarounds;
+  workarounds.insert(EXIT_ON_CONTEXT_LOST);
+  workarounds.insert(INIT_VERTEX_ATTRIBUTES);
+  EXPECT_EQ(2u, workarounds.size());
+  GpuDriverBugList::AppendWorkaroundsFromCommandLine(
+      &workarounds, command_line);
+  EXPECT_EQ(3u, workarounds.size());
+  EXPECT_EQ(1u, workarounds.count(DISABLE_MULTISAMPLING));
+}
+
+TEST_F(GpuDriverBugListTest, AppendForceGPUWorkaround) {
+  base::CommandLine command_line(0, NULL);
+  command_line.AppendSwitch(
+      GpuDriverBugWorkaroundTypeToString(FORCE_DISCRETE_GPU));
+  std::set<int> workarounds;
+  workarounds.insert(EXIT_ON_CONTEXT_LOST);
+  workarounds.insert(FORCE_INTEGRATED_GPU);
+  EXPECT_EQ(2u, workarounds.size());
+  EXPECT_EQ(1u, workarounds.count(FORCE_INTEGRATED_GPU));
+  GpuDriverBugList::AppendWorkaroundsFromCommandLine(
+      &workarounds, command_line);
+  EXPECT_EQ(2u, workarounds.size());
+  EXPECT_EQ(0u, workarounds.count(FORCE_INTEGRATED_GPU));
+  EXPECT_EQ(1u, workarounds.count(FORCE_DISCRETE_GPU));
+}
+
 }  // namespace gpu
 

gpu/config/gpu_util.cc

@@ -65,11 +65,10 @@ void ApplyGpuDriverBugWorkarounds(
   scoped_ptr<GpuDriverBugList> list(GpuDriverBugList::Create());
   list->LoadList(kGpuDriverBugListJson,
                  GpuControlList::kCurrentOsOnly);
-  std::set<int> workarounds = WorkaroundsFromCommandLine(command_line);
-  if (workarounds.empty()) {
-    workarounds = list->MakeDecision(
-        GpuControlList::kOsAny, std::string(), gpu_info);
-  }
+  std::set<int> workarounds = list->MakeDecision(
+      GpuControlList::kOsAny, std::string(), gpu_info);
+  GpuDriverBugList::AppendWorkaroundsFromCommandLine(
+      &workarounds, *command_line);
   if (!workarounds.empty()) {
     command_line->AppendSwitchASCII(switches::kGpuDriverBugWorkarounds,
                                     IntSetToString(workarounds));
@@ -81,16 +80,4 @@ void StringToFeatureSet(
   StringToIntSet(str, feature_set);
 }
 
-std::set<int> WorkaroundsFromCommandLine(CommandLine* command_line) {
-  std::set<int> workarounds;
-  const struct DriverBugInfo* kFeatureList = GetDriverBugWorkarounds();
-
-  for (int i = 0; i < NUMBER_OF_GPU_DRIVER_BUG_WORKAROUND_TYPES; i++) {
-    if (command_line->HasSwitch(kFeatureList[i].feature_name))
-      workarounds.insert(kFeatureList[i].feature_type);
-  }
-
-  return workarounds;
-}
-
 }  // namespace gpu

gpu/config/gpu_util.h

@@ -37,10 +37,6 @@ GPU_EXPORT void ApplyGpuDriverBugWorkarounds(
 GPU_EXPORT void StringToFeatureSet(
     const std::string& str, std::set<int>* feature_set);
 
-// Get the set of workarounds from switches provided in |command_line|
-GPU_EXPORT std::set<int> WorkaroundsFromCommandLine(
-    base::CommandLine* command_line);
-
 }  // namespace gpu
 
 #endif  // GPU_CONFIG_GPU_UTIL_H_