Relax software rendering list parsing.

content/browser/gpu/gpu_blacklist.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/gpu/gpu_blacklist.h"
 
 #include "base/json/json_reader.h"
 #include "base/logging.h"
 #include "base/string_number_conversions.h"
 #include "base/string_piece.h"
@@ skipping 188 matching lines @@
     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;
 }
 
+// static
 GpuBlacklist::GpuBlacklistEntry*
 GpuBlacklist::GpuBlacklistEntry::GetGpuBlacklistEntryFromValue(
     DictionaryValue* value, bool top_level) {
   DCHECK(value);
-  scoped_ptr<GpuBlacklistEntry> entry(new GpuBlacklistEntry());
+  ScopedGpuBlacklistEntry entry(new GpuBlacklistEntry());
 
   size_t dictionary_entry_count = 0;
 
   if (top_level) {
     uint32 id;
     if (!value->GetInteger("id", reinterpret_cast<int*>(&id)) ||
         !entry->SetId(id)) {
       LOG(WARNING) << "Malformed id entry " << entry->id();
       return NULL;
     }
@@ skipping 162 matching lines @@
 
   if (top_level) {
     ListValue* exception_list_value = NULL;
     if (value->GetList("exceptions", &exception_list_value)) {
       for (size_t i = 0; i < exception_list_value->GetSize(); ++i) {
         DictionaryValue* exception_value = NULL;
         if (!exception_list_value->GetDictionary(i, &exception_value)) {
           LOG(WARNING) << "Malformed exceptions entry " << entry->id();
           return NULL;
         }
-        GpuBlacklistEntry* exception = GetGpuBlacklistEntryFromValue(
-            exception_value, false);
+        ScopedGpuBlacklistEntry exception(
+            GetGpuBlacklistEntryFromValue(exception_value, false));
         if (exception == NULL) {
           LOG(WARNING) << "Malformed exceptions entry " << entry->id();
           return NULL;
         }
-        entry->AddException(exception);
+        if (exception->contains_unknown_fields_) {
+          LOG(WARNING) << "Exception with unknown fields " << entry->id();
+          entry->contains_unknown_fields_ = true;
+        } else {
+          entry->AddException(exception);
+        }
       }
       dictionary_entry_count++;
     }
 
     DictionaryValue* browser_version_value = NULL;
     // browser_version is processed in LoadGpuBlacklist().
     if (value->GetDictionary("browser_version", &browser_version_value))
       dictionary_entry_count++;
   }
 
   if (value->size() != dictionary_entry_count) {
-    LOG(WARNING) << "Malformed entry " << entry->id();
-    return NULL;
+    LOG(WARNING) << "Entry with unknown fields " << entry->id();
+    entry->contains_unknown_fields_ = true;
   }
   return entry.release();

[Ken Russell (switch to Gerrit), 2011/08/25 18:03:31]

This is the leak. The ScopedGpuBlacklistEntry constructor above gives the
GpuBlacklistEntry a ref count of 1. This release call does *not* adjust the
reference count, just sets the referent to NULL, so the returned pointer still
has a reference count of 1. The caller wraps the return value in a
ScopedGpuBlacklistEntry, incrementing its reference count again (to 2) and when
that wrapper is later deleted the refcount only goes down to 1, never 0, so all
of the blacklist entries are being leaked. The fix is to change the return value
of this function to a ScopedGpuBlacklistEntry.

 }
 
-GpuBlacklist::GpuBlacklistEntry::~GpuBlacklistEntry() {
-  for (size_t i = 0; i < exceptions_.size(); ++i)
-    delete exceptions_[i];
-}
-
 GpuBlacklist::GpuBlacklistEntry::GpuBlacklistEntry()
     : id_(0),
-      vendor_id_(0) {
+      vendor_id_(0),
+      contains_unknown_fields_(false),
+      contains_unknown_features_(false) {
 }
 
 bool GpuBlacklist::GpuBlacklistEntry::SetId(uint32 id) {
   if (id != 0) {
     id_ = id;
     return true;
   }
   return false;
 }
 
@@ skipping 69 matching lines @@
         GpuFeatureFlags::StringToGpuFeatureType(blacklisted_features[i]);
     switch (type) {
       case GpuFeatureFlags::kGpuFeatureAccelerated2dCanvas:
       case GpuFeatureFlags::kGpuFeatureAcceleratedCompositing:
       case GpuFeatureFlags::kGpuFeatureWebgl:
       case GpuFeatureFlags::kGpuFeatureMultisampling:
       case GpuFeatureFlags::kGpuFeatureAll:
         flags |= type;
         break;
       case GpuFeatureFlags::kGpuFeatureUnknown:
-        return false;
+        contains_unknown_features_ = true;
+        break;
     }
   }
   feature_flags_.reset(new GpuFeatureFlags());
   feature_flags_->set_flags(flags);
   return true;
 }
 
 void GpuBlacklist::GpuBlacklistEntry::AddException(
     GpuBlacklistEntry* exception) {
   exceptions_.push_back(exception);
@@ skipping 51 matching lines @@
 
 uint32 GpuBlacklist::GpuBlacklistEntry::id() const {
   return id_;
 }
 
 GpuFeatureFlags GpuBlacklist::GpuBlacklistEntry::GetGpuFeatureFlags() const {
   return *feature_flags_;
 }
 
 GpuBlacklist::GpuBlacklist(const std::string& browser_version_string)
-    : max_entry_id_(0) {
+    : max_entry_id_(0),
+      contains_unknown_fields_(false) {
   browser_version_.reset(Version::GetVersionFromString(browser_version_string));
   DCHECK(browser_version_.get() != NULL);
 }
 
 GpuBlacklist::~GpuBlacklist() {
   Clear();
 }
 
-bool GpuBlacklist::LoadGpuBlacklist(const std::string& json_context,
-                                    bool current_os_only) {
+bool GpuBlacklist::LoadGpuBlacklist(
+    const std::string& json_context, GpuBlacklist::OsFilter os_filter) {
   scoped_ptr<Value> root;
   root.reset(base::JSONReader::Read(json_context, false));
   if (root.get() == NULL || !root->IsType(Value::TYPE_DICTIONARY))
     return false;
 
   DictionaryValue* root_dictionary = static_cast<DictionaryValue*>(root.get());
   DCHECK(root_dictionary);
-  return LoadGpuBlacklist(*root_dictionary, current_os_only);
+  return LoadGpuBlacklist(*root_dictionary, os_filter);
 }
 
-bool GpuBlacklist::LoadGpuBlacklist(const DictionaryValue& parsed_json,
-                                    bool current_os_only) {
-  std::vector<GpuBlacklistEntry*> entries;
+bool GpuBlacklist::LoadGpuBlacklist(
+    const DictionaryValue& parsed_json, GpuBlacklist::OsFilter os_filter) {
+  std::vector<ScopedGpuBlacklistEntry> entries;
 
   std::string version_string;
   parsed_json.GetString("version", &version_string);
   version_.reset(Version::GetVersionFromString(version_string));
   if (version_.get() == NULL)
     return false;
 
   ListValue* list = NULL;
   if (!parsed_json.GetList("entries", &list))
     return false;
 
   uint32 max_entry_id = 0;
-  size_t entry_count_expectation = list->GetSize();
+  bool contains_unknown_fields = false;
   for (size_t i = 0; i < list->GetSize(); ++i) {
     DictionaryValue* list_item = NULL;
     bool valid = list->GetDictionary(i, &list_item);
-    if (!valid)
-      break;
-    if (list_item == NULL)
-      break;
+    if (!valid || list_item == NULL)
+      return false;
     // Check browser version compatibility: if the entry is not for the
     // current browser version, don't process it.
     BrowserVersionSupport browser_version_support =
         IsEntrySupportedByCurrentBrowserVersion(list_item);
     if (browser_version_support == kMalformed)
-      break;
-    if (browser_version_support == kUnsupported) {
-      entry_count_expectation--;
+      return false;
+    if (browser_version_support == kUnsupported)
+      continue;
+    DCHECK(browser_version_support == kSupported);
+    ScopedGpuBlacklistEntry entry(
+        GpuBlacklistEntry::GetGpuBlacklistEntryFromValue(list_item, true));
+    if (entry == NULL)
+      return false;
+    if (entry->id() > max_entry_id)
+      max_entry_id = entry->id();
+    // If an unknown field is encountered, skip the entry; if an unknown
+    // feature is encountered, ignore the feature, but keep the entry.
+    if (entry->contains_unknown_fields()) {
+      contains_unknown_fields = true;
       continue;
     }
-    DCHECK(browser_version_support == kSupported);
-    GpuBlacklistEntry* entry =
-        GpuBlacklistEntry::GetGpuBlacklistEntryFromValue(list_item, true);
-    if (entry == NULL)
-      break;
-    if (entry->id() > max_entry_id)
-      max_entry_id = entry->id();
+    if (entry->contains_unknown_features())
+      contains_unknown_fields = true;
     entries.push_back(entry);
   }
 
-  if (entries.size() != entry_count_expectation) {
-    for (size_t i = 0; i < entries.size(); ++i)
-      delete entries[i];
-    return false;
-  }
-
   Clear();
-  // Don't apply GPU blacklist for a non-registered OS.
-  OsType os_filter = GetOsType();
-  if (os_filter != kOsUnknown) {
-    for (size_t i = 0; i < entries.size(); ++i) {
-      OsType entry_os = entries[i]->GetOsType();
-      if (!current_os_only ||
-          entry_os == kOsAny || entry_os == os_filter)
-        blacklist_.push_back(entries[i]);
-      else
-        delete entries[i];
-    }
+  OsType my_os = GetOsType();
+  for (size_t i = 0; i < entries.size(); ++i) {
+    OsType entry_os = entries[i]->GetOsType();
+    if (os_filter == GpuBlacklist::kAllOs ||
+        entry_os == kOsAny || entry_os == my_os)
+      blacklist_.push_back(entries[i]);
   }
   max_entry_id_ = max_entry_id;
+  contains_unknown_fields_ = contains_unknown_fields;
   return true;
 }
 
 GpuFeatureFlags GpuBlacklist::DetermineGpuFeatureFlags(
     GpuBlacklist::OsType os,
     Version* os_version,
     const GPUInfo& gpu_info) {
   active_entries_.clear();
   GpuFeatureFlags flags;
 
@@ skipping 175 matching lines @@
     if (disable_multisampling) {
       DictionaryValue* problem = new DictionaryValue();
       problem->SetString("description",
           "Multisampling has been disabled, either via about:flags "
           "or command line");
       problem->Set("crBugs", new ListValue());
       problem->Set("webkitBugs", new ListValue());
       problem_list->Append(problem);
     }
     for (size_t i = 0; i < active_entries_.size(); ++i) {
-      GpuBlacklistEntry* entry = active_entries_[i];
+      ScopedGpuBlacklistEntry entry = active_entries_[i];
       DictionaryValue* problem = new DictionaryValue();
 
       problem->SetString("description", entry->description());
 
       ListValue* cr_bugs = new ListValue();
       for (size_t j = 0; j < entry->cr_bugs().size(); ++j)
         cr_bugs->Append(Value::CreateIntegerValue(
             entry->cr_bugs()[j]));
       problem->Set("crBugs", cr_bugs);
 
       ListValue* webkit_bugs = new ListValue();
       for (size_t j = 0; j < entry->webkit_bugs().size(); ++j)
         webkit_bugs->Append(Value::CreateIntegerValue(
             entry->webkit_bugs()[j]));
       problem->Set("webkitBugs", webkit_bugs);
 
       problem_list->Append(problem);
     }
     status->Set("problems", problem_list);
   }
   return status;
 }
 
+size_t GpuBlacklist::num_entries() const {
+  return blacklist_.size();
+}
+
 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;
@@ skipping 32 matching lines @@
 #elif defined(OS_LINUX)
   return kOsLinux;
 #elif defined(OS_MACOSX)
   return kOsMacosx;
 #else
   return kOsUnknown;
 #endif
 }
 
 void GpuBlacklist::Clear() {
-  for (size_t i = 0; i < blacklist_.size(); ++i)
-    delete blacklist_[i];
   blacklist_.clear();
   active_entries_.clear();
+  max_entry_id_ = 0;
+  contains_unknown_fields_ = false;
 }
 
 GpuBlacklist::BrowserVersionSupport
 GpuBlacklist::IsEntrySupportedByCurrentBrowserVersion(
     DictionaryValue* value) {
   DCHECK(value);
   DictionaryValue* browser_version_value = NULL;
   if (value->GetDictionary("browser_version", &browser_version_value)) {
     std::string version_op = "any";
     std::string version_string;
     std::string version_string2;
     browser_version_value->GetString("op", &version_op);
     browser_version_value->GetString("number", &version_string);
     browser_version_value->GetString("number2", &version_string2);
     scoped_ptr<VersionInfo> browser_version_info;
     browser_version_info.reset(
       new VersionInfo(version_op, version_string, version_string2));
     if (!browser_version_info->IsValid())
       return kMalformed;
     if (browser_version_info->Contains(*browser_version_))
       return kSupported;
     return kUnsupported;
   }
   return kSupported;
 }