Changing PluginPrefs to use PluginFinder's async interface.

chrome/browser/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/plugin_prefs.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/singleton.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "base/version.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/plugin_installer.h"
 #include "chrome/browser/plugin_prefs_factory.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/browser/profiles/profile_manager.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"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/plugin_service.h"
 #include "webkit/plugins/npapi/plugin_group.h"
 #include "webkit/plugins/npapi/plugin_list.h"
 #include "webkit/plugins/webplugininfo.h"
 
 using content::BrowserThread;
 using content::PluginService;
 
 namespace {
 
 // Default state for a plug-in (not state of the default plug-in!).
 // Accessed only on the UI thread.
 base::LazyInstance<std::map<FilePath, bool> > g_default_plugin_state =
     LAZY_INSTANCE_INITIALIZER;
 
 class CallbackBarrier : public base::RefCountedThreadSafe<CallbackBarrier> {
  public:
-  explicit CallbackBarrier(const base::Closure& callback)
+  explicit CallbackBarrier(const base::Callback<void(bool)>& callback)
       : callback_(callback),
         outstanding_callbacks_(0) {
     DCHECK(!callback_.is_null());
   }
 
-  base::Closure CreateCallback() {
+  base::Callback<void(bool)> CreateCallback() {
     outstanding_callbacks_++;
     return base::Bind(&CallbackBarrier::MaybeRunCallback, this);
   }
 
  private:
   friend class base::RefCountedThreadSafe<CallbackBarrier>;
 
   ~CallbackBarrier() {
     DCHECK(callback_.is_null());
   }
 
-  void MaybeRunCallback() {
+  void MaybeRunCallback(bool dummy) {

[Bernhard Bauer, 2012/08/29 16:53:21]

Naming this parameter "dummy" is a bit confusing, because it's not actually a
dummy parameter.

It's also a bit weird to always return the last value. Maybe we should modify
the behavior so that it returns true if all (or any?) of the plug-ins could be
enabled?

[ibraaaa, 2012/08/29 17:37:53]

If I didn't miss something:

I checked the callers for EnablePluginGlobally, it is only called when Pepper
flash plugin is freshly installed. It is also called without checking if the
plugin state can be changed or not. So this gets us back a little bit, I assumed
when you asked to merge CanEnablePlugin and EnablePlugin that checking if the
plugin state can change (i.e. calling CanEnablePlugin) before calling
EnablePlugin will not affect the behavior of the code in this case (I mean in
EnablePluginGlobally).

Now, I intentionally named the parameter dummy and didn't apply any logic
because according to the old behavior of the  code there is no check need to be
performed on the result of 'CanEnablePlugin' simply because it was not called
before :)

I just don't see the point in Converting "dummy" to not "dummy"?

Done.

On 2012/08/29 16:53:21, Bernhard Bauer wrote:

[Bernhard Bauer, 2012/08/29 18:10:17]

What I meant was that it's weird to return an arbitrary value, even if we know
that it's going to be ignored. We could either not return any value at all, or
have a defined behavior (which is what you implemented). I'm fine with any of
those.

[ibraaaa, 2012/08/29 18:28:19]

Ah, I see. My intention was not returning any value at all so I thought that
fulfilling this can just be done by naming the value returned dummy, apparently
not :)

On 2012/08/29 18:10:17, Bernhard Bauer wrote:

     DCHECK_GT(outstanding_callbacks_, 0);
     if (--outstanding_callbacks_ == 0) {
-      callback_.Run();
+      callback_.Run(dummy);
       callback_.Reset();
     }
   }
 
-  base::Closure callback_;
+  base::Callback<void(bool)> callback_;
   int outstanding_callbacks_;
 };
 
 }  // namespace
 
 // How long to wait to save the plugin enabled information, which might need to
 // go to disk.
 #define kPluginUpdateDelayMs (60 * 1000)
 
 // static
 scoped_refptr<PluginPrefs> PluginPrefs::GetForProfile(Profile* profile) {
   return PluginPrefsFactory::GetPrefsForProfile(profile);
 }
 
 // static
 scoped_refptr<PluginPrefs> PluginPrefs::GetForTestingProfile(
     Profile* profile) {
   return static_cast<PluginPrefs*>(
       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) {
-  PluginService::GetInstance()->GetPluginGroups(
-        base::Bind(&PluginPrefs::EnablePluginGroupInternal,
-                   this, enabled, 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));
 }
 
 void PluginPrefs::EnablePluginGroupInternal(
     bool enabled,
     const string16& group_name,
-    const std::vector<webkit::npapi::PluginGroup>& groups) {
+    PluginFinder* finder,
+    const std::vector<webkit::WebPluginInfo>& plugins) {
   base::AutoLock auto_lock(lock_);
 
   // 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 < groups.size(); ++i) {
-    if (groups[i].GetGroupName() != group_name)
+  for (size_t i = 0; i < plugins.size(); ++i) {
+    PluginInstaller* installer = finder->GetPluginInstaller(plugins[i]);
+    if (group_name != installer->name())
       continue;
-    const std::vector<webkit::WebPluginInfo>& plugins =
-        groups[i].web_plugin_infos();
-    for (size_t j = 0; j < plugins.size(); ++j)
-      plugin_state_[plugins[j].path] = enabled;
-    break;
+    plugin_state_[plugins[i].path] = enabled;
   }
 
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-      base::Bind(&PluginPrefs::OnUpdatePreferences, this, groups));
+      base::Bind(&PluginPrefs::OnUpdatePreferences, this, plugins, finder));
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
       base::Bind(&PluginPrefs::NotifyPluginStatusChanged, this));
 }
 
-bool PluginPrefs::CanEnablePlugin(bool enabled, const FilePath& path) {
-  webkit::npapi::PluginList* plugin_list = GetPluginList();
+void PluginPrefs::EnablePluginIfPossibleCallback(
+    bool enabled, const FilePath& path,
+    const base::Callback<void(bool)>& callback,
+    PluginFinder* finder) {
   webkit::WebPluginInfo plugin;
+  bool can_enable = true;
   if (PluginService::GetInstance()->GetPluginInfoByPath(path, &plugin)) {
-    scoped_ptr<webkit::npapi::PluginGroup> group(
-        plugin_list->GetPluginGroup(plugin));
+    PluginInstaller* installer = finder->GetPluginInstaller(plugin);
     PolicyStatus plugin_status = PolicyStatusForPlugin(plugin.name);
-    PolicyStatus group_status = PolicyStatusForPlugin(group->GetGroupName());
+    PolicyStatus group_status = PolicyStatusForPlugin(installer->name());
     if (enabled) {
       if (plugin_status == POLICY_DISABLED || group_status == POLICY_DISABLED)
-        return false;
+        can_enable = false;
     } else {
       if (plugin_status == POLICY_ENABLED || group_status == POLICY_ENABLED)
-        return false;
+        can_enable = false;
     }
   } else {
     NOTREACHED();
   }
-  return true;
+
+  if (!can_enable) {
+    callback.Run(false);
+    return;
+  }
+
+  PluginService::GetInstance()->GetPlugins(
+      base::Bind(&PluginPrefs::EnablePluginInternal, this,
+                 enabled, path, finder, callback));
 }
 
-void PluginPrefs::EnablePlugin(bool enabled, const FilePath& path,
-                               const base::Closure& callback) {
-  PluginService::GetInstance()->GetPluginGroups(
-      base::Bind(&PluginPrefs::EnablePluginInternal, this,
-                 enabled, path, 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,
-    const base::Closure& callback,
-    const std::vector<webkit::npapi::PluginGroup>& groups) {
+    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_[path] = enabled;
   }
 
-  bool found_group = false;
-  for (size_t i = 0; i < groups.size(); ++i) {
-    bool all_disabled = true;
-    const std::vector<webkit::WebPluginInfo>& plugins =
-        groups[i].web_plugin_infos();
-    for (size_t j = 0; j < plugins.size(); ++j) {
-      all_disabled = all_disabled && !IsPluginEnabled(plugins[j]);
-      if (plugins[j].path == path) {
-        found_group = true;
-        DCHECK_EQ(enabled, IsPluginEnabled(plugins[j]));
-      }
-    }
-    if (found_group) {
-      // Update the state for the corresponding plug-in group.
-      base::AutoLock auto_lock(lock_);
-      plugin_group_state_[groups[i].GetGroupName()] = !all_disabled;
+  string16 group_name;
+  for (size_t i = 0; i < plugins.size(); ++i) {
+    if (plugins[i].path == path) {
+      PluginInstaller* installer =
+          plugin_finder->GetPluginInstaller(plugins[i]);
+      // set the group name for this plug-in.
+      group_name = installer->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()) {
+      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, groups));
+      base::Bind(&PluginPrefs::OnUpdatePreferences, this,
+                 plugins, plugin_finder));
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
       base::Bind(&PluginPrefs::NotifyPluginStatusChanged, this));
-  callback.Run();
+  callback.Run(true);
 }
 
 // static
-void PluginPrefs::EnablePluginGlobally(bool enable, const FilePath& file_path,
-                                       const base::Closure& callback) {
+void PluginPrefs::EnablePluginGlobally(
+    bool enable,
+    const FilePath& file_path,
+    const base::Callback<void(bool)>& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   g_default_plugin_state.Get()[file_path] = enable;
   std::vector<Profile*> profiles =
       g_browser_process->profile_manager()->GetLoadedProfiles();
   scoped_refptr<CallbackBarrier> barrier = new CallbackBarrier(callback);
   for (std::vector<Profile*>::iterator it = profiles.begin();
        it != profiles.end(); ++it) {
     PluginPrefs* plugin_prefs = PluginPrefs::GetForProfile(*it);
     DCHECK(plugin_prefs);
     plugin_prefs->EnablePlugin(enable, file_path, barrier->CreateCallback());
@@ skipping 289 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() {
-  std::vector<webkit::npapi::PluginGroup> groups;
+  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->GetPluginGroups(false, &groups);
+  plugin_list->GetPluginsNoRefresh(&plugins);
 
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-      base::Bind(&PluginPrefs::OnUpdatePreferences, this, groups));
+      base::Bind(&PluginPrefs::OnUpdatePreferences, this, plugins, finder));
 }
 
 void PluginPrefs::OnUpdatePreferences(
-    const std::vector<webkit::npapi::PluginGroup>& groups) {
+    const std::vector<webkit::WebPluginInfo>& plugins,
+    PluginFinder* finder) {
   if (!prefs_)
     return;
 
   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;
+    std::map<FilePath, bool>::iterator it = plugin_state_.find(plugins[i].path);
+    if (it != plugin_state_.end())
+      enabled = it->second;
+    summary->SetBoolean("enabled", enabled);
+    plugins_list->Append(summary);
+
+    PluginInstaller* installer = finder->GetPluginInstaller(plugins[i]);
+    // Insert into a set of all group names.
+    group_names.insert(installer->name());
+  }
+
   // Add the plug-in groups.
-  for (size_t i = 0; i < groups.size(); ++i) {
-    // Add the plugin files to the same list.
-    const std::vector<webkit::WebPluginInfo>& plugins =
-        groups[i].web_plugin_infos();
-    for (size_t j = 0; j < plugins.size(); ++j) {
-      DictionaryValue* summary = new DictionaryValue();
-      summary->SetString("path", plugins[j].path.value());
-      summary->SetString("name", plugins[j].name);
-      summary->SetString("version", plugins[j].version);
-      bool enabled = true;
-      std::map<FilePath, bool>::iterator it =
-          plugin_state_.find(plugins[j].path);
-      if (it != plugin_state_.end())
-        enabled = it->second;
-      summary->SetBoolean("enabled", enabled);
-      plugins_list->Append(summary);
-    }
-
+  for (std::set<string16>::const_iterator it = group_names.begin();
+      it != group_names.end(); ++it) {
     DictionaryValue* summary = new DictionaryValue();
-    string16 name = groups[i].GetGroupName();
-    summary->SetString("name", name);
+    summary->SetString("name", *it);
     bool enabled = true;
-    std::map<string16, bool>::iterator it =
-        plugin_group_state_.find(name);
-    if (it != plugin_group_state_.end())
-      enabled = it->second;
+    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());
 }