Handle malformed built-in PluginFinder data.

chrome/browser/plugins/plugin_finder.cc

Index: chrome/browser/plugins/plugin_finder.cc
diff --git a/chrome/browser/plugins/plugin_finder.cc b/chrome/browser/plugins/plugin_finder.cc
index 626b0a3980ed24ca0a873ec4874a70598d53fbda..6827689692b0ba3b505d235d9ac941ada4a6dec3 100644
--- a/chrome/browser/plugins/plugin_finder.cc
+++ b/chrome/browser/plugins/plugin_finder.cc
@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/json/json_reader.h"
 #include "base/message_loop/message_loop.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/stl_util.h"
@@ -128,6 +129,14 @@ PluginMetadata* CreatePluginMetadata(
   return plugin;
 }
 
+void RecordBuiltInPluginListError(int error_code) {
+  // JSONReader::JSON_PARSE_ERROR_COUNT is used for the case where the JSON
+  // value has the wrong type.
+  UMA_HISTOGRAM_ENUMERATION("PluginFinder.BuiltInPluginList.ErrorCode",
+                            error_code,
+                            base::JSONReader::JSON_PARSE_ERROR_COUNT + 1);

[Alexei Svitkine (slow), 2015/04/20 17:15:10]

You don't need +1 here.

Also, please update that enum definition with a comment mentioning it's used in
UMA and entries shouldn't be deleted or renumbered.

[Bernhard Bauer, 2015/04/21 16:34:11]

I'm (ab)using JSONReader::JSON_PARSE_ERROR_COUNT as a value (see comment above),
so the boundary value is the next one.
Done.

[Alexei Svitkine (slow), 2015/04/21 16:50:36]

I see. I don't think we should do this. Otherwise, if someone ever adds a new
entry to the enum, it will renumber things. Why not just add it to the enum?

+}
+
 }  // namespace
 
 // static
@@ -151,7 +160,13 @@ void PluginFinder::Init() {
   // Load the built-in plugin list first. If we have a newer version stored
   // locally or download one, we will replace this one with it.
   scoped_ptr<base::DictionaryValue> plugin_list(LoadBuiltInPluginList());
-  DCHECK(plugin_list);
+
+  // Gracefully handle the case where we couldn't parse the built-in plugin list
+  // for some reason (https://crbug.com/388560). TODO(bauerb): Change back to a
+  // DCHECK once we have gathered more data about the underlying problem.
+  if (!plugin_list)
+    return;
+
   ReinitializePlugins(plugin_list.get());
 }
 
@@ -161,17 +176,24 @@ base::DictionaryValue* PluginFinder::LoadBuiltInPluginList() {
       ResourceBundle::GetSharedInstance().GetRawDataResource(
           IDR_PLUGIN_DB_JSON));
   std::string error_str;
+  int error_code = base::JSONReader::JSON_NO_ERROR;
   scoped_ptr<base::Value> value(base::JSONReader::ReadAndReturnError(
-      json_resource,
-      base::JSON_PARSE_RFC,
-      NULL,
-      &error_str));
-  if (!value.get()) {
+      json_resource, base::JSON_PARSE_RFC, &error_code, &error_str));
+  if (!value) {
     DLOG(ERROR) << error_str;
-    return NULL;
+    RecordBuiltInPluginListError(error_code);
+    return nullptr;
+  }
+
+  if (value->GetType() != base::Value::TYPE_DICTIONARY) {
+    // JSONReader::JSON_PARSE_ERROR_COUNT is used for the case where the JSON
+    // value has the wrong type.
+    RecordBuiltInPluginListError(base::JSONReader::JSON_PARSE_ERROR_COUNT);
+    return nullptr;
   }
-  if (value->GetType() != base::Value::TYPE_DICTIONARY)
-    return NULL;
+
+  DCHECK_EQ(base::JSONReader::JSON_NO_ERROR, error_code);
+  RecordBuiltInPluginListError(error_code);
   return static_cast<base::DictionaryValue*>(value.release());
 }