Clear plugin map in PluginFinder when reinitializing.

The call to clear() was accidentally removed in https://crrev.com/456456,
but the bug didn't surface until recently when the remote plugin list
was updated before the baked-in one.

Other cleanup:
* Remove unused FindPlugin() method
* Use built-in static instead of base::Singleton
* Use SequenceChecker instead of BrowserThread thread checking
* Use new base::Value APIs and moar std::unique_ptr

TBR: finnur@chromium.org
Bug: 778856
Change-Id: I050dd2292882ab047b54f63983cdfd2f7da3aded
Reviewed-on: https://chromium-review.googlesource.com/746841
Commit-Queue: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512853}

chrome/browser/extensions/api/content_settings/content_settings_api.cc

@@ -32,6 +32,7 @@
 #include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "content/public/browser/plugin_service.h"
+#include "content/public/common/webplugininfo.h"
 #include "extensions/browser/extension_prefs_scope.h"
 #include "extensions/common/error_utils.h"
 

chrome/browser/plugins/plugin_finder.cc

@@ -11,6 +11,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/strings/string16.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
@@ -22,13 +23,11 @@
 #include "chrome/grit/browser_resources.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
-#include "content/public/browser/browser_thread.h"
-#include "content/public/browser/plugin_service.h"
+#include "content/public/common/webplugininfo.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "url/gurl.h"
 
 using base::DictionaryValue;
-using content::PluginService;
 
 namespace {
 
@@ -154,17 +153,16 @@ void PluginFinder::RegisterPrefs(PrefRegistrySimple* registry) {
 
 // static
 PluginFinder* PluginFinder::GetInstance() {
-  // PluginFinder::GetInstance() is the only method that's allowed to call
-  // base::Singleton<PluginFinder>::get().
-  return base::Singleton<PluginFinder>::get();
+  static PluginFinder* instance = new PluginFinder();
+  return instance;
 }
 
 PluginFinder::PluginFinder() : version_(-1) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }
 
 void PluginFinder::Init() {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   // 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.
   std::unique_ptr<base::DictionaryValue> plugin_list(LoadBuiltInPluginList());
@@ -179,7 +177,7 @@ void PluginFinder::Init() {
 }
 
 // static
-base::DictionaryValue* PluginFinder::LoadBuiltInPluginList() {
+std::unique_ptr<base::DictionaryValue> PluginFinder::LoadBuiltInPluginList() {
   base::StringPiece json_resource(
       ui::ResourceBundle::GetSharedInstance().GetRawDataResource(
           IDR_PLUGIN_DB_JSON));
@@ -234,36 +232,12 @@ base::DictionaryValue* PluginFinder::LoadBuiltInPluginList() {
 
   DCHECK_EQ(base::JSONReader::JSON_NO_ERROR, error_code);
   RecordBuiltInPluginListError(PluginListError::PLUGIN_LIST_NO_ERROR);
-  return static_cast<base::DictionaryValue*>(value.release());
+  return base::DictionaryValue::From(std::move(value));
 }
 
 PluginFinder::~PluginFinder() {
 }
 
-bool PluginFinder::FindPlugin(
-    const std::string& mime_type,
-    const std::string& language,
-    PluginInstaller** installer,
-    std::unique_ptr<PluginMetadata>* plugin_metadata) {
-  if (g_browser_process->local_state()->GetBoolean(prefs::kDisablePluginFinder))
-    return false;
-
-  base::AutoLock lock(mutex_);
-  auto metadata_it = identifier_plugin_.begin();
-  for (; metadata_it != identifier_plugin_.end(); ++metadata_it) {
-    if (language == metadata_it->second->language() &&
-        metadata_it->second->HasMimeType(mime_type)) {
-      *plugin_metadata = metadata_it->second->Clone();
-
-      auto installer_it = installers_.find(metadata_it->second->identifier());
-      DCHECK(installer_it != installers_.end());
-      *installer = installer_it->second.get();
-      return true;
-    }
-  }
-  return false;
-}
-
 bool PluginFinder::FindPluginWithIdentifier(
     const std::string& identifier,
     PluginInstaller** installer,
@@ -293,10 +267,11 @@ void PluginFinder::ReinitializePlugins(
     return;
 
   version_ = version;
+  identifier_plugin_.clear();
 
   for (base::DictionaryValue::Iterator plugin_it(*plugin_list);
       !plugin_it.IsAtEnd(); plugin_it.Advance()) {
-    const base::DictionaryValue* plugin = NULL;
+    const base::DictionaryValue* plugin = nullptr;
     const std::string& identifier = plugin_it.key();
     if (plugin_list->GetDictionaryWithoutPathExpansion(identifier, &plugin)) {
       DCHECK(identifier_plugin_.find(identifier) == identifier_plugin_.end());

chrome/browser/plugins/plugin_finder.h

@@ -12,16 +12,17 @@
 #include "base/callback.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
-#include "base/memory/singleton.h"
-#include "base/strings/string16.h"
+#include "base/sequence_checker.h"
 #include "base/synchronization/lock.h"
-#include "chrome/common/features.h"
-#include "content/public/common/webplugininfo.h"
 
 namespace base {
 class DictionaryValue;
 }
 
+namespace content {
+struct WebPluginInfo;
+}
+
 class PluginInstaller;
 class PluginMetadata;
 class PrefRegistrySimple;
@@ -40,15 +41,6 @@ class PluginFinder {
 
   void ReinitializePlugins(const base::DictionaryValue* json_metadata);
 
-  // Finds a plugin for the given MIME type and language (specified as an IETF
-  // language tag, i.e. en-US). If found, sets |installer| to the
-  // corresponding PluginInstaller and |plugin_metadata| to a copy of the
-  // corresponding PluginMetadata.
-  bool FindPlugin(const std::string& mime_type,
-                  const std::string& language,
-                  PluginInstaller** installer,
-                  std::unique_ptr<PluginMetadata>* plugin_metadata);
-
   // Finds the plugin with the given identifier. If found, sets |installer|
   // to the corresponding PluginInstaller and |plugin_metadata| to a copy
   // of the corresponding PluginMetadata. |installer| may be null.
@@ -62,16 +54,17 @@ class PluginFinder {
       const content::WebPluginInfo& plugin);
 
  private:
-  friend struct base::DefaultSingletonTraits<PluginFinder>;
   FRIEND_TEST_ALL_PREFIXES(PluginFinderTest, JsonSyntax);
-  FRIEND_TEST_ALL_PREFIXES(PluginFinderTest, PluginGroups);
+  FRIEND_TEST_ALL_PREFIXES(PluginFinderTest, ReinitializePlugins);
 
   PluginFinder();
   ~PluginFinder();
 
   // Loads the plugin information from the browser resources and parses it.
   // Returns null if the plugin list couldn't be parsed.
-  static base::DictionaryValue* LoadBuiltInPluginList();
+  static std::unique_ptr<base::DictionaryValue> LoadBuiltInPluginList();
+
+  SEQUENCE_CHECKER(sequence_checker_);
 
   std::map<std::string, std::unique_ptr<PluginInstaller>> installers_;
 

chrome/browser/plugins/plugin_finder_unittest.cc

@@ -12,9 +12,9 @@ using base::DictionaryValue;
 using base::ListValue;
 
 TEST(PluginFinderTest, JsonSyntax) {
-  std::unique_ptr<base::DictionaryValue> plugin_list(
-      PluginFinder::LoadBuiltInPluginList());
-  ASSERT_TRUE(plugin_list.get());
+  std::unique_ptr<base::DictionaryValue> plugin_list =
+      PluginFinder::LoadBuiltInPluginList();
+  ASSERT_TRUE(plugin_list);
   std::unique_ptr<base::Value> version;
   ASSERT_TRUE(plugin_list->Remove("x-version", &version));
   EXPECT_EQ(base::Value::Type::INTEGER, version->type());
@@ -73,3 +73,21 @@ TEST(PluginFinderTest, JsonSyntax) {
     }
   }
 }
+
+TEST(PluginFinderTest, ReinitializePlugins) {
+  PluginFinder* plugin_finder = PluginFinder::GetInstance();
+
+  plugin_finder->Init();
+
+  std::unique_ptr<base::DictionaryValue> plugin_list =
+      PluginFinder::LoadBuiltInPluginList();
+
+  // Increment the version number by one.
+  const base::Value* version_value = plugin_list->FindKey("x-version");
+  ASSERT_TRUE(version_value);
+  int version = 0;
+  ASSERT_TRUE(version_value->GetAsInteger(&version));
+  plugin_list->SetKey("x-version", base::Value(version + 1));
+
+  plugin_finder->ReinitializePlugins(plugin_list.get());
+}