Separate plugin_metadata from plugin_installer, thread-safe plugin_finder

chrome/browser/plugins/plugin_prefs.cc

 // Copyright (c) 2012 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 "chrome/browser/plugins/plugin_prefs.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/browser/plugins/plugin_installer.h"
+#include "chrome/browser/plugins/plugin_metadata.h"
 #include "chrome/browser/plugins/plugin_prefs_factory.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_keyed_service.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_content_client.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
@@ skipping 72 matching lines @@
       PluginPrefsFactory::GetInstance()->SetTestingFactoryAndUse(
           profile, &PluginPrefsFactory::CreateForTestingProfile).get());
 }
 
 void PluginPrefs::SetPluginListForTesting(
     webkit::npapi::PluginList* plugin_list) {
   plugin_list_ = plugin_list;
 }
 
 void PluginPrefs::EnablePluginGroup(bool enabled, const string16& group_name) {
-  PluginFinder::Get(
-      base::Bind(&PluginPrefs::GetPluginFinderForEnablePluginGroup,
-                 this, enabled, group_name));
-}
-
-void PluginPrefs::GetPluginFinderForEnablePluginGroup(
-    bool enabled,
-    const string16& group_name,
-    PluginFinder* finder) {
   PluginService::GetInstance()->GetPlugins(
       base::Bind(&PluginPrefs::EnablePluginGroupInternal,
-                 this, enabled, group_name, finder));
+                 this, enabled, group_name));
 }
 
 void PluginPrefs::EnablePluginGroupInternal(
     bool enabled,
     const string16& group_name,
-    PluginFinder* finder,
     const std::vector<webkit::WebPluginInfo>& plugins) {
   base::AutoLock auto_lock(lock_);
+  PluginFinder* finder = PluginFinder::GetInstance();
 
   // Set the desired state for the group.
   plugin_group_state_[group_name] = enabled;
 
   // Update the state for all plug-ins in the group.
   for (size_t i = 0; i < plugins.size(); ++i) {
-    PluginInstaller* installer = finder->GetPluginInstaller(plugins[i]);
-    if (group_name != installer->name())
+    PluginMetadata* plugin = finder->GetPluginMetadata(plugins[i]);
+    if (group_name != plugin->name())
       continue;
     plugin_state_.Set(plugins[i].path, enabled);
   }
 
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-      base::Bind(&PluginPrefs::OnUpdatePreferences, this, plugins, finder));
+      base::Bind(&PluginPrefs::OnUpdatePreferences, this, plugins));
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
       base::Bind(&PluginPrefs::NotifyPluginStatusChanged, this));
 }
 
-void PluginPrefs::EnablePluginIfPossibleCallback(
+void PluginPrefs::EnablePlugin(
     bool enabled, const FilePath& path,
-    const base::Callback<void(bool)>& callback,
-    PluginFinder* finder) {
+    const base::Callback<void(bool)>& callback) {
+  PluginFinder* finder = PluginFinder::GetInstance();
   webkit::WebPluginInfo plugin;
   bool can_enable = true;
   if (PluginService::GetInstance()->GetPluginInfoByPath(path, &plugin)) {
-    PluginInstaller* installer = finder->GetPluginInstaller(plugin);
+    PluginMetadata* plugin_metadata = finder->GetPluginMetadata(plugin);
     PolicyStatus plugin_status = PolicyStatusForPlugin(plugin.name);
-    PolicyStatus group_status = PolicyStatusForPlugin(installer->name());
+    PolicyStatus group_status = PolicyStatusForPlugin(plugin_metadata->name());
     if (enabled) {
       if (plugin_status == POLICY_DISABLED || group_status == POLICY_DISABLED)
         can_enable = false;
     } else {
       if (plugin_status == POLICY_ENABLED || group_status == POLICY_ENABLED)
         can_enable = false;
     }
   } else {
     NOTREACHED();
   }
 
   if (!can_enable) {
     callback.Run(false);

[ibraaaa, 2012/09/24 19:10:21]

Do you mean calling this callback by MessageLoop::Current()->PostTask instead of
calling it synchronously?

[Bernhard Bauer, 2012/09/24 19:34:28]

Yes, exactly. 

(Sorry, forgot to submit my comments) The advantage is that this way the
contract is simpler: We always return asynchronously, regardless of success or
failure (which a non-unit-test caller would not even know in advance). 

[ibraaaa, 2012/09/24 20:07:13]

Done.

     return;
   }
 
   PluginService::GetInstance()->GetPlugins(
       base::Bind(&PluginPrefs::EnablePluginInternal, this,
                  enabled, path, finder, callback));
 }
 
-void PluginPrefs::EnablePlugin(
-    bool enabled, const FilePath& path,
-    const base::Callback<void(bool)>& callback) {
-  PluginFinder::Get(base::Bind(&PluginPrefs::EnablePluginIfPossibleCallback,
-                               this, enabled, path, callback));
-}
-
 void PluginPrefs::EnablePluginInternal(
     bool enabled,
     const FilePath& path,
     PluginFinder* plugin_finder,
     const base::Callback<void(bool)>& callback,
     const std::vector<webkit::WebPluginInfo>& plugins) {
   {
     // Set the desired state for the plug-in.
     base::AutoLock auto_lock(lock_);
     plugin_state_.Set(path, enabled);
   }
 
   string16 group_name;
   for (size_t i = 0; i < plugins.size(); ++i) {
     if (plugins[i].path == path) {
-      PluginInstaller* installer =
-          plugin_finder->GetPluginInstaller(plugins[i]);
+      PluginMetadata* plugin_metadata =
+          plugin_finder->GetPluginMetadata(plugins[i]);
       // set the group name for this plug-in.
-      group_name = installer->name();
+      group_name = plugin_metadata->name();
       DCHECK_EQ(enabled, IsPluginEnabled(plugins[i]));
       break;
     }
   }
 
   bool all_disabled = true;
   for (size_t i = 0; i < plugins.size(); ++i) {
-    PluginInstaller* installer = plugin_finder->GetPluginInstaller(plugins[i]);
-    DCHECK(!installer->name().empty());
-    if (group_name == installer->name()) {
+    PluginMetadata* plugin_metadata =
+        plugin_finder->GetPluginMetadata(plugins[i]);
+    DCHECK(!plugin_metadata->name().empty());
+    if (group_name == plugin_metadata->name()) {
       all_disabled = all_disabled && !IsPluginEnabled(plugins[i]);
     }
   }
 
   if (!group_name.empty()) {
     // Update the state for the corresponding plug-in group.
     base::AutoLock auto_lock(lock_);
     plugin_group_state_[group_name] = !all_disabled;
   }
 
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-      base::Bind(&PluginPrefs::OnUpdatePreferences, this,
-                 plugins, plugin_finder));
+      base::Bind(&PluginPrefs::OnUpdatePreferences, this, plugins));
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
       base::Bind(&PluginPrefs::NotifyPluginStatusChanged, this));
   callback.Run(true);
 }
 
 PluginPrefs::PolicyStatus PluginPrefs::PolicyStatusForPlugin(
     const string16& name) const {
   base::AutoLock auto_lock(lock_);
   if (IsStringMatchedInSet(name, policy_enabled_plugin_patterns_)) {
     return POLICY_ENABLED;
   } else if (IsStringMatchedInSet(name, policy_disabled_plugin_patterns_) &&
              !IsStringMatchedInSet(
                  name, policy_disabled_plugin_exception_patterns_)) {
     return POLICY_DISABLED;
   } else {
     return NO_POLICY;
   }
 }
 
 bool PluginPrefs::IsPluginEnabled(const webkit::WebPluginInfo& plugin) const {
-  scoped_ptr<webkit::npapi::PluginGroup> group(
-      GetPluginList()->GetPluginGroup(plugin));
-  string16 group_name = group->GetGroupName();
+  PluginFinder* finder = PluginFinder::GetInstance();
+  string16 group_name = finder->GetPluginMetadata(plugin)->name();
 
   // Check if the plug-in or its group is enabled by policy.
   PolicyStatus plugin_status = PolicyStatusForPlugin(plugin.name);
   PolicyStatus group_status = PolicyStatusForPlugin(group_name);
   if (plugin_status == POLICY_ENABLED || group_status == POLICY_ENABLED)
     return true;
 
   // Check if the plug-in or its group is disabled by policy.
   if (plugin_status == POLICY_DISABLED || group_status == POLICY_DISABLED)
     return false;
@@ skipping 322 matching lines @@
   policy_enabled_plugin_patterns_ = enabled_patterns;
 }
 
 webkit::npapi::PluginList* PluginPrefs::GetPluginList() const {
   if (plugin_list_)
     return plugin_list_;
   return PluginService::GetInstance()->GetPluginList();
 }
 
 void PluginPrefs::GetPreferencesDataOnFileThread() {
-  PluginFinder::Get(
-      base::Bind(&PluginPrefs::GetPluginFinderForGetPreferencesDataOnFileThread,
-                 this));
-}
-
-void PluginPrefs::GetPluginFinderForGetPreferencesDataOnFileThread(
-    PluginFinder* finder) {
   std::vector<webkit::WebPluginInfo> plugins;
   webkit::npapi::PluginList* plugin_list = GetPluginList();
   plugin_list->GetPluginsNoRefresh(&plugins);
 
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-      base::Bind(&PluginPrefs::OnUpdatePreferences, this, plugins, finder));
+      base::Bind(&PluginPrefs::OnUpdatePreferences, this, plugins));
 }
 
 void PluginPrefs::OnUpdatePreferences(
-    const std::vector<webkit::WebPluginInfo>& plugins,
-    PluginFinder* finder) {
+    const std::vector<webkit::WebPluginInfo>& plugins) {
   if (!prefs_)
     return;
 
+  PluginFinder* finder = PluginFinder::GetInstance();
   ListPrefUpdate update(prefs_, prefs::kPluginsPluginsList);
   ListValue* plugins_list = update.Get();
   plugins_list->Clear();
 
   FilePath internal_dir;
   if (PathService::Get(chrome::DIR_INTERNAL_PLUGINS, &internal_dir))
     prefs_->SetFilePath(prefs::kPluginsLastInternalDirectory, internal_dir);
 
   base::AutoLock auto_lock(lock_);
 
   // Add the plugin files.
   std::set<string16> group_names;
   for (size_t i = 0; i < plugins.size(); ++i) {
     DictionaryValue* summary = new DictionaryValue();
     summary->SetString("path", plugins[i].path.value());
     summary->SetString("name", plugins[i].name);
     summary->SetString("version", plugins[i].version);
     bool enabled = true;
     plugin_state_.Get(plugins[i].path, &enabled);
     summary->SetBoolean("enabled", enabled);
     plugins_list->Append(summary);
 
-    PluginInstaller* installer = finder->GetPluginInstaller(plugins[i]);
+    PluginMetadata* plugin_metadata = finder->GetPluginMetadata(plugins[i]);
     // Insert into a set of all group names.
-    group_names.insert(installer->name());
+    group_names.insert(plugin_metadata->name());
   }
 
   // Add the plug-in groups.
   for (std::set<string16>::const_iterator it = group_names.begin();
       it != group_names.end(); ++it) {
     DictionaryValue* summary = new DictionaryValue();
     summary->SetString("name", *it);
     bool enabled = true;
     std::map<string16, bool>::iterator gstate_it =
         plugin_group_state_.find(*it);
     if (gstate_it != plugin_group_state_.end())
       enabled = gstate_it->second;
     summary->SetBoolean("enabled", enabled);
     plugins_list->Append(summary);
   }
 }
 
 void PluginPrefs::NotifyPluginStatusChanged() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_PLUGIN_ENABLE_STATUS_CHANGED,
       content::Source<Profile>(profile_),
       content::NotificationService::NoDetails());
 }