Ensure we don't load plugins on the IO thread.

I had to move the locks from PluginService to PluginList, so that a lock (which can block other threads) isn't held while loading the plugins.

BUG=17938
TEST=added asserts which crash if plugins loaded on IO thread, current UI tests exercise them
Review URL: http://codereview.chromium.org/164305

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

1 files changed, 59 insertions, 46 deletions

diff --git a/webkit/glue/plugins/plugin_list.cc b/webkit/glue/plugins/plugin_list.cc
index f443364..44f7428 100644
--- a/webkit/glue/plugins/plugin_list.cc
+++ b/webkit/glue/plugins/plugin_list.cc
@@ -25,49 +25,44 @@ base::LazyInstance<PluginList> g_singleton(base::LINKER_INITIALIZED);
 
 // static
 PluginList* PluginList::Singleton() {
-  PluginList* singleton = g_singleton.Pointer();
-  if (!singleton->plugins_loaded_) {
-    singleton->LoadPlugins(false);
-    DCHECK(singleton->plugins_loaded_);
-  }
-  return singleton;
+  return g_singleton.Pointer();
+}
+
+bool PluginList::PluginsLoaded() {
+  AutoLock lock(lock_);
+  return plugins_loaded_;
 }
 
-// static
 void PluginList::ResetPluginsLoaded() {
-  // We access the singleton directly, and not through Singleton(), since
-  // we don't want LoadPlugins() to be called.
-  g_singleton.Pointer()->plugins_loaded_ = false;
+  AutoLock lock(lock_);
+  plugins_loaded_ = false;
 }
 
-// static
 void PluginList::AddExtraPluginPath(const FilePath& plugin_path) {
-  DCHECK(!g_singleton.Pointer()->plugins_loaded_);
-  g_singleton.Pointer()->extra_plugin_paths_.push_back(plugin_path);
+  AutoLock lock(lock_);
+  extra_plugin_paths_.push_back(plugin_path);
 }
 
-// static
 void PluginList::AddExtraPluginDir(const FilePath& plugin_dir) {
-  DCHECK(!g_singleton.Pointer()->plugins_loaded_);
-  g_singleton.Pointer()->extra_plugin_dirs_.push_back(plugin_dir);
+  AutoLock lock(lock_);
+  extra_plugin_dirs_.push_back(plugin_dir);
 }
 
 void PluginList::RegisterInternalPlugin(const PluginVersionInfo& info) {
-  DCHECK(!g_singleton.Pointer()->plugins_loaded_);
-  g_singleton.Pointer()->internal_plugins_.push_back(info);
+  AutoLock lock(lock_);
+  internal_plugins_.push_back(info);
 }
 
 bool PluginList::ReadPluginInfo(const FilePath &filename,
                                 WebPluginInfo* info,
                                 const PluginEntryPoints** entry_points) {
-  // We access the singleton directly, and not through Singleton(), since
-  // we might be in a LoadPlugins call and don't want to call it recursively!
-  const std::vector<PluginVersionInfo>& internal_plugins =
-      g_singleton.Pointer()->internal_plugins_;
-  for (size_t i = 0; i < internal_plugins.size(); ++i) {
-    if (filename == internal_plugins[i].path) {
-      *entry_points = &internal_plugins[i].entry_points;
-      return CreateWebPluginInfo(internal_plugins[i], info);
+  {
+    AutoLock lock(lock_);
+    for (size_t i = 0; i < internal_plugins_.size(); ++i) {
+      if (filename == internal_plugins_[i].path) {
+        *entry_points = &internal_plugins_[i].entry_points;
+        return CreateWebPluginInfo(internal_plugins_[i], info);
+      }
     }
   }
 
@@ -146,46 +141,60 @@ PluginList::PluginList() : plugins_loaded_(false) {
 }
 
 void PluginList::LoadPlugins(bool refresh) {
-  if (plugins_loaded_ && !refresh)
-    return;
-
-  plugins_.clear();
-  plugins_loaded_ = true;
+  // 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.
+  std::vector<FilePath> extra_plugin_paths;
+  std::vector<FilePath> extra_plugin_dirs;
+  {
+    AutoLock lock(lock_);
+    if (plugins_loaded_ && !refresh)
+      return;
+
+    extra_plugin_paths = extra_plugin_paths_;
+    extra_plugin_dirs = extra_plugin_dirs_;
+  }
 
   base::TimeTicks start_time = base::TimeTicks::Now();
 
+  std::vector<WebPluginInfo> new_plugins;
+
   std::vector<FilePath> directories_to_scan;
   GetPluginDirectories(&directories_to_scan);
 
-  for (size_t i = 0; i < extra_plugin_paths_.size(); ++i)
-    LoadPlugin(extra_plugin_paths_[i]);
+  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_dirs_.size(); ++i) {
-    LoadPluginsFromDir(extra_plugin_dirs_[i]);
+  for (size_t i = 0; i < extra_plugin_dirs.size(); ++i) {
+    LoadPluginsFromDir(extra_plugin_dirs[i], &new_plugins);
   }
 
   for (size_t i = 0; i < directories_to_scan.size(); ++i) {
-    LoadPluginsFromDir(directories_to_scan[i]);
+    LoadPluginsFromDir(directories_to_scan[i], &new_plugins);
   }
 
-  LoadInternalPlugins();
+  LoadInternalPlugins(&new_plugins);
 
   if (webkit_glue::IsDefaultPluginEnabled())
-    LoadPlugin(FilePath(kDefaultPluginLibraryName));
+    LoadPlugin(FilePath(kDefaultPluginLibraryName), &new_plugins);
 
   base::TimeTicks end_time = base::TimeTicks::Now();
   base::TimeDelta elapsed = end_time - start_time;
   DLOG(INFO) << "Loaded plugin list in " << elapsed.InMilliseconds() << " ms.";
+
+  AutoLock lock(lock_);
+  plugins_ = new_plugins;
+  plugins_loaded_ = true;
 }
 
-void PluginList::LoadPlugin(const FilePath &path) {
+void PluginList::LoadPlugin(const FilePath &path,
+                            std::vector<WebPluginInfo>* plugins) {
   WebPluginInfo plugin_info;
   const PluginEntryPoints* entry_points;
 
   if (!ReadPluginInfo(path, &plugin_info, &entry_points))
     return;
 
-  if (!ShouldLoadPlugin(plugin_info))
+  if (!ShouldLoadPlugin(plugin_info, plugins))
     return;
 
   if (path.value() != kDefaultPluginLibraryName
@@ -203,7 +212,7 @@ void PluginList::LoadPlugin(const FilePath &path) {
     }
   }
 
-  plugins_.push_back(plugin_info);
+  plugins->push_back(plugin_info);
 }
 
 bool PluginList::FindPlugin(const std::string& mime_type,
@@ -212,6 +221,8 @@ bool PluginList::FindPlugin(const std::string& mime_type,
                             WebPluginInfo* info) {
   DCHECK(mime_type == StringToLowerASCII(mime_type));
 
+  LoadPlugins(false);
+  AutoLock lock(lock_);
   for (size_t i = 0; i < plugins_.size(); ++i) {
     if (SupportsType(plugins_[i], mime_type, allow_wildcard)) {
 #if defined(OS_WIN)
@@ -232,6 +243,8 @@ bool PluginList::FindPlugin(const std::string& mime_type,
 
 bool PluginList::FindPlugin(const GURL &url, std::string* actual_mime_type,
                             WebPluginInfo* info) {
+  LoadPlugins(false);
+  AutoLock lock(lock_);
   std::string path = url.path();
   std::string::size_type last_dot = path.rfind('.');
   if (last_dot == std::string::npos)
@@ -286,13 +299,11 @@ bool PluginList::SupportsExtension(const WebPluginInfo& info,
 }
 
 
-bool PluginList::GetPlugins(bool refresh, std::vector<WebPluginInfo>* plugins) {
-  if (refresh)
-    LoadPlugins(true);
+void PluginList::GetPlugins(bool refresh, std::vector<WebPluginInfo>* plugins) {
+  LoadPlugins(refresh);
 
+  AutoLock lock(lock_);
   *plugins = plugins_;
-
-  return true;
 }
 
 bool PluginList::GetPluginInfo(const GURL& url,
@@ -322,6 +333,8 @@ bool PluginList::GetPluginInfo(const GURL& url,
 
 bool PluginList::GetPluginInfoByPath(const FilePath& plugin_path,
                                      WebPluginInfo* info) {
+  LoadPlugins(false);
+  AutoLock lock(lock_);
   for (size_t i = 0; i < plugins_.size(); ++i) {
     if (plugins_[i].path == plugin_path) {
       *info = plugins_[i];