PluginList cleanup to fix a race condition and decrease the API surface for future refactorings:

* Remove "webkit/glue/plugins/plugin_list.h" includes in favor of "webkit/npapi/plugins/plugin_list.h"
* Remove |refresh| parameter from |GetPlugins| in favor of calling |RefreshPlugins| beforehand.
* Remove |GetPluginInfo| in favor of calling |GetPluginInfoArray| and looking for the first enabled plug-in.
* Remove |GetEnabledPlugins| in favor of calling |GetPlugins| and filtering out disabled plugins.
* Remove |stale| in favor of an outparameter to |GetPluginInfoArray|, to remove the race condition.

BUG=69516,80794
TEST=none

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

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

3 files changed, 27 insertions, 118 deletions

diff --git a/webkit/plugins/npapi/plugin_list.cc b/webkit/plugins/npapi/plugin_list.cc
index 6441a76..1fae6b3 100644
--- a/webkit/plugins/npapi/plugin_list.cc
+++ b/webkit/plugins/npapi/plugin_list.cc
@@ -397,10 +397,10 @@ void PluginList::LoadPluginsInternal(ScopedVector<PluginGroup>* plugin_groups) {
     LoadPlugin(FilePath(kDefaultPluginLibraryName), plugin_groups);
 }
 
-void PluginList::LoadPlugins(bool refresh) {
+void PluginList::LoadPlugins() {
   {
     base::AutoLock lock(lock_);
-    if (!refresh && !plugins_need_refresh_)
+    if (!plugins_need_refresh_)
       return;
   }
 
@@ -482,7 +482,7 @@ void PluginList::LoadPlugin(const FilePath& path,
       // WebKit hands to the Plugin before it tries
       // to handle mimeTypes on its own.
       const std::string &mime_type = plugin_info.mime_types[i].mime_type;
-      if (mime_type == "*" )
+      if (mime_type == "*")
         return;
     }
   }
@@ -491,9 +491,8 @@ void PluginList::LoadPlugin(const FilePath& path,
   AddToPluginGroups(plugin_info, plugin_groups);
 }
 
-void PluginList::GetPlugins(bool refresh, std::vector<WebPluginInfo>* plugins) {
-  LoadPlugins(refresh);
-
+void PluginList::GetPlugins(std::vector<WebPluginInfo>* plugins) {
+  LoadPlugins();
   base::AutoLock lock(lock_);
   for (size_t i = 0; i < plugin_groups_.size(); ++i) {
     const std::vector<WebPluginInfo>& gr_plugins =
@@ -502,33 +501,21 @@ void PluginList::GetPlugins(bool refresh, std::vector<WebPluginInfo>* plugins) {
   }
 }
 
-void PluginList::GetEnabledPlugins(bool refresh,
-                                   std::vector<WebPluginInfo>* plugins) {
-  LoadPlugins(refresh);
-
-  plugins->clear();
-  base::AutoLock lock(lock_);
-  for (size_t i = 0; i < plugin_groups_.size(); ++i) {
-    const std::vector<WebPluginInfo>& gr_plugins =
-        plugin_groups_[i]->web_plugins_info();
-    for (size_t i = 0; i < gr_plugins.size(); ++i) {
-      if (IsPluginEnabled(gr_plugins[i]))
-        plugins->push_back(gr_plugins[i]);
-    }
-  }
-}
-
 void PluginList::GetPluginInfoArray(
     const GURL& url,
     const std::string& mime_type,
     bool allow_wildcard,
+    bool* use_stale,
     std::vector<WebPluginInfo>* info,
     std::vector<std::string>* actual_mime_types) {
   DCHECK(mime_type == StringToLowerASCII(mime_type));
   DCHECK(info);
 
-  LoadPlugins(false);
+  if (!use_stale)
+    LoadPlugins();
   base::AutoLock lock(lock_);
+  if (use_stale)
+    *use_stale = plugins_need_refresh_;
   info->clear();
   if (actual_mime_types)
     actual_mime_types->clear();
@@ -617,38 +604,9 @@ void PluginList::GetPluginInfoArray(
   }
 }
 
-bool PluginList::GetPluginInfo(const GURL& url,
-                               const std::string& mime_type,
-                               bool allow_wildcard,
-                               WebPluginInfo* info,
-                               std::string* actual_mime_type) {
-  DCHECK(info);
-  std::vector<WebPluginInfo> info_list;
-
-  // GetPluginInfoArray has slightly less work to do if we can pass
-  // NULL for the mime type list...
-  if (actual_mime_type) {
-    std::vector<std::string> mime_type_list;
-    GetPluginInfoArray(
-        url, mime_type, allow_wildcard, &info_list, &mime_type_list);
-    if (!info_list.empty()) {
-      *info = info_list[0];
-      *actual_mime_type = mime_type_list[0];
-      return true;
-    }
-  } else {
-    GetPluginInfoArray(url, mime_type, allow_wildcard, &info_list, NULL);
-    if (!info_list.empty()) {
-      *info = info_list[0];
-      return true;
-    }
-  }
-  return false;
-}
-
 bool PluginList::GetPluginInfoByPath(const FilePath& plugin_path,
                                      WebPluginInfo* info) {
-  LoadPlugins(false);
+  LoadPlugins();
   base::AutoLock lock(lock_);
   for (size_t i = 0; i < plugin_groups_.size(); ++i) {
     const std::vector<WebPluginInfo>& plugins =
@@ -668,7 +626,7 @@ void PluginList::GetPluginGroups(
     bool load_if_necessary,
     std::vector<PluginGroup>* plugin_groups) {
   if (load_if_necessary)
-    LoadPlugins(false);
+    LoadPlugins();
   base::AutoLock lock(lock_);
   plugin_groups->clear();
   for (size_t i = 0; i < plugin_groups_.size(); ++i) {
diff --git a/webkit/plugins/npapi/plugin_list.h b/webkit/plugins/npapi/plugin_list.h
index e575a91..46c0bf6 100644
--- a/webkit/plugins/npapi/plugin_list.h
+++ b/webkit/plugins/npapi/plugin_list.h
@@ -118,10 +118,7 @@ class PluginList {
                              std::vector<WebPluginMimeType>* parsed_mime_types);
 
   // Get all the plugins.
-  void GetPlugins(bool refresh, std::vector<WebPluginInfo>* plugins);
-
-  // Get all the enabled plugins.
-  void GetEnabledPlugins(bool refresh, std::vector<WebPluginInfo>* plugins);
+  void GetPlugins(std::vector<WebPluginInfo>* plugins);
 
   // Returns a list in |info| containing plugins that are found for
   // the given url and mime type (including disabled plugins, for
@@ -133,20 +130,16 @@ class PluginList {
   // type).  The |info| parameter is required to be non-NULL.  The
   // list is in order of "most desirable" to "least desirable",
   // meaning that the default plugin is at the end of the list.
+  // If |use_stale| is NULL, this will load the plug-in list if necessary.
+  // If it is not NULL, the plug-in list will not be loaded, and |*use_stale|
+  // will be true iff the plug-in list was stale.
   void GetPluginInfoArray(const GURL& url,
                           const std::string& mime_type,
                           bool allow_wildcard,
+                          bool* use_stale,
                           std::vector<WebPluginInfo>* info,
                           std::vector<std::string>* actual_mime_types);
 
-  // Returns the first item from the list returned in GetPluginInfo in |info|.
-  // Returns true if it found a match.  |actual_mime_type| may be NULL.
-  bool GetPluginInfo(const GURL& url,
-                     const std::string& mime_type,
-                     bool allow_wildcard,
-                     WebPluginInfo* info,
-                     std::string* actual_mime_type);
-
   // Get plugin info by plugin path (including disabled plugins). Returns true
   // if the plugin is found and WebPluginInfo has been filled in |info|.
   bool GetPluginInfoByPath(const FilePath& plugin_path,
@@ -203,10 +196,6 @@ class PluginList {
   // version.
   void DisableOutdatedPluginGroups();
 
-  // Returns true if the plugin list is stale, i.e. it will need to be
-  // (re)loaded on the next access.
-  bool stale() { return plugins_need_refresh_; }
-
   virtual ~PluginList();
 
  protected:
@@ -238,7 +227,7 @@ class PluginList {
   virtual void LoadPluginsInternal(ScopedVector<PluginGroup>* plugin_groups);
 
   // Load all plugins from the default plugins directory
-  void LoadPlugins(bool refresh);
+  void LoadPlugins();
 
   // Load all plugins from a specific directory.
   // |plugins| is updated with loaded plugin information.
diff --git a/webkit/plugins/npapi/plugin_list_unittest.cc b/webkit/plugins/npapi/plugin_list_unittest.cc
index 785fc3c..0782eb8 100644
--- a/webkit/plugins/npapi/plugin_list_unittest.cc
+++ b/webkit/plugins/npapi/plugin_list_unittest.cc
@@ -74,19 +74,12 @@ class PluginListTest : public testing::Test {
 
 TEST_F(PluginListTest, GetPlugins) {
   std::vector<WebPluginInfo> plugins;
-  plugin_list_.GetPlugins(false, &plugins);
+  plugin_list_.GetPlugins(&plugins);
   EXPECT_EQ(2u, plugins.size());
   EXPECT_TRUE(Contains(plugins, foo_plugin_, true));
   EXPECT_TRUE(Contains(plugins, bar_plugin_, true));
 }
 
-TEST_F(PluginListTest, GetEnabledPlugins) {
-  std::vector<WebPluginInfo> plugins;
-  plugin_list_.GetEnabledPlugins(false, &plugins);
-  EXPECT_EQ(1u, plugins.size());
-  EXPECT_TRUE(Contains(plugins, foo_plugin_, true));
-}
-
 TEST_F(PluginListTest, GetPluginGroup) {
   const PluginGroup* foo_group = plugin_list_.GetPluginGroup(foo_plugin_);
   EXPECT_EQ(ASCIIToUTF16(kFooGroupName), foo_group->GetGroupName());
@@ -98,41 +91,6 @@ TEST_F(PluginListTest, GetPluginGroup) {
   EXPECT_FALSE(bar_group->Enabled());
 }
 
-TEST_F(PluginListTest, EnableDisablePlugin) {
-  // Disable "foo" plugin.
-  plugin_list_.DisablePlugin(foo_plugin_.path);
-  std::vector<WebPluginInfo> plugins;
-  plugin_list_.GetEnabledPlugins(false, &plugins);
-  EXPECT_FALSE(Contains(plugins, foo_plugin_, false));
-  const PluginGroup* foo_group = plugin_list_.GetPluginGroup(foo_plugin_);
-  EXPECT_FALSE(foo_group->Enabled());
-  // Enable "bar" plugin.
-  plugin_list_.EnablePlugin(bar_plugin_.path);
-  plugin_list_.GetEnabledPlugins(false, &plugins);
-  EXPECT_TRUE(Contains(plugins, bar_plugin_, false));
-  const PluginGroup* bar_group = plugin_list_.GetPluginGroup(bar_plugin_);
-  EXPECT_TRUE(bar_group->Enabled());
-}
-
-TEST_F(PluginListTest, EnableGroup) {
-  // Disable "foo" plugin group.
-  const PluginGroup* foo_group = plugin_list_.GetPluginGroup(foo_plugin_);
-  EXPECT_TRUE(foo_group->Enabled());
-  EXPECT_TRUE(plugin_list_.EnableGroup(false, foo_group->GetGroupName()));
-  EXPECT_FALSE(foo_group->Enabled());
-  std::vector<WebPluginInfo> plugins;
-  plugin_list_.GetEnabledPlugins(false, &plugins);
-  EXPECT_EQ(0u, plugins.size());
-  EXPECT_FALSE(Contains(plugins, foo_plugin_, false));
-  // Enable "bar" plugin group.
-  const PluginGroup* bar_group = plugin_list_.GetPluginGroup(bar_plugin_);
-  EXPECT_FALSE(bar_group->Enabled());
-  plugin_list_.EnableGroup(true, bar_group->GetGroupName());
-  EXPECT_TRUE(bar_group->Enabled());
-  plugin_list_.GetEnabledPlugins(false, &plugins);
-  EXPECT_TRUE(Contains(plugins, bar_plugin_, false));
-}
-
 TEST_F(PluginListTest, EmptyGroup) {
   std::vector<PluginGroup> groups;
   plugin_list_.GetPluginGroups(false, &groups);
@@ -157,8 +115,9 @@ TEST_F(PluginListTest, DisableOutdated) {
   plugin_list_.AddPluginToLoad(plugin_3043);
   plugin_list_.AddPluginToLoad(plugin_3045);
   // Enfore the load to run.
+  plugin_list_.RefreshPlugins();
   std::vector<WebPluginInfo> plugins;
-  plugin_list_.GetPlugins(true, &plugins);
+  plugin_list_.GetPlugins(&plugins);
   PluginGroup* group_3043 =
       const_cast<PluginGroup*>(plugin_list_.GetPluginGroup(plugin_3043));
   const PluginGroup* group_3045 = plugin_list_.GetPluginGroup(plugin_3045);
@@ -181,8 +140,9 @@ TEST_F(PluginListTest, BadPluginDescription) {
   plugin_list_.ClearPluginsToLoad();
   plugin_list_.AddPluginToLoad(plugin_3043);
   // Now we should have them in the state we specified above.
+  plugin_list_.RefreshPlugins();
   std::vector<WebPluginInfo> plugins;
-  plugin_list_.GetPlugins(true, &plugins);
+  plugin_list_.GetPlugins(&plugins);
   ASSERT_TRUE(Contains(plugins, plugin_3043, true));
 }
 
@@ -204,8 +164,9 @@ TEST_F(PluginListTest, DisableAndEnableBeforeLoad) {
   plugin_list_.AddPluginToLoad(plugin_3043);
   plugin_list_.AddPluginToLoad(plugin_3045);
   // Now we should have them in the state we specified above.
+  plugin_list_.RefreshPlugins();
   std::vector<WebPluginInfo> plugins;
-  plugin_list_.GetPlugins(true, &plugins);
+  plugin_list_.GetPlugins(&plugins);
   plugin_3043.enabled = WebPluginInfo::USER_DISABLED_POLICY_UNMANAGED;
   ASSERT_TRUE(Contains(plugins, plugin_3043, true));
   ASSERT_TRUE(Contains(plugins, plugin_3045, true));
@@ -231,8 +192,9 @@ TEST_F(PluginListTest, DisableBeforeLoad) {
 
   EXPECT_TRUE(plugin_list_.EnableGroup(false, ASCIIToUTF16(kFooGroupName)));
 
+  plugin_list_.RefreshPlugins();
   std::vector<WebPluginInfo> plugins;
-  plugin_list_.GetPlugins(true, &plugins);
+  plugin_list_.GetPlugins(&plugins);
   ASSERT_EQ(2u, plugins.size());
   ASSERT_EQ(WebPluginInfo::USER_DISABLED_POLICY_UNMANAGED, plugins[0].enabled);
 }