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

chrome/browser/gpu_blacklist.h

Index: chrome/browser/gpu_blacklist.h
===================================================================
--- chrome/browser/gpu_blacklist.h	(revision 72077)
+++ chrome/browser/gpu_blacklist.h	(working copy)
@@ -9,6 +9,7 @@
 // Determines whether certain gpu-related features are blacklisted or not.
 // A valid gpu_blacklist.json file are in the format of
 // {
+//   "version": "x.y",
 //   "entries": [
 //     { // entry 1
 //     },
@@ -25,16 +26,20 @@
 //    "version" is a VERSION structure (defined later).
 // 2. "vendor_id" has the value of a string.
 // 3. "device_id" has the value of a string.
-// 4. "driver_version" is a VERSION structure (defined later).
-// 5. "blacklist" is a list of gpu feature strings, valid values include
+// 4. "driver_vendor" is a STRING structure (defined later).
+// 5. "driver_version" is a VERSION structure (defined later).
+// 6. "gl_renderer" is a STRING structure (defined later).
+// 6. "blacklist" is a list of gpu feature strings, valid values include

[Ken Russell (switch to Gerrit), 2011/01/21 19:57:52]

6 -> 7

[Zhenyao Mo, 2011/01/21 21:51:40]

Done.

 //    "accelerated_2d_canvas", "accelerated_compositing", "webgl", and "all".
 //    Currently whatever feature is selected, the effect is the same as "all",
 //    i.e., it's not supported to turn off one GPU feature and not the others.
 // VERSION includes "op" "number", and "number2".  "op" can be any of the
-// following value: "=", "<", "<=", ">", ">=", "any", "between".  "number2" is
+// following values: "=", "<", "<=", ">", ">=", "any", "between".  "number2" is
 // only used if "op" is "between".  "number" is used for all "op" values except
 // "any". "number" and "number2" are in the format of x, x.x, x.x.x, ect.
 // Check out "gpu_blacklist_unittest.cc" for examples.
+// STRING includes "op" and "value".  "op" can be any of the following values:
+// "contains", "beginwith", "endwith", "=".  "value" is a string.
 
 #include <string>
 #include <vector>
@@ -74,8 +79,22 @@
   // current OS version.
   GpuFeatureFlags DetermineGpuFeatureFlags(OsType os,
                                            Version* os_version,
-                                           const GPUInfo& gpu_info) const;
+                                           const GPUInfo& gpu_info);
 
+  // Collects the entries that set the "feature" flag from the last
+  // DeterminGpuFeatureFlags() call.  This tells which entries are responsible

[Ken Russell (switch to Gerrit), 2011/01/21 19:57:52]

Determin -> Determine

[Zhenyao Mo, 2011/01/21 21:51:40]

Done.

+  // for raising a certain flag.
+  // The "feature" could be a single feature or a combination of multiple ones.

[Ken Russell (switch to Gerrit), 2011/01/21 19:57:52]

Could you improve this comment? It talks about features and flags, but these are
vague concepts. An example would help. Also, documenting what it means to have a
flag set would be helpful. It looks like having a feature flag set means that
it's blacklisted.

[Zhenyao Mo, 2011/01/21 21:51:40]

Done.

+  void GetGpuFeatureFlagEntries(GpuFeatureFlags::GpuFeatureType feature,
+                                std::vector<uint32>& entry_ids) const;
+
+  // Return the largest entry id.  This is used for histogram purpose.
+  uint32 max_entry_id() const;
+
+  // Collects the version of the current blacklist.  Return false and set major
+  // and minor to 0 on failure.
+  bool GetVersion(uint16* major, uint16* monir) const;
+
  private:
   class VersionInfo {
    public:
@@ -134,6 +153,33 @@
     scoped_ptr<VersionInfo> version_info_;
   };
 
+  class StringInfo {
+   public:
+    StringInfo(const std::string& string_op, const std::string& string_value);
+    ~StringInfo();
+
+    // Determines if a given string is included in the StringInfo.
+    bool Contains(const std::string& value) const;
+
+    // Determines if the StringInfo contains valid information.
+    bool IsValid() const;
+
+   private:
+    enum Op {
+      kContains,  // contains
+      kBeginWith,  // beginwith
+      kEndWith,  // endwith
+      kEQ,  // =
+      kUnknown  // Indicates StringInfo data is invalid.
+    };
+
+    // Maps string to Op; returns kUnknown if it's not a valid Op.
+    static Op StringToOp(const std::string& string_op);
+
+    Op op_;
+    std::string value_;
+  };
+
   class GpuBlacklistEntry {
    public:
     // Constructs GpuBlacklistEntry from DictionaryValue loaded from json.
@@ -143,11 +189,16 @@
     // Determines if a given os/gc/driver is included in the Entry set.
     bool 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;
 
     // Returns the OsType.
     OsType GetOsType() const;
 
+    // Returns the entry's unique id.  0 is preserved.
+    uint32 id() const;
+
     // Returns the GpuFeatureFlags.
     GpuFeatureFlags GetGpuFeatureFlags() const;
 
@@ -156,6 +207,8 @@
    private:
     GpuBlacklistEntry();
 
+    bool SetId(const std::string& id_string);
+
     bool SetOsInfo(const std::string& os,
                    const std::string& version_op,
                    const std::string& version_string,
@@ -165,17 +218,26 @@
 
     bool SetDeviceId(const std::string& device_id_string);
 
+    bool SetDriverVendorInfo(const std::string& vendor_op,
+                             const std::string& vendor_value);
+
     bool SetDriverVersionInfo(const std::string& version_op,
                               const std::string& version_string,
                               const std::string& version_string2);
 
+    bool SetGlRendererInfo(const std::string& renderer_op,

[Ken Russell (switch to Gerrit), 2011/01/21 19:57:52]

Gl -> GL

[Zhenyao Mo, 2011/01/21 21:51:40]

Done.

+                           const std::string& renderer_value);
+
     bool SetBlacklistedFeatures(
         const std::vector<std::string>& blacklisted_features);
 
+    uint32 id_;
     scoped_ptr<OsInfo> os_info_;
     uint32 vendor_id_;
     uint32 device_id_;
+    scoped_ptr<StringInfo> driver_vendor_info_;
     scoped_ptr<VersionInfo> driver_version_info_;
+    scoped_ptr<StringInfo> gl_renderer_info_;
     scoped_ptr<GpuFeatureFlags> feature_flags_;
   };
 
@@ -184,8 +246,17 @@
 
   void Clear();
 
+  scoped_ptr<Version> version_;
   std::vector<GpuBlacklistEntry*> blacklist_;
 
+  // These two vectors are updated everytime DetermineGpuFeatureFlags() is
+  // called.  They are used by GetGpuFeatureFlagEntries().
+  // Two vectors should always have the same number of elements.
+  std::vector<uint32> features_;
+  std::vector<uint32> entry_ids_;
+
+  uint32 max_entry_id_;
+
   DISALLOW_COPY_AND_ASSIGN(GpuBlacklist);
 };