Fix a deadlock in NPAPI::PluginList::LoadPlugins.

Also, keep the list of disabled plug-in groups in sync with their state.

BUG=57953
TEST=See bug for manual test

Review URL: http://codereview.chromium.org/3565009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@61927 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 32 insertions, 40 deletions

diff --git a/webkit/glue/plugins/plugin_group.cc b/webkit/glue/plugins/plugin_group.cc
index 7287c17..f774957 100644
--- a/webkit/glue/plugins/plugin_group.cc
+++ b/webkit/glue/plugins/plugin_group.cc
@@ -414,13 +414,3 @@ void PluginGroup::Enable(bool enable) {
     }
   }
 }
-
-std::vector<FilePath> PluginGroup::GetPaths() const {
-  std::vector<FilePath> rv;
-  for (std::vector<WebPluginInfo>::const_iterator it =
-       web_plugin_infos_.begin();
-       it != web_plugin_infos_.end(); ++it) {
-    rv.push_back(it->path);
-  }
-  return rv;
-}
diff --git a/webkit/glue/plugins/plugin_group.h b/webkit/glue/plugins/plugin_group.h
index 43640fd..fb193d5 100644
--- a/webkit/glue/plugins/plugin_group.h
+++ b/webkit/glue/plugins/plugin_group.h
@@ -131,10 +131,6 @@ class PluginGroup {
   // minimum version.
   void DisableOutdatedPlugins();
 
- protected:
-  friend class NPAPI::PluginList;
-  std::vector<FilePath> GetPaths() const;
-
  private:
   FRIEND_TEST_ALL_PREFIXES(PluginGroupTest, PluginGroupDefinition);
 
diff --git a/webkit/glue/plugins/plugin_list.cc b/webkit/glue/plugins/plugin_list.cc
index c6ec882..f312ab8 100644
--- a/webkit/glue/plugins/plugin_list.cc
+++ b/webkit/glue/plugins/plugin_list.cc
@@ -168,6 +168,15 @@ PluginList::PluginList()
   PlatformInit();
 }
 
+bool PluginList::ShouldDisableGroup(const string16& group_name) {
+  AutoLock lock(lock_);
+  if (PluginGroup::IsPluginNameDisabledByPolicy(group_name)) {
+    disabled_groups_.insert(group_name);
+    return true;
+  }
+  return disabled_groups_.count(group_name) > 0;
+}
+
 void PluginList::LoadPlugins(bool refresh) {
   // Don't want to hold the lock while loading new plugins, so we don't block
   // other methods if they're called on other threads.
@@ -229,15 +238,6 @@ void PluginList::LoadPlugins(bool refresh) {
   if (webkit_glue::IsDefaultPluginEnabled())
     LoadPlugin(FilePath(kDefaultPluginLibraryName), &new_plugins);
 
-  // Only update the data now since loading plugins can take a while.
-  AutoLock lock(lock_);
-
-  // Mark disabled plugins as such.
-  for (size_t i = 0; i < new_plugins.size(); ++i) {
-    if (disabled_plugins_.count(new_plugins[i].path))
-      new_plugins[i].enabled = false;
-  }
-
   // Disable all of the plugins and plugin groups that are disabled by policy.
   // There's currenly a bug that makes it impossible to correctly re-enable
   // plugins or plugin-groups to their original, "pre-policy" state, so
@@ -247,28 +247,28 @@ void PluginList::LoadPlugins(bool refresh) {
   GetPluginGroups(&new_plugins, &plugin_groups);
   for (PluginMap::const_iterator it = plugin_groups.begin();
        it != plugin_groups.end(); ++it) {
-    string16 group_name = it->second->GetGroupName();
-    if (PluginGroup::IsPluginNameDisabledByPolicy(group_name) &&
-        !disabled_groups_.count(group_name)) {
-      disabled_groups_.insert(group_name);
+    PluginGroup* group = it->second.get();
+    string16 group_name = group->GetGroupName();
+    if (ShouldDisableGroup(group_name)) {
+      it->second->Enable(false);
     }
 
-    if (disabled_groups_.count(group_name)) {
-      // Disable the plugins manually instead of PluginGroup::Enable(false)
-      // since that will need to acquire a lock which we already have.
-      std::vector<FilePath> paths = it->second->GetPaths();
-      for (size_t i = 0; i < paths.size(); ++i) {
-        for (size_t j = 0; j < new_plugins.size(); ++j) {
-          if (new_plugins[j].path == paths[i]) {
-            new_plugins[j].enabled = false;
-            break;
-          }
-        }
+    if (disable_outdated_plugins_) {
+      group->DisableOutdatedPlugins();
+      if (!group->Enabled()) {
+        AutoLock lock(lock_);
+        disabled_groups_.insert(group_name);
       }
     }
+  }
 
-    if (disable_outdated_plugins_)
-      it->second->DisableOutdatedPlugins();
+  // Only update the data now since loading plugins can take a while.
+  AutoLock lock(lock_);
+
+  // Mark disabled plugins as such.
+  for (size_t i = 0; i < new_plugins.size(); ++i) {
+    if (disabled_plugins_.count(new_plugins[i].path))
+      new_plugins[i].enabled = false;
   }
 
   plugins_ = new_plugins;
diff --git a/webkit/glue/plugins/plugin_list.h b/webkit/glue/plugins/plugin_list.h
index a67254f..2ab0b0a 100644
--- a/webkit/glue/plugins/plugin_list.h
+++ b/webkit/glue/plugins/plugin_list.h
@@ -211,6 +211,12 @@ class PluginList {
   bool ShouldLoadPlugin(const WebPluginInfo& info,
                         std::vector<WebPluginInfo>* plugins);
 
+  // Return whether a plug-in group with the given name should be disabled,
+  // either because it already is on the list of disabled groups, or because it
+  // is blacklisted by a policy. In the latter case, add the plugin group to the
+  // list of disabled groups as well.
+  bool ShouldDisableGroup(const string16& group_name);
+
   // Find a plugin by mime type; only searches enabled plugins.
   // The allow_wildcard parameter controls whether this function returns
   // plugins which support wildcard mime types (* as the mime type)