Don't attempt to load a failed plugin twice.

We keep track of which plugins we've already considered loading,
and skip them if we encounter them again.  This situation is common
on Linux where people have many different symlinks pointing to the
same plugin.

BUG=38764
TEST=See bug for bad output without this patch; output with this patch is now good.

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

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

5 files changed, 33 insertions, 24 deletions

diff --git a/webkit/glue/plugins/plugin_list.cc b/webkit/glue/plugins/plugin_list.cc
index 2b78507..e8565d8 100644
--- a/webkit/glue/plugins/plugin_list.cc
+++ b/webkit/glue/plugins/plugin_list.cc
@@ -176,6 +176,7 @@ void PluginList::LoadPlugins(bool refresh) {
   base::TimeTicks start_time = base::TimeTicks::Now();
 
   std::vector<WebPluginInfo> new_plugins;
+  std::set<FilePath> visited_plugins;
 
   std::vector<FilePath> directories_to_scan;
   GetPluginDirectories(&directories_to_scan);
@@ -189,15 +190,20 @@ void PluginList::LoadPlugins(bool refresh) {
     LoadPlugin(internal_plugins[i].path, &new_plugins);
   }
 
-  for (size_t i = 0; i < extra_plugin_paths.size(); ++i)
-    LoadPlugin(extra_plugin_paths[i], &new_plugins);
+  for (size_t i = 0; i < extra_plugin_paths.size(); ++i) {
+    const FilePath& path = extra_plugin_paths[i];
+    if (visited_plugins.find(path) != visited_plugins.end())
+      continue;
+    LoadPlugin(path, &new_plugins);
+    visited_plugins.insert(path);
+  }
 
   for (size_t i = 0; i < extra_plugin_dirs.size(); ++i) {
-    LoadPluginsFromDir(extra_plugin_dirs[i], &new_plugins);
+    LoadPluginsFromDir(extra_plugin_dirs[i], &new_plugins, &visited_plugins);
   }
 
   for (size_t i = 0; i < directories_to_scan.size(); ++i) {
-    LoadPluginsFromDir(directories_to_scan[i], &new_plugins);
+    LoadPluginsFromDir(directories_to_scan[i], &new_plugins, &visited_plugins);
   }
 
   // Load the default plugin last.
diff --git a/webkit/glue/plugins/plugin_list.h b/webkit/glue/plugins/plugin_list.h
index c920397..0017399 100644
--- a/webkit/glue/plugins/plugin_list.h
+++ b/webkit/glue/plugins/plugin_list.h
@@ -7,6 +7,7 @@
 
 #include <string>
 #include <vector>
+#include <set>
 
 #include "base/basictypes.h"
 #include "base/file_path.h"
@@ -143,9 +144,13 @@ class PluginList {
   // Load all plugins from the default plugins directory
   void LoadPlugins(bool refresh);
 
-  // Load all plugins from a specific directory
+  // Load all plugins from a specific directory.
+  // |plugins| is updated with loaded plugin information.
+  // |visited_plugins| is updated with paths to all plugins that were considered
+  //   (including those we didn't load)
   void LoadPluginsFromDir(const FilePath& path,
-                          std::vector<WebPluginInfo>* plugins);
+                          std::vector<WebPluginInfo>* plugins,
+                          std::set<FilePath>* visited_plugins);
 
   // Returns true if we should load the given plugin, or false otherwise.
   // plugins is the list of plugins we have crawled in the current plugin
diff --git a/webkit/glue/plugins/plugin_list_mac.mm b/webkit/glue/plugins/plugin_list_mac.mm
index 7748f152..ab82398 100644
--- a/webkit/glue/plugins/plugin_list_mac.mm
+++ b/webkit/glue/plugins/plugin_list_mac.mm
@@ -57,13 +57,15 @@ void PluginList::GetPluginDirectories(std::vector<FilePath>* plugin_dirs) {
 }
 
 void PluginList::LoadPluginsFromDir(const FilePath &path,
-                                    std::vector<WebPluginInfo>* plugins) {
+                                    std::vector<WebPluginInfo>* plugins,
+                                    std::set<FilePath>* visited_plugins) {
   file_util::FileEnumerator enumerator(path,
                                        false, // not recursive
                                        file_util::FileEnumerator::DIRECTORIES);
   for (FilePath path = enumerator.Next(); !path.value().empty();
        path = enumerator.Next()) {
     LoadPlugin(path, plugins);
+    visited_plugins->insert(path);
   }
 }
 
diff --git a/webkit/glue/plugins/plugin_list_posix.cc b/webkit/glue/plugins/plugin_list_posix.cc
index b623a57..f46a84b 100644
--- a/webkit/glue/plugins/plugin_list_posix.cc
+++ b/webkit/glue/plugins/plugin_list_posix.cc
@@ -115,15 +115,16 @@ void PluginList::GetPluginDirectories(std::vector<FilePath>* plugin_dirs) {
 #endif
 }
 
-void PluginList::LoadPluginsFromDir(const FilePath& path,
-                                    std::vector<WebPluginInfo>* plugins) {
+void PluginList::LoadPluginsFromDir(const FilePath& dir_path,
+                                    std::vector<WebPluginInfo>* plugins,
+                                    std::set<FilePath>* visited_plugins) {
   // See ScanPluginsDirectory near
   // http://mxr.mozilla.org/firefox/source/modules/plugin/base/src/nsPluginHostImpl.cpp#5052
 
   // Construct and stat a list of all filenames under consideration, for
   // later sorting by mtime.
   FileTimeList files;
-  file_util::FileEnumerator enumerator(path,
+  file_util::FileEnumerator enumerator(dir_path,
                                        false,  // not recursive
                                        file_util::FileEnumerator::FILES);
   for (FilePath path = enumerator.Next(); !path.value().empty();
@@ -140,6 +141,12 @@ void PluginList::LoadPluginsFromDir(const FilePath& path,
     file_util::AbsolutePath(&path);
     PLUG_LOG << "Resolved " << orig_path.value() << " -> " << path.value();
 
+    if (visited_plugins->find(path) != visited_plugins->end()) {
+      PLUG_LOG << "Skipping duplicate instance of " << path.value();
+      continue;
+    }
+    visited_plugins->insert(path);
+
     // Flash stops working if the containing directory involves 'netscape'.
     // No joke.  So use the other path if it's better.
     static const char kFlashPlayerFilename[] = "libflashplayer.so";
@@ -161,19 +168,6 @@ void PluginList::LoadPluginsFromDir(const FilePath& path,
     if (!file_util::GetFileInfo(path, &info))
       continue;
 
-    // Skip duplicates of the same file in our list.
-    bool skip = false;
-    for (size_t i = 0; i < plugins->size(); ++i) {
-      if (plugins->at(i).path == path) {
-        skip = true;
-        break;
-      }
-    }
-    if (skip) {
-      PLUG_LOG << "Skipping duplicate instance of " << path.value();
-      continue;
-    }
-
     files.push_back(std::make_pair(path, info.last_modified));
   }
 
diff --git a/webkit/glue/plugins/plugin_list_win.cc b/webkit/glue/plugins/plugin_list_win.cc
index 25a3be4..a3feb0e 100644
--- a/webkit/glue/plugins/plugin_list_win.cc
+++ b/webkit/glue/plugins/plugin_list_win.cc
@@ -244,7 +244,8 @@ void PluginList::GetPluginDirectories(std::vector<FilePath>* plugin_dirs) {
 }
 
 void PluginList::LoadPluginsFromDir(const FilePath &path,
-                                    std::vector<WebPluginInfo>* plugins) {
+                                    std::vector<WebPluginInfo>* plugins,
+                                    std::set<FilePath>* visited_plugins) {
   WIN32_FIND_DATA find_file_data;
   HANDLE find_handle;
 
@@ -260,6 +261,7 @@ void PluginList::LoadPluginsFromDir(const FilePath &path,
     if (!(find_file_data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)) {
       FilePath filename = path.Append(find_file_data.cFileName);
       LoadPlugin(filename, plugins);
+      visited_plugins->insert(filename);
     }
   } while (FindNextFile(find_handle, &find_file_data) != 0);