Improve GPU software rendering list histograms.

content/browser/gpu/gpu_data_manager.cc

Index: content/browser/gpu/gpu_data_manager.cc
===================================================================
--- content/browser/gpu/gpu_data_manager.cc	(revision 96780)
+++ content/browser/gpu/gpu_data_manager.cc	(working copy)
@@ -12,6 +12,9 @@
 #include "base/metrics/histogram.h"
 #include "base/stringprintf.h"
 #include "base/string_number_conversions.h"
+#include "base/sys_info.h"
+#include "base/values.h"
+#include "base/version.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/gpu/gpu_blacklist.h"
 #include "content/browser/gpu/gpu_process_host.h"
@@ -79,6 +82,42 @@
 
 #endif  // OS_WIN
 
+std::string GetOSString() {
+  std::string rt;
+#if defined(OS_CHROMEOS)
+  rt = "ChromeOS";
+#elif defined(OS_WIN)
+  rt = "Win";
+  std::string version_str = base::SysInfo::OperatingSystemVersion();
+  size_t pos = version_str.find_first_not_of("0123456789.");
+  if (pos != std::string::npos)
+    version_str = version_str.substr(0, pos);
+  scoped_ptr<Version> os_version(Version::GetVersionFromString(version_str));
+  DCHECK(os_version.get());
+  const std::vector<uint16>& version_numbers = os_version->components();
+  DCHECK_GE(version_numbers.size(), 2);

[vangelis, 2011/08/17 17:40:17]

I think that in addition to the DCHECK we'll need to bail out safely if for some
reason we don't get enough fields in the version (to avoid nasty crashes).

[Zhenyao Mo, 2011/08/17 19:29:54]

Done.

+  if (version_numbers[0] == 5)
+    rt += "XP";
+  else if (version_numbers[0] == 6 && version_numbers[1] == 0)
+    rt += "Vista";
+  else if (version_numbers[0] == 6 && version_numbers[1] == 1)
+    rt += "7";
+#elif defined(OS_LINUX)
+  rt = "Linux";
+#elif defined(OS_MACOSX)
+  rt = "Mac";
+#else
+  rt = "UnknownOS";
+#endif
+  return rt;
+}
+
+void LogToHistogram(
+    const std::string& name, uint32 sample, uint32 boundary_value) {
+  base::LinearHistogram::FactoryGet(name, 1, boundary_value, boundary_value + 1,
+      base::Histogram::kUmaTargetedHistogramFlag)->Add(sample);
+}
+
 }  // namespace anonymous
 
 GpuDataManager::GpuDataManager()
@@ -361,24 +400,47 @@
         GpuBlacklist::kOsAny, NULL, gpu_info_);
   }
 
-  uint32 max_entry_id = gpu_blacklist->max_entry_id();
-  if (!gpu_feature_flags_.flags()) {
-    UMA_HISTOGRAM_ENUMERATION("GPU.BlacklistTestResultsPerEntry",
-        0, max_entry_id + 1);
-    return;
-  }
-
   // Notify clients that GpuInfo state has changed
   RunGpuInfoUpdateCallbacks();
 
-  // TODO(zmo): move histograming to GpuBlacklist::DetermineGpuFeatureFlags.
-  std::vector<uint32> flag_entries;
-  gpu_blacklist->GetGpuFeatureFlagEntries(
-      GpuFeatureFlags::kGpuFeatureAll, flag_entries);
-  DCHECK_GT(flag_entries.size(), 0u);
-  for (size_t i = 0; i < flag_entries.size(); ++i) {
-    UMA_HISTOGRAM_ENUMERATION("GPU.BlacklistTestResultsPerEntry",
-        flag_entries[i], max_entry_id + 1);
+  uint32 max_entry_id = gpu_blacklist->max_entry_id();
+
+  const GpuFeatureFlags::GpuFeatureType kGpuFeatures[] = {
+      GpuFeatureFlags::kGpuFeatureAccelerated2dCanvas,
+      GpuFeatureFlags::kGpuFeatureAcceleratedCompositing,
+      GpuFeatureFlags::kGpuFeatureWebgl
+  };
+  const std::string kGpuBlacklistHistogramPerEntryNames[] = {
+      "GPU.BlacklistAccelerated2dCanvasTestResultsPerEntry",
+      "GPU.BlacklistAcceleratedCompositingTestResultsPerEntry",
+      "GPU.BlacklistWebglTestResultsPerEntry"
+  };
+  const std::string kOSString = GetOSString();
+  const std::string kGpuBlacklistHistogramNames[] = {
+      "GPU.BlacklistAccelerated2dCanvasTestResults" + kOSString,
+      "GPU.BlacklistAcceleratedCompositingTestResults" + kOSString,
+      "GPU.BlacklistWebglTestResults" + kOSString
+  };
+
+  uint32 flags = gpu_feature_flags_.flags();
+
+  for (size_t i = 0;
+       i < sizeof(kGpuFeatures) / sizeof(GpuFeatureFlags::GpuFeatureType);
+       ++i) {
+    if ((flags & kGpuFeatures[i]) == 0) {
+      LogToHistogram(kGpuBlacklistHistogramPerEntryNames[i],
+          0, max_entry_id + 1);
+      LogToHistogram(kGpuBlacklistHistogramNames[i], 0, 2);

[vangelis, 2011/08/17 17:40:17]

Sorry, I should have spotted that earlier but I don't think we want to be adding
a tick to the kGpuBlacklistHistograPerEntry for every feature that's disabled. 
I think a better strategy would be to update that histogram if any of the
features is blacklisted.
 

[Zhenyao Mo, 2011/08/17 19:29:54]

Done.

+    } else {
+      std::vector<uint32> flag_entries;
+      gpu_blacklist->GetGpuFeatureFlagEntries(kGpuFeatures[i], flag_entries);
+      DCHECK_GT(flag_entries.size(), 0u);
+      for (size_t j = 0; j < flag_entries.size(); ++j) {
+        LogToHistogram(kGpuBlacklistHistogramPerEntryNames[i],
+            flag_entries[j], max_entry_id + 1);
+      }
+      LogToHistogram(kGpuBlacklistHistogramNames[i], 1, 2);
+    }
   }
 }