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 578 matching lines @@
     : max_entry_id_(0) {
   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 current_os_only,

[vangelis, 2011/08/18 21:15:11]

One more thing. Having two bool's next to each other in the function signature
is confusing for the caller.  It will be cleaner if they are converted to enums
so the caller can do:

LoadGpuBlacklist(context, kCurrentOSOnly, kIgnoreUnknownFields)

or

LoadGpuBlacklist(context, kAllOS, kFailOnUknownFields)

or similar.

And another nit: tolerate_errors is somewhat misleading.  Maybe something like
ignore_unknown_fields or similar? 

+                                    bool tolerate_errors) {
   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, current_os_only, tolerate_errors);
 }
 
 bool GpuBlacklist::LoadGpuBlacklist(const DictionaryValue& parsed_json,
-                                    bool current_os_only) {
+                                    bool current_os_only,
+                                    bool tolerate_errors) {
   std::vector<GpuBlacklistEntry*> 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))
@@ skipping 15 matching lines @@
     if (browser_version_support == kMalformed)
       break;
     if (browser_version_support == kUnsupported) {
       entry_count_expectation--;
       continue;
     }
     DCHECK(browser_version_support == kSupported);
     GpuBlacklistEntry* entry =
         GpuBlacklistEntry::GetGpuBlacklistEntryFromValue(list_item, true);
     if (entry == NULL)
-      break;
+      continue;

[vangelis, 2011/08/18 20:51:11]

It seems to me that entries with unknown fields will be completely skipped,
which is not what we need. We want them processed but their additional fields
ignored.  
I believe GetGpuBlacklistEntryFromValue() needs to be modified to account for
that and a test added.

     if (entry->id() > max_entry_id)
       max_entry_id = entry->id();
     entries.push_back(entry);
   }
 
-  if (entries.size() != entry_count_expectation) {
+  if (!tolerate_errors && 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) {
@@ skipping 218 matching lines @@
             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 61 matching lines @@
     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;
 }