Improve blacklist logic: use more fields (driver_vendor, gl_renderer, ect) fo...

chrome/browser/gpu_blacklist.cc

Index: chrome/browser/gpu_blacklist.cc
===================================================================
--- chrome/browser/gpu_blacklist.cc	(revision 72173)
+++ chrome/browser/gpu_blacklist.cc	(working copy)
@@ -135,6 +135,48 @@
   return kOsUnknown;
 }
 
+GpuBlacklist::StringInfo::StringInfo(const std::string& string_op,
+                                     const std::string& string_value) {
+  op_ = StringToOp(string_op);
+  value_ = StringToLowerASCII(string_value);
+}
+
+GpuBlacklist::StringInfo::~StringInfo() {

[Vangelis Kokkevis, 2011/01/24 20:55:21]

No point in defining a destructor if you're not doing anything special in it.

[Zhenyao Mo, 2011/01/24 22:38:22]

Removed.

On 2011/01/24 20:55:21, Vangelis Kokkevis wrote:

+}
+
+bool GpuBlacklist::StringInfo::Contains(const std::string& value) const {
+  std::string my_value = StringToLowerASCII(value);
+  switch (op_) {
+    case kContains:
+      return strstr(my_value.c_str(), value_.c_str()) != NULL;
+    case kBeginWith:
+      return StartsWithASCII(my_value, value_, false);
+    case kEndWith:
+      return EndsWith(my_value, value_, false);
+    case kEQ:
+      return value_ == my_value;
+    default:
+      return false;
+  }
+}
+
+bool GpuBlacklist::StringInfo::IsValid() const {
+  return op_ != kUnknown;
+}
+
+GpuBlacklist::StringInfo::Op GpuBlacklist::StringInfo::StringToOp(
+    const std::string& string_op) {
+  if (string_op == "=")
+    return kEQ;
+  else if (string_op == "contains")
+    return kContains;
+  else if (string_op == "beginwith")
+    return kBeginWith;
+  else if (string_op == "endwith")
+    return kEndWith;
+  return kUnknown;
+}
+
 GpuBlacklist::GpuBlacklistEntry*
 GpuBlacklist::GpuBlacklistEntry::GetGpuBlacklistEntryFromValue(
     DictionaryValue* value) {
@@ -143,6 +185,12 @@
 
   GpuBlacklistEntry* entry = new GpuBlacklistEntry();
 
+  std::string id;
+  if (!value->GetString("id", &id) || !entry->SetId(id)) {
+    delete entry;
+    return NULL;
+  }
+
   DictionaryValue* os_value = NULL;
   if (value->GetDictionary("os", &os_value)) {
     std::string os_type;
@@ -179,6 +227,18 @@
     }
   }
 
+  DictionaryValue* driver_vendor_value = NULL;
+  if (value->GetDictionary("driver_vendor", &driver_vendor_value)) {
+    std::string vendor_op;
+    std::string vendor_value;
+    driver_vendor_value->GetString("op", &vendor_op);
+    driver_vendor_value->GetString("value", &vendor_value);
+    if (!entry->SetDriverVendorInfo(vendor_op, vendor_value)) {
+      delete entry;
+      return NULL;
+    }
+  }
+
   DictionaryValue* driver_version_value = NULL;
   if (value->GetDictionary("driver_version", &driver_version_value)) {
     std::string driver_version_op = "any";
@@ -194,6 +254,18 @@
     }
   }
 
+  DictionaryValue* gl_renderer_value = NULL;
+  if (value->GetDictionary("gl_renderer", &gl_renderer_value)) {
+    std::string renderer_op;
+    std::string renderer_value;
+    gl_renderer_value->GetString("op", &renderer_op);
+    gl_renderer_value->GetString("value", &renderer_value);
+    if (!entry->SetGLRendererInfo(renderer_op, renderer_value)) {
+      delete entry;
+      return NULL;
+    }
+  }
+
   ListValue* blacklist_value = NULL;
   if (!value->GetList("blacklist", &blacklist_value)) {
     delete entry;
@@ -224,6 +296,14 @@
       device_id_(0) {
 }
 
+bool GpuBlacklist::GpuBlacklistEntry::SetId(
+    const std::string& id_string) {
+  id_ = 0;
+  return (base::HexStringToInt(id_string,
+                               reinterpret_cast<int*>(&id_)) &&

[Vangelis Kokkevis, 2011/01/24 20:55:21]

reinterpret_cast 's are kind of scary and potentially unsafe.  Would static_cast
work in this case?  If not, could you split the conversion to an int to a
separate step and then convert to a uint32?

[Zhenyao Mo, 2011/01/24 22:38:22]

Done.

+          id_ != 0);
+}
+
 bool GpuBlacklist::GpuBlacklistEntry::SetOsInfo(
     const std::string& os,
     const std::string& version_op,
@@ -247,6 +327,14 @@
                               reinterpret_cast<int*>(&device_id_));
 }
 
+bool GpuBlacklist::GpuBlacklistEntry::SetDriverVendorInfo(
+    const std::string& vendor_op,
+    const std::string& vendor_value) {
+  driver_vendor_info_.reset(
+      new StringInfo(vendor_op, vendor_value));
+  return driver_vendor_info_->IsValid();
+}
+
 bool GpuBlacklist::GpuBlacklistEntry::SetDriverVersionInfo(
     const std::string& version_op,
     const std::string& version_string,
@@ -256,6 +344,14 @@
   return driver_version_info_->IsValid();
 }
 
+bool GpuBlacklist::GpuBlacklistEntry::SetGLRendererInfo(
+    const std::string& renderer_op,
+    const std::string& renderer_value) {
+  gl_renderer_info_.reset(
+      new StringInfo(renderer_op, renderer_value));
+  return gl_renderer_info_->IsValid();
+}
+
 bool GpuBlacklist::GpuBlacklistEntry::SetBlacklistedFeatures(
     const std::vector<std::string>& blacklisted_features) {
   size_t size = blacklisted_features.size();
@@ -284,7 +380,9 @@
 bool GpuBlacklist::GpuBlacklistEntry::Contains(
     OsType os_type, const Version& os_version,
     uint32 vendor_id, uint32 device_id,
-    const Version& driver_version) const {
+    const std::string& driver_vendor,
+    const Version& driver_version,
+    const std::string& gl_renderer) const {
   DCHECK(os_type != kOsAny);
   if (os_info_.get() != NULL && !os_info_->Contains(os_type, os_version))
     return false;
@@ -292,9 +390,16 @@
     return false;
   if (device_id_ != 0 && device_id_ != device_id)
     return false;
-  if (driver_version_info_.get() == NULL)
-    return true;
-  return driver_version_info_->Contains(driver_version);
+  if (driver_vendor_info_.get() != NULL &&
+      !driver_vendor_info_->Contains(driver_vendor))
+    return false;
+  if (driver_version_info_.get() != NULL &&
+      !driver_version_info_->Contains(driver_version))
+    return false;
+  if (gl_renderer_info_.get() != NULL &&
+      !gl_renderer_info_->Contains(gl_renderer))
+    return false;
+  return true;
 }
 
 GpuBlacklist::OsType GpuBlacklist::GpuBlacklistEntry::GetOsType() const {
@@ -303,6 +408,10 @@
   return os_info_->type();
 }
 
+uint32 GpuBlacklist::GpuBlacklistEntry::id() const {
+  return id_;
+}
+
 GpuFeatureFlags GpuBlacklist::GpuBlacklistEntry::GetGpuFeatureFlags() const {
   return *feature_flags_;
 }
@@ -322,11 +431,20 @@
   if (root.get() == NULL || !root->IsType(Value::TYPE_DICTIONARY))
     return false;
 
+  DictionaryValue* root_dictionary = static_cast<DictionaryValue*>(root.get());
+  DCHECK(root_dictionary);
+  std::string version_string;
+  root_dictionary->GetString("version", &version_string);
+  version_.reset(Version::GetVersionFromString(version_string));
+  if (version_.get() == NULL)
+    return false;
+
   ListValue* list = NULL;
-  static_cast<DictionaryValue*>(root.get())->GetList("entries", &list);
+  root_dictionary->GetList("entries", &list);
   if (list == NULL)
     return false;
 
+  uint32 max_entry_id = 0;
   for (size_t i = 0; i < list->GetSize(); ++i) {
     DictionaryValue* list_item = NULL;
     bool valid = list->GetDictionary(i, &list_item);
@@ -336,6 +454,8 @@
         GpuBlacklistEntry::GetGpuBlacklistEntryFromValue(list_item);
     if (entry == NULL)
       break;
+    if (entry->id() > max_entry_id)
+      max_entry_id = entry->id();
     entries.push_back(entry);
   }
 
@@ -358,13 +478,16 @@
         delete entries[i];
     }
   }
+  max_entry_id_ = max_entry_id;
   return true;
 }
 
 GpuFeatureFlags GpuBlacklist::DetermineGpuFeatureFlags(
     GpuBlacklist::OsType os,
     Version* os_version,
-    const GPUInfo& gpu_info) const {
+    const GPUInfo& gpu_info) {
+  features_.clear();
+  entry_ids_.clear();
   GpuFeatureFlags flags;
   // No need to go through blacklist entries if GPUInfo isn't available.
   if (gpu_info.progress() == GPUInfo::kUninitialized)
@@ -403,13 +526,46 @@
   for (size_t i = 0; i < blacklist_.size(); ++i) {
     if (blacklist_[i]->Contains(os, *os_version,
                                 gpu_info.vendor_id(), gpu_info.device_id(),
-                                *driver_version)) {
+                                gpu_info.driver_vendor(),
+                                *driver_version,
+                                gpu_info.gl_renderer())) {
       flags.Combine(blacklist_[i]->GetGpuFeatureFlags());
+      features_.push_back(blacklist_[i]->GetGpuFeatureFlags().flags());
+      entry_ids_.push_back(blacklist_[i]->id());
     }
   }
   return flags;
 }
 
+void GpuBlacklist::GetGpuFeatureFlagEntries(
+    GpuFeatureFlags::GpuFeatureType feature,
+    std::vector<uint32>& entry_ids) const {
+  DCHECK(features_.size() == entry_ids_.size());
+  entry_ids.clear();
+  for (size_t i = 0; i < features_.size(); ++i) {
+      if ((feature & features_[i]) != 0)
+          entry_ids.push_back(entry_ids_[i]);
+  }
+}
+
+uint32 GpuBlacklist::max_entry_id() const {
+  return max_entry_id_;
+}
+
+bool GpuBlacklist::GetVersion(uint16* major, uint16* minor) const {
+  DCHECK(major && minor);
+  *major = 0;
+  *minor = 0;
+  if (version_.get() == NULL)
+    return false;
+  const std::vector<uint16>& components_reference = version_->components();
+  if (components_reference.size() != 2)
+    return false;
+  *major = components_reference[0];
+  *minor = components_reference[1];
+  return true;
+}
+
 GpuBlacklist::OsType GpuBlacklist::GetOsType() {
 #if defined(OS_WIN)
   return kOsWin;