Update JsonCpp usage

DEPS

@@ -35,6 +35,17 @@ vars = {
   # Note: this dep cannot be auto-rolled b/c of nesting.
   'jsoncpp_revision': '645250b6690785be60ab6780ce4b58698d884d11',
 
+  # Current revision of Chrome's third_party jsoncpp directory. This repository
+  # is mirrored as a separate repository, with separate git hashes that
+  # don't match the external JsonCpp repository or Chrome. Mirrored patches
+  # will have a different git hash associated with them.
+  # To roll, first get the new hash for chromium_jsoncpp_revision from the
+  # mirror of third_party/jsoncpp located here:
+  # https://chromium.googlesource.com/chromium/src/third_party/jsoncpp/
+  # Then get the new hash for jsoncpp_revision from the root Chrome DEPS file:
+  # https://source.chromium.org/chromium/chromium/src/+/master:DEPS
+  'chromium_jsoncpp_revision': 'ec647b85b61f525a1a74e4da7477b0c5371c50f4',
+
   # Current revision of patched-yasm.
   # Note: this dep cannot be auto-rolled b/c of nesting.
   'patched_yasm_revision': '720b70524a4424b15fc57e82263568c8ba0496ad',
@@ -136,7 +147,7 @@ deps = {
   },
 
   'third_party/jsoncpp': {
-    'url': '{chromium_git}/chromium/src/third_party/jsoncpp@ec647b85b61f525a1a74e4da7477b0c5371c50f4',
+    'url': '{chromium_git}/chromium/src/third_party/jsoncpp@{chromium_jsoncpp_revision}',
     'condition': 'not build_with_chromium',
    },
 

src/feature_support_util/feature_support_util.cpp

@@ -17,7 +17,12 @@
 #    include <unistd.h>
 #endif
 #include <fstream>
+#include <ios>
 #include <list>
+#include <memory>
+#include <sstream>
+#include <utility>
+#include <vector>
 #include "../gpu_info_util/SystemInfo.h"
 
 namespace angle
@@ -124,9 +129,19 @@ constexpr char kJsonVerPatch[] = "VerPatch";
 class StringPart
 {
   public:
-    StringPart() : mPart(""), mWildcard(true) {}
-    StringPart(const std::string part) : mPart(part), mWildcard(false) {}
-    ~StringPart() {}
+    StringPart() = default;
+    explicit StringPart(const std::string part) : mPart(part), mWildcard(false) {}
+    ~StringPart() = default;
+
+    static StringPart FromJson(const Json::Value &parent, const char *key)
+    {
+        if (parent.isMember(key) && parent[key].isString())
+        {
+            return StringPart(parent[key].asString());
+        }
+        return {};
+    }
+
     bool match(const StringPart &toCheck) const
     {
         return (mWildcard || toCheck.mWildcard || (toCheck.mPart == mPart));
@@ -134,7 +149,7 @@ class StringPart
 
   public:
     std::string mPart;
-    bool mWildcard;
+    bool mWildcard = true;
 };
 
 // This encapsulates a 32-bit unsigned integer.  The default constructor (not given a number)
@@ -142,17 +157,27 @@ class StringPart
 class IntegerPart
 {
   public:
-    IntegerPart() : mPart(0), mWildcard(true) {}
-    IntegerPart(uint32_t part) : mPart(part), mWildcard(false) {}
-    ~IntegerPart() {}
+    IntegerPart() = default;
+    explicit IntegerPart(uint32_t part) : mPart(part), mWildcard(false) {}
+    ~IntegerPart() = default;
+
+    static IntegerPart FromJson(const Json::Value &parent, const char *key)
+    {
+        if (parent.isMember(key) && parent[key].isInt())
+        {
+            return IntegerPart(parent[key].asInt());
+        }
+        return {};
+    }
+
     bool match(const IntegerPart &toCheck) const
     {
         return (mWildcard || toCheck.mWildcard || (toCheck.mPart == mPart));
     }
 
   public:
-    uint32_t mPart;
-    bool mWildcard;
+    uint32_t mPart = 0;
+    bool mWildcard = true;
 };
 
 // This encapsulates a list of other classes, each of which will have a match() and logItem()
@@ -162,59 +187,61 @@ template <class T>
 class ListOf
 {
   public:
-    ListOf(const std::string listType) : mWildcard(true), mListType(listType) {}
+    explicit ListOf(const std::string listType) : mWildcard(true), mListType(listType) {}
     ~ListOf() { mList.clear(); }
-    void addItem(const T &toAdd)
+    void addItem(T &&toAdd)
     {
-        mList.push_back(toAdd);
+        mList.push_back(std::move(toAdd));
         mWildcard = false;
     }
     bool match(const T &toCheck) const
     {
-        VERBOSE("\t\t Within ListOf<%s> match: wildcards are %s and %s,\n", mListType.c_str(),
-                mWildcard ? "true" : "false", toCheck.mWildcard ? "true" : "false");
+        VERBOSE("\t\t Matching ListOf<%s> against item:\n", mListType.c_str());
         if (mWildcard || toCheck.mWildcard)
         {
+            VERBOSE("\t\t\t Successful match due to wildcard.\n");
             return true;
         }
         for (const T &it : mList)
         {
-            VERBOSE("\t\t   Within ListOf<%s> match: calling match on sub-item is %s,\n",
-                    mListType.c_str(), it.match(toCheck) ? "true" : "false");
             if (it.match(toCheck))
             {
+                VERBOSE("\t\t\t Successful match due to list item match.\n");
                 return true;
             }
         }
+        VERBOSE("\t\t\t Failed to match.\n");
         return false;
     }
     bool match(const ListOf<T> &toCheck) const
     {
-        VERBOSE("\t\t Within ListOf<%s> match: wildcards are %s and %s,\n", mListType.c_str(),
-                mWildcard ? "true" : "false", toCheck.mWildcard ? "true" : "false");
+        VERBOSE("\t\t Matching ListOf<%s>:\n", mListType.c_str());
         if (mWildcard || toCheck.mWildcard)
         {
+            VERBOSE("\t\t\t Successful match due to wildcard.\n");
             return true;
         }
-        // If we make it to here, both this and toCheck have at least one item in their mList
+        // If we make it to here, both this and toCheck have at least one item in their mList.
         for (const T &it : toCheck.mList)
         {
             if (match(it))
             {
+                VERBOSE("\t\t\t Successful match due to list item match.\n");
                 return true;
             }
         }
+        VERBOSE("\t\t\t Failed to match list.\n");
         return false;
     }
     void logListOf(const std::string prefix, const std::string name) const
     {
         if (mWildcard)
         {
-            VERBOSE("%sListOf%s is wildcarded to always match", prefix.c_str(), name.c_str());
+            VERBOSE("%sListOf%s is wildcarded to always match\n", prefix.c_str(), name.c_str());
         }
         else
         {
-            VERBOSE("%sListOf%s has %d item(s):", prefix.c_str(), name.c_str(),
+            VERBOSE("%sListOf%s has %d item(s):\n", prefix.c_str(), name.c_str(),
                     static_cast<int>(mList.size()));
             for (auto &it : mList)
             {
@@ -238,122 +265,88 @@ class Version
 {
   public:
     Version(uint32_t major, uint32_t minor, uint32_t subminor, uint32_t patch)
-        : mMajor(major), mMinor(minor), mSubminor(subminor), mPatch(patch), mWildcard(false)
-    {}
-    Version(uint32_t major, uint32_t minor, uint32_t subminor)
-        : mMajor(major), mMinor(minor), mSubminor(subminor), mWildcard(false)
-    {}
-    Version(uint32_t major, uint32_t minor) : mMajor(major), mMinor(minor), mWildcard(false) {}
-    Version(uint32_t major) : mMajor(major), mWildcard(false) {}
-    Version() : mWildcard(true) {}
-    Version(const Version &toCopy)
-        : mMajor(toCopy.mMajor),
-          mMinor(toCopy.mMinor),
-          mSubminor(toCopy.mSubminor),
-          mPatch(toCopy.mPatch),
-          mWildcard(toCopy.mWildcard)
+        : mMajor(major), mMinor(minor), mSubminor(subminor), mPatch(patch)
     {}
-    ~Version() {}
 
-    static Version *CreateVersionFromJson(const Json::Value &jObject)
+    Version()                = default;
+    Version(const Version &) = default;
+    Version(Version &&)      = default;
+    Version &operator=(const Version &) = default;
+    Version &operator=(Version &&) = default;
+    ~Version()                     = default;
+
+    static Version FromJson(const Json::Value &jObject)
     {
-        Version *version = nullptr;
-        // A major version must be provided before a minor, and so on:
-        if (jObject.isMember(kJsonVerMajor) && jObject[kJsonVerMajor].isInt())
+        Version version;
+        version.mMajor = IntegerPart::FromJson(jObject, kJsonVerMajor);
+        if (version.mMajor.mWildcard)
         {
-            int major = jObject[kJsonVerMajor].asInt();
-            if (jObject.isMember(kJsonVerMinor) && jObject[kJsonVerMinor].isInt())
-            {
-                int minor = jObject[kJsonVerMinor].asInt();
-                if (jObject.isMember(kJsonVerSubMinor) && jObject[kJsonVerSubMinor].isInt())
-                {
-                    int subMinor = jObject[kJsonVerSubMinor].asInt();
-                    if (jObject.isMember(kJsonVerPatch) && jObject[kJsonVerPatch].isInt())
-                    {
-                        int patch = jObject[kJsonVerPatch].asInt();
-                        version   = new Version(major, minor, subMinor, patch);
-                    }
-                    else
-                    {
-                        version = new Version(major, minor, subMinor);
-                    }
-                }
-                else
-                {
-                    version = new Version(major, minor);
-                }
-            }
-            else
-            {
-                version = new Version(major);
-            }
+            return version;
+        }
+        // Revision fields are only checked if their parent version field
+        // is set.
+        version.mMinor = IntegerPart::FromJson(jObject, kJsonVerMinor);
+        if (version.mMinor.mWildcard)
+        {
+            return version;
+        }
+
+        version.mSubminor = IntegerPart::FromJson(jObject, kJsonVerSubMinor);
+        if (version.mSubminor.mWildcard)
+        {
+            return version;
         }
+
+        version.mPatch = IntegerPart::FromJson(jObject, kJsonVerPatch);
         return version;
     }
 
     bool match(const Version &toCheck) const
     {
-        VERBOSE("\t\t\t Within Version %d,%d,%d,%d match(%d,%d,%d,%d): wildcards are %s and %s,\n",
-                mMajor.mPart, mMinor.mPart, mSubminor.mPart, mPatch.mPart, toCheck.mMajor.mPart,
-                toCheck.mMinor.mPart, toCheck.mSubminor.mPart, toCheck.mPatch.mPart,
-                mWildcard ? "true" : "false", toCheck.mWildcard ? "true" : "false");
-        if (!(mWildcard || toCheck.mWildcard))
-        {
-            VERBOSE("\t\t\t   mMajor match is %s, mMinor is %s, mSubminor is %s, mPatch is %s\n",
-                    mMajor.match(toCheck.mMajor) ? "true" : "false",
-                    mMinor.match(toCheck.mMinor) ? "true" : "false",
-                    mSubminor.match(toCheck.mSubminor) ? "true" : "false",
-                    mPatch.match(toCheck.mPatch) ? "true" : "false");
-        }
-        return (mWildcard || toCheck.mWildcard ||
+        VERBOSE("\t\t\t Matching Version %s against %s\n", getString().c_str(),
+                toCheck.getString().c_str());
+        return (isWildcard() || toCheck.isWildcard() ||
                 (mMajor.match(toCheck.mMajor) && mMinor.match(toCheck.mMinor) &&
                  mSubminor.match(toCheck.mSubminor) && mPatch.match(toCheck.mPatch)));
     }
+
     std::string getString() const
     {
-        if (mWildcard)
+        if (mMajor.mWildcard)
         {
             return "*";
         }
-        else
+
+        std::ostringstream ss;
+        ss << mMajor.mPart;
+        // Must at least have a major version:
+        if (!mMinor.mWildcard)
         {
-            char ret[100];
-            // Must at least have a major version:
-            if (!mMinor.mWildcard)
+            ss << "." << mMinor.mPart;
+            if (!mSubminor.mWildcard)
             {
-                if (!mSubminor.mWildcard)
-                {
-                    if (!mPatch.mWildcard)
-                    {
-                        snprintf(ret, 100, "%d.%d.%d.%d", mMajor.mPart, mMinor.mPart,
-                                 mSubminor.mPart, mPatch.mPart);
-                    }
-                    else
-                    {
-                        snprintf(ret, 100, "%d.%d.%d.*", mMajor.mPart, mMinor.mPart,
-                                 mSubminor.mPart);
-                    }
-                }
-                else
+                ss << "." << mSubminor.mPart;
+
+                if (!mPatch.mWildcard)
                 {
-                    snprintf(ret, 100, "%d.%d.*", mMajor.mPart, mMinor.mPart);
+                    ss << "." << mPatch.mPart;
                 }
             }
-            else
-            {
-                snprintf(ret, 100, "%d.*", mMajor.mPart);
-            }
-            std::string retString = ret;
-            return retString;
         }
+        if (mPatch.mWildcard)
+        {
+            ss << ".*";
+        }
+        return ss.str();
     }
 
+    bool isWildcard() const { return mMajor.mWildcard; }
+
   public:
     IntegerPart mMajor;
     IntegerPart mMinor;
     IntegerPart mSubminor;
     IntegerPart mPatch;
-    bool mWildcard;
 };
 
 // This encapsulates an application, and potentially the application's Version.  The default
@@ -363,32 +356,23 @@ class Version
 class Application
 {
   public:
-    Application(const std::string name, const Version &version)
+    Application(StringPart name, Version version = {})
         : mName(name), mVersion(version), mWildcard(false)
     {}
-    Application(const std::string name) : mName(name), mVersion(), mWildcard(false) {}
-    Application() : mName(), mVersion(), mWildcard(true) {}
-    ~Application() {}
+    Application()  = default;
+    ~Application() = default;
 
-    static Application *CreateApplicationFromJson(const Json::Value &jObject)
+    static bool FromJson(const Json::Value &jObject, Application *out)
     {
-        Application *application = nullptr;
-
         // If an application is listed, the application's name is required:
-        std::string appName = jObject[kJsonAppName].asString();
-
-        // The application's version is optional:
-        Version *version = Version::CreateVersionFromJson(jObject);
-        if (version)
-        {
-            application = new Application(appName, *version);
-            delete version;
-        }
-        else
+        auto name = StringPart::FromJson(jObject, kJsonAppName);
+        if (name.mWildcard)
         {
-            application = new Application(appName);
+            return false;
         }
-        return application;
+        auto version = Version::FromJson(jObject);
+        *out         = Application{std::move(name), std::move(version)};
+        return true;
     }
 
     bool match(const Application &toCheck) const
@@ -400,23 +384,23 @@ class Application
     {
         if (mWildcard)
         {
-            VERBOSE("      Wildcard (i.e. will match all applications)");
+            VERBOSE("      Wildcard (i.e. will match all applications)\n");
         }
-        else if (!mVersion.mWildcard)
+        else if (!mVersion.isWildcard())
         {
-            VERBOSE("      Application \"%s\" (version: %s)", mName.mPart.c_str(),
+            VERBOSE("      Application \"%s\" (version: %s)\n", mName.mPart.c_str(),
                     mVersion.getString().c_str());
         }
         else
         {
-            VERBOSE("      Application \"%s\"", mName.mPart.c_str());
+            VERBOSE("      Application \"%s\"\n", mName.mPart.c_str());
         }
     }
 
   public:
     StringPart mName;
     Version mVersion;
-    bool mWildcard;
+    bool mWildcard = true;
 };
 
 // This encapsulates a GPU and its driver.  The default constructor (not given any values) assumes
@@ -425,101 +409,67 @@ class Application
 class GPU
 {
   public:
-    GPU(const std::string vendor, uint32_t deviceId, const Version &version)
-        : mVendor(vendor), mDeviceId(IntegerPart(deviceId)), mVersion(version), mWildcard(false)
+    GPU(StringPart vendor, IntegerPart deviceId, Version version)
+        : mVendor(std::move(vendor)),
+          mDeviceId(std::move(deviceId)),
+          mVersion(version),
+          mWildcard(false)
     {}
-    GPU(const std::string vendor, uint32_t deviceId)
-        : mVendor(vendor), mDeviceId(IntegerPart(deviceId)), mVersion(), mWildcard(false)
+    GPU(std::string vendor, uint32_t deviceId, Version version)
+        : GPU(StringPart(std::move(vendor)), IntegerPart(deviceId), std::move(version))
     {}
-    GPU(const std::string vendor) : mVendor(vendor), mDeviceId(), mVersion(), mWildcard(false) {}
-    GPU() : mVendor(), mDeviceId(), mVersion(), mWildcard(true) {}
+    GPU()  = default;
+    ~GPU() = default;
     bool match(const GPU &toCheck) const
     {
-        VERBOSE("\t\t Within GPU match: wildcards are %s and %s,\n", mWildcard ? "true" : "false",
-                toCheck.mWildcard ? "true" : "false");
-        VERBOSE("\t\t   mVendor = \"%s\" and toCheck.mVendor = \"%s\"\n", mVendor.mPart.c_str(),
-                toCheck.mVendor.mPart.c_str());
-        VERBOSE("\t\t   mDeviceId = %d and toCheck.mDeviceId = %d\n", mDeviceId.mPart,
-                toCheck.mDeviceId.mPart);
-        VERBOSE("\t\t   mVendor match is %s, mDeviceId is %s, mVersion is %s\n",
-                toCheck.mVendor.match(mVendor) ? "true" : "false",
-                toCheck.mDeviceId.match(mDeviceId) ? "true" : "false",
-                toCheck.mVersion.match(mVersion) ? "true" : "false");
+        VERBOSE("\t\t Matching %s \n\t\t  against %s\n", toString().c_str(),
+                toCheck.toString().c_str());
         return (mWildcard || toCheck.mWildcard ||
                 (toCheck.mVendor.match(mVendor) && toCheck.mDeviceId.match(mDeviceId) &&
                  toCheck.mVersion.match(mVersion)));
     }
-    ~GPU() {}
 
-    static GPU *CreateGpuFromJson(const Json::Value &jObject)
+    // Returns true if out is set to a valid GPU instance.
+    static bool CreateGpuFromJson(const Json::Value &jObject, GPU *out)
     {
-        GPU *gpu = nullptr;
-
         // If a GPU is listed, the vendor name is required:
-        if (jObject.isMember(kJsonVendor) && jObject[kJsonVendor].isString())
-        {
-            std::string vendor = jObject[kJsonVendor].asString();
-            // If a version is given, the deviceId is required:
-            if (jObject.isMember(kJsonDeviceId) && jObject[kJsonDeviceId].isUInt())
-            {
-                uint32_t deviceId = jObject[kJsonDeviceId].asUInt();
-                Version *version  = Version::CreateVersionFromJson(jObject);
-                if (version)
-                {
-                    gpu = new GPU(vendor, deviceId, *version);
-                    delete version;
-                }
-                else
-                {
-                    gpu = new GPU(vendor, deviceId);
-                }
-            }
-            else
-            {
-                gpu = new GPU(vendor);
-            }
-        }
-        else
+        auto vendor = StringPart::FromJson(jObject, kJsonVendor);
+        if (vendor.mWildcard)
         {
-            WARN("Asked to parse a GPU, but no vendor found");
+            WARN("Asked to parse a GPU, but no vendor found.\n");
+            return false;
         }
 
-        return gpu;
+        auto deviceId = IntegerPart::FromJson(jObject, kJsonDeviceId);
+        auto version  = Version::FromJson(jObject);
+        *out          = GPU{std::move(vendor), std::move(deviceId), std::move(version)};
+        return true;
     }
 
-    void logItem() const
+    std::string toString() const
     {
         if (mWildcard)
         {
-            VERBOSE("          Wildcard (i.e. will match all GPUs)");
+            return std::string("Wildcard (i.e. will match all GPUs)");
         }
-        else
+
+        std::ostringstream ss;
+        ss << "GPU vendor: " << mVendor.mPart;
+        if (!mDeviceId.mWildcard)
         {
-            if (!mDeviceId.mWildcard)
-            {
-                if (!mVersion.mWildcard)
-                {
-                    VERBOSE("\t     GPU vendor: %s, deviceId: 0x%x, version: %s",
-                            mVendor.mPart.c_str(), mDeviceId.mPart, mVersion.getString().c_str());
-                }
-                else
-                {
-                    VERBOSE("\t     GPU vendor: %s, deviceId: 0x%x", mVendor.mPart.c_str(),
-                            mDeviceId.mPart);
-                }
-            }
-            else
-            {
-                VERBOSE("\t     GPU vendor: %s", mVendor.mPart.c_str());
-            }
+            ss << ", deviceId: " << std::hex << mDeviceId.mPart;
         }
+        ss << ", version: " << mVersion.getString();
+        return ss.str();
     }
 
+    void logItem() const { VERBOSE("\t     %s\n", toString().c_str()); }
+
   public:
     StringPart mVendor;
     IntegerPart mDeviceId;
     Version mVersion;
-    bool mWildcard;
+    bool mWildcard = true;
 };
 
 // This encapsulates a device, and potentially the device's model and/or a list of GPUs/drivers
@@ -529,58 +479,51 @@ class GPU
 class Device
 {
   public:
-    Device(const std::string manufacturer, const std::string model)
-        : mManufacturer(manufacturer), mModel(model), mGpuList("GPU"), mWildcard(false)
+    Device(StringPart manufacturer, StringPart model)
+        : mManufacturer(std::move(manufacturer)),
+          mModel(std::move(model)),
+          mGpuList("GPU"),
+          mWildcard(false)
     {}
-    Device(const std::string manufacturer)
-        : mManufacturer(manufacturer), mModel(), mGpuList("GPU"), mWildcard(false)
-    {}
-    Device() : mManufacturer(), mModel(), mGpuList("GPU"), mWildcard(true) {}
-    ~Device() {}
+    Device() : mGpuList("GPU") {}
+    ~Device() = default;
 
-    static Device *CreateDeviceFromJson(const Json::Value &jObject)
+    static Device FromJson(const Json::Value &jObject)
     {
-        Device *device = nullptr;
-        if (jObject.isMember(kJsonManufacturer) && jObject[kJsonManufacturer].isString())
-        {
-            std::string manufacturerName = jObject[kJsonManufacturer].asString();
-            // We don't let a model be specified without also specifying an Manufacturer:
-            if (jObject.isMember(kJsonModel) && jObject[kJsonModel].isString())
-            {
-                std::string model = jObject[kJsonModel].asString();
-                device            = new Device(manufacturerName, model);
-            }
-            else
-            {
-                device = new Device(manufacturerName);
-            }
-        }
-        else
+        auto manufacturer = StringPart::FromJson(jObject, kJsonManufacturer);
+        if (!manufacturer.mWildcard)
         {
-            // This case is not treated as an error because a rule may wish to only call out one or
-            // more GPUs, but not any specific Manufacturer (e.g. for any manufacturer's device
-            // that uses a GPU from Vendor-A, with DeviceID-Foo, and with driver version 1.2.3.4):
-            device = new Device();
+            // We don't let a model be specified without also specifying a manufacturer:
+            auto model = StringPart::FromJson(jObject, kJsonModel);
+            return Device(std::move(manufacturer), std::move(model));
         }
-        return device;
+        // This case is not treated as an error because a rule may wish to only call out one or
+        // more GPUs, but not any specific manufacturer (e.g. for any manufacturer's device
+        // that uses a GPU from Vendor-A, with DeviceID-Foo, and with driver version 1.2.3.4):
+        return Device();
     }
 
-    void addGPU(const GPU &gpu) { mGpuList.addItem(gpu); }
+    void addGPU(GPU &&gpu) { mGpuList.addItem(std::move(gpu)); }
     bool match(const Device &toCheck) const
     {
-        VERBOSE("\t Within Device match: wildcards are %s and %s,\n", mWildcard ? "true" : "false",
-                toCheck.mWildcard ? "true" : "false");
-        if (!(mWildcard || toCheck.mWildcard))
+        // GPU lists must always match, even when wildcards are used.
+        VERBOSE("\t   Checking ListOf<GPU>:\n");
+        if (!mGpuList.match(toCheck.mGpuList))
+        {
+            VERBOSE("\t Failed to match due to mismatched GPU list.\n");
+            return false;
+        }
+        if (mWildcard || toCheck.mWildcard)
+        {
+            VERBOSE("\t  Matching due to wildcard.\n");
+            return true;
+        }
+        if (toCheck.mManufacturer.match(mManufacturer) && toCheck.mModel.match(mModel))
         {
-            VERBOSE("\t   Manufacturer match is %s, model is %s\n",
-                    toCheck.mManufacturer.match(mManufacturer) ? "true" : "false",
-                    toCheck.mModel.match(mModel) ? "true" : "false");
+            VERBOSE("\t  Matching due to manufacturer and model match.\n");
+            return true;
         }
-        VERBOSE("\t   Need to check ListOf<GPU>\n");
-        return ((mWildcard || toCheck.mWildcard ||
-                 // The wildcards can override the Manufacturer/Model check, but not the GPU check
-                 (toCheck.mManufacturer.match(mManufacturer) && toCheck.mModel.match(mModel))) &&
-                mGpuList.match(toCheck.mGpuList));
+        return false;
     }
     void logItem() const
     {
@@ -588,14 +531,14 @@ class Device
         {
             if (mGpuList.mWildcard)
             {
-                VERBOSE("      Wildcard (i.e. will match all devices)");
+                VERBOSE("      Wildcard (i.e. will match all devices)\n");
                 return;
             }
             else
             {
                 VERBOSE(
                     "      Device with any manufacturer and model"
-                    ", and with the following GPUs:");
+                    ", and with the following GPUs:\n");
             }
         }
         else
@@ -604,14 +547,14 @@ class Device
             {
                 VERBOSE(
                     "      Device manufacturer: \"%s\" and model \"%s\""
-                    ", and with the following GPUs:",
+                    ", and with the following GPUs:\n",
                     mManufacturer.mPart.c_str(), mModel.mPart.c_str());
             }
             else
             {
                 VERBOSE(
                     "      Device manufacturer: \"%s\""
-                    ", and with the following GPUs:",
+                    ", and with the following GPUs:\n",
                     mManufacturer.mPart.c_str());
             }
         }
@@ -622,7 +565,7 @@ class Device
     StringPart mManufacturer;
     StringPart mModel;
     ListOf<GPU> mGpuList;
-    bool mWildcard;
+    bool mWildcard = true;
 };
 
 // This encapsulates a particular scenario to check against the rules.  A Scenario is similar to a
@@ -633,15 +576,16 @@ class Scenario
 {
   public:
     Scenario(const char *appName, const char *deviceMfr, const char *deviceModel)
-        : mApplication(Application(appName)), mDevice(Device(deviceMfr, deviceModel))
+        : mApplication(Application(StringPart(appName))),
+          mDevice(Device(StringPart(deviceMfr), StringPart(deviceModel)))
     {}
-    ~Scenario() {}
+    ~Scenario() = default;
     void logScenario()
     {
-        VERBOSE("  Scenario to compare against the rules");
-        VERBOSE("    Application:");
+        VERBOSE("  Scenario to compare against the rules:\n");
+        VERBOSE("    Application:\n");
         mApplication.logItem();
-        VERBOSE("    Device:");
+        VERBOSE("    Device:\n");
         mDevice.logItem();
     }
 
@@ -662,20 +606,29 @@ class Rule
           mDevList("Device"),
           mUseANGLE(useANGLE)
     {}
-    ~Rule() {}
-    void addApp(const Application &app) { mAppList.addItem(app); }
-    void addDevice(const Device &dev) { mDevList.addItem(dev); }
+    ~Rule() = default;
+    void addApp(Application &&app) { mAppList.addItem(std::move(app)); }
+    void addDevice(Device &&dev) { mDevList.addItem(std::move(dev)); }
     bool match(const Scenario &toCheck) const
     {
-        VERBOSE("    Within \"%s\" Rule: application match is %s and device match is %s\n",
-                mDescription.c_str(), mAppList.match(toCheck.mApplication) ? "true" : "false",
-                mDevList.match(toCheck.mDevice) ? "true" : "false");
-        return (mAppList.match(toCheck.mApplication) && mDevList.match(toCheck.mDevice));
+        VERBOSE("    Matching rule \"%s\" against scenario:\n", mDescription.c_str());
+        if (!mAppList.match(toCheck.mApplication))
+        {
+            VERBOSE("\tFailed to match rule due to mismatched application.\n");
+            return false;
+        }
+        if (!mDevList.match(toCheck.mDevice))
+        {
+            VERBOSE("\tFailed to match rule due to mismatched device.\n");
+            return false;
+        }
+        VERBOSE("\tSuccessfully matched rule.");
+        return true;
     }
     bool getUseANGLE() const { return mUseANGLE; }
     void logRule() const
     {
-        VERBOSE("  Rule: \"%s\" %s ANGLE", mDescription.c_str(),
+        VERBOSE("  Rule: \"%s\" %s ANGLE\n", mDescription.c_str(),
                 mUseANGLE ? "enables" : "disables");
         mAppList.logListOf("    ", "Applications");
         mDevList.logListOf("    ", "Devices");
@@ -701,90 +654,93 @@ class RuleList
         RuleList *rules = new RuleList;
 
         // Open the file and start parsing it:
-        Json::Reader jReader;
+        Json::CharReaderBuilder builder;
+        // Json::CharReaderBuilder::strictMode(&builder.settings_);
+        std::unique_ptr<Json::CharReader> reader(builder.newCharReader());
+
         Json::Value jTopLevelObject;
-        jReader.parse(jsonFileContents, jTopLevelObject);
-        Json::Value jRules = jTopLevelObject[kJsonRules];
-        for (unsigned int ruleIndex = 0; ruleIndex < jRules.size(); ruleIndex++)
+        std::string errorMessage;
+        const bool succeeded = reader->parse(&*jsonFileContents.begin(), &*jsonFileContents.end(),
+                                             &jTopLevelObject, &errorMessage);
+        if (!succeeded)
+        {
+            VERBOSE("Failed to parse rules from json file. Error: %s\n", errorMessage.c_str());
+            return nullptr;
+        }
+
+        for (const auto &jRule : jTopLevelObject[kJsonRules])
         {
-            Json::Value jRule           = jRules[ruleIndex];
             std::string ruleDescription = jRule[kJsonRule].asString();
             bool useANGLE               = jRule[kJsonUseANGLE].asBool();
-            Rule *newRule               = new Rule(ruleDescription, useANGLE);
+            Rule newRule(std::move(ruleDescription), useANGLE);
 
-            Json::Value jApps = jRule[kJsonApplications];
-            for (unsigned int appIndex = 0; appIndex < jApps.size(); appIndex++)
+            for (const auto &jApp : jRule[kJsonApplications])
             {
-                Json::Value jApp    = jApps[appIndex];
-                Application *newApp = Application::CreateApplicationFromJson(jApp);
-                newRule->addApp(*newApp);
-                delete newApp;
+                Application app;
+                if (Application::FromJson(jApp, &app))
+                {
+                    newRule.addApp(std::move(app));
+                }
             }
 
-            Json::Value jDevs = jRule[kJsonDevices];
-            for (unsigned int deviceIndex = 0; deviceIndex < jDevs.size(); deviceIndex++)
+            for (const auto &jDev : jRule[kJsonDevices])
             {
-                Json::Value jDev = jDevs[deviceIndex];
-                Device *newDev   = Device::CreateDeviceFromJson(jDev);
-
-                Json::Value jGPUs = jDev[kJsonGPUs];
-                for (unsigned int gpuIndex = 0; gpuIndex < jGPUs.size(); gpuIndex++)
+                Device newDev = Device::FromJson(jDev);
+                for (const auto &jGPU : jDev[kJsonGPUs])
                 {
-                    Json::Value jGPU = jGPUs[gpuIndex];
-                    GPU *newGPU      = GPU::CreateGpuFromJson(jGPU);
-                    if (newGPU)
+                    GPU newGPU;
+                    if (GPU::CreateGpuFromJson(jGPU, &newGPU))
                     {
-                        newDev->addGPU(*newGPU);
-                        delete newGPU;
+                        newDev.addGPU(std::move(newGPU));
                     }
                 }
-                newRule->addDevice(*newDev);
-                delete newDev;
+                newRule.addDevice(std::move(newDev));
             }
 
-            rules->addRule(*newRule);
-            delete newRule;
+            rules->addRule(std::move(newRule));
         }
 
         // Make sure there is at least one, default rule.  If not, add it here:
-        if (rules->mRuleList.size() == 0)
+        if (rules->mRuleList.empty())
         {
             Rule defaultRule("Default Rule", false);
-            rules->addRule(defaultRule);
+            rules->addRule(std::move(defaultRule));
         }
         return rules;
     }
 
-    void addRule(const Rule &rule) { mRuleList.push_back(rule); }
+    void addRule(Rule &&rule) { mRuleList.push_back(std::move(rule)); }
     bool getUseANGLE(const Scenario &toCheck)
     {
         // Initialize useANGLE to the system-wide default (that should be set in the default
         // rule, but just in case, set it here too):
         bool useANGLE = false;
-        VERBOSE("Checking scenario against %d ANGLE-for-Android rules:",
+        VERBOSE("Checking scenario against %d ANGLE-for-Android rules:\n",
                 static_cast<int>(mRuleList.size()));
 
         for (const Rule &rule : mRuleList)
         {
-            VERBOSE("  Checking Rule: \"%s\" (to see whether there's a match)",
+            VERBOSE("  Checking Rule: \"%s\" (to see whether there's a match)\n",
                     rule.mDescription.c_str());
             if (rule.match(toCheck))
             {
-                VERBOSE("  -> Rule matches.  Setting useANGLE to %s",
+                VERBOSE("  -> Rule matches.  Updating useANGLE to %s.\n",
                         rule.getUseANGLE() ? "true" : "false");
+                // The ANGLE rules are ordered from least to greatest specificity, meaning that
+                // the last rule with a match should dictate whether or not ANGLE should be
+                // recommended for use.
                 useANGLE = rule.getUseANGLE();
             }
             else
             {
-                VERBOSE("  -> Rule doesn't match.");
+                VERBOSE("  -> Rule doesn't match.\n");
             }
         }
-
         return useANGLE;
     }
     void logRules()
     {
-        VERBOSE("Showing %d ANGLE-for-Android rules:", static_cast<int>(mRuleList.size()));
+        VERBOSE("Showing %d ANGLE-for-Android rules:\n", static_cast<int>(mRuleList.size()));
         for (const Rule &rule : mRuleList)
         {
             rule.logRule();
@@ -831,6 +787,11 @@ ANGLE_EXPORT bool ANGLEAndroidParseRulesString(const char *rulesString,
 
     std::string rulesFileContents = rulesString;
     RuleList *rules               = RuleList::ReadRulesFromJsonString(rulesFileContents);
+    if (!rules)
+    {
+        return false;
+    }
+
     rules->logRules();
 
     *rulesHandle  = rules;
@@ -894,12 +855,13 @@ ANGLE_EXPORT bool ANGLEShouldBeUsedForApplication(const RulesHandle rulesHandle,
                              systemInfo->gpus[0].detailedDriverVersion.minor,
                              systemInfo->gpus[0].detailedDriverVersion.subMinor,
                              systemInfo->gpus[0].detailedDriverVersion.patch);
-    GPU gpuDriver(systemInfo->gpus[0].driverVendor, systemInfo->gpus[0].deviceId, gpuDriverVersion);
-    scenario.mDevice.addGPU(gpuDriver);
+    GPU gpuDriver(systemInfo->gpus[0].driverVendor, systemInfo->gpus[0].deviceId,
+                  std::move(gpuDriverVersion));
+    scenario.mDevice.addGPU(std::move(gpuDriver));
     scenario.logScenario();
 
-    bool rtn = rules->getUseANGLE(scenario);
-    VERBOSE("Application \"%s\" should %s ANGLE", appName, rtn ? "use" : "NOT use");
+    bool rtn = rules->getUseANGLE(std::move(scenario));
+    VERBOSE("Application \"%s\" should %s ANGLE.\n", appName, rtn ? "use" : "NOT use");
 
     return rtn;
 }

src/feature_support_util/feature_support_util_unittest.cpp

@@ -131,7 +131,7 @@ TEST_F(FeatureSupportUtilTest, TestRuleProcessing)
                         {
                             "Vendor" : "GPUVendorA",
                             "DeviceId" : 234,
-                            "VerMajor" : 1, "VerMinor" : 2, "VerSubMinor" : 3, "VerPatch" : 4}
+                            "VerMajor" : 1, "VerMinor" : 2, "VerSubMinor" : 3, "VerPatch" : 4
                         }
                     ]
                 }

src/tests/perf_tests/ANGLEPerfTest.cpp

@@ -128,20 +128,16 @@ void DumpTraceEventsToJSONFile(const std::vector<TraceEvent> &traceEvents,
     {
         Json::Value value(Json::objectValue);
 
-        std::stringstream phaseName;
-        phaseName << traceEvent.phase;
-
-        const auto microseconds =
-            static_cast<Json::LargestInt>(traceEvent.timestamp * 1000.0 * 1000.0);
+        const auto microseconds = static_cast<Json::LargestInt>(traceEvent.timestamp) * 1000 * 1000;
 
         value["name"] = traceEvent.name;
         value["cat"]  = traceEvent.categoryName;
-        value["ph"]   = phaseName.str();
+        value["ph"]   = std::string(1, traceEvent.phase);
         value["ts"]   = microseconds;
         value["pid"]  = strcmp(traceEvent.categoryName, "gpu.angle.gpu") == 0 ? "GPU" : "ANGLE";
         value["tid"]  = 1;
 
-        eventsValue.append(value);
+        eventsValue.append(std::move(value));
     }
 
     Json::Value root(Json::objectValue);
@@ -150,8 +146,10 @@ void DumpTraceEventsToJSONFile(const std::vector<TraceEvent> &traceEvents,
     std::ofstream outFile;
     outFile.open(outputFileName);
 
-    Json::StyledWriter styledWrite;
-    outFile << styledWrite.write(root);
+    Json::StreamWriterBuilder factory;
+    std::unique_ptr<Json::StreamWriter> const writer(factory.newStreamWriter());
+    std::ostringstream stream;
+    writer->write(root, &outFile);
 
     outFile.close();
 }