Convert the PluginService interface to be an async wrapper around PluginList.

content/browser/plugin_service.cc

 // Copyright (c) 2011 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 "content/browser/plugin_service.h"
 
+#include "base/bind.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/file_path.h"
+#include "base/message_loop.h"
+#include "base/message_loop_proxy.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/thread.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/content_browser_client.h"
 #include "content/browser/plugin_service_filter.h"
 #include "content/browser/ppapi_plugin_process_host.h"
 #include "content/browser/renderer_host/render_process_host.h"
 #include "content/browser/renderer_host/render_view_host.h"
 #include "content/browser/resource_context.h"
 #include "content/common/content_notification_types.h"
 #include "content/common/content_switches.h"
 #include "content/common/notification_service.h"
 #include "content/common/pepper_plugin_registry.h"
 #include "content/common/plugin_messages.h"
 #include "content/common/view_messages.h"
+#include "webkit/plugins/npapi/mock_plugin_list.h"

[Bernhard Bauer, 2011/09/21 14:31:26]

Nit: Should PluginService really know about testing-specific code? You could
just as well pass in a regular PluginList to SetPluginListForTesting.

 #include "webkit/plugins/npapi/plugin_constants_win.h"
 #include "webkit/plugins/npapi/plugin_list.h"
 #include "webkit/plugins/webplugininfo.h"
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX)
 using ::base::files::FilePathWatcher;
 #endif
 
 using content::PluginServiceFilter;
 
+namespace {
+
+// Helper function that merely runs the callback with the result. Called on the
+// thread on which the original GetPlugins() call was made.
+static void RunGetPluginsCallback(
+    const PluginService::GetPluginsCallback& callback,
+    std::vector<webkit::WebPluginInfo> result) {
+  callback.Run(result);
+}
+
+// A callback for GetPlugins() that then gets the freshly loaded plugin groups
+// and runs the callback for GetPluginGroups().
+static void GetPluginsForGroupsCallback(
+    const PluginService::GetPluginGroupsCallback& callback,
+    std::vector<webkit::WebPluginInfo> /* unused */) {

[jam, 2011/09/21 00:04:51]

nit: const ref.

also, "unused" isn't a convention in any of the plugin classes for unused
parameters. i think it's clear to the reader that it's unused, so you can just
put infos

actually, why pass in webplugininfo if it's not used at all? and even, why not
just give it the PluginGroup objects as a parameter instead of calling PS?

[Robert Sesek, 2011/09/21 01:13:44]

As stated in the comments, this is the callback for the call to GetPlugins().
This is done so that the plugin cache is fresh and reloaded if necessary, after
which the plugin groups can be fetched directly from the cache.

+  std::vector<webkit::npapi::PluginGroup> groups;
+  PluginService::GetInstance()->GetCachedPluginGroups(&groups);
+  callback.Run(groups);
+}
+
+}  // namespace
+
 #if defined(OS_MACOSX)
 static void NotifyPluginsOfActivation() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   for (BrowserChildProcessHost::Iterator iter(ChildProcessInfo::PLUGIN_PROCESS);
        !iter.Done(); ++iter) {
     PluginProcessHost* plugin = static_cast<PluginProcessHost*>(*iter);
     plugin->OnAppActivation();
   }
 }
@@ skipping 14 matching lines @@
 #endif
 
 // static
 PluginService* PluginService::GetInstance() {
   return Singleton<PluginService>::get();
 }
 
 PluginService::PluginService()
     : ui_locale_(
           content::GetContentClient()->browser()->GetApplicationLocale()),
-      filter_(NULL) {
+      filter_(NULL),
+      plugin_list_(NULL) {
   RegisterPepperPlugins();
 
   // Load any specified on the command line as well.
   const CommandLine* command_line = CommandLine::ForCurrentProcess();
   FilePath path = command_line->GetSwitchValuePath(switches::kLoadPlugin);
   if (!path.empty())
     webkit::npapi::PluginList::Singleton()->AddExtraPluginPath(path);
   path = command_line->GetSwitchValuePath(switches::kExtraPluginDir);
   if (!path.empty())
     webkit::npapi::PluginList::Singleton()->AddExtraPluginDir(path);
@@ skipping 43 matching lines @@
   }
 #elif defined(OS_POSIX) && !defined(OS_MACOSX)
 // The FilePathWatcher produces too many false positives on MacOS (access time
 // updates?) which will lead to enforcing updates of the plugins way too often.
 // On ChromeOS the user can't install plugins anyway and on Windows all
 // important plugins register themselves in the registry so no need to do that.
   file_watcher_delegate_ = new PluginDirWatcherDelegate();
   // Get the list of all paths for registering the FilePathWatchers
   // that will track and if needed reload the list of plugins on runtime.
   std::vector<FilePath> plugin_dirs;
-  webkit::npapi::PluginList::Singleton()->GetPluginDirectories(
-      &plugin_dirs);
+  GetPluginList()->GetPluginDirectories(&plugin_dirs);
 
   for (size_t i = 0; i < plugin_dirs.size(); ++i) {
     // FilePathWatcher can not handle non-absolute paths under windows.
     // We don't watch for file changes in windows now but if this should ever
     // be extended to Windows these lines might save some time of debugging.
 #if defined(OS_WIN)
     if (!plugin_dirs[i].IsAbsolute())
       continue;
 #endif
     FilePathWatcher* watcher = new FilePathWatcher();
@@ skipping 60 matching lines @@
 
 PluginProcessHost* PluginService::FindOrStartNpapiPluginProcess(
     const FilePath& plugin_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   PluginProcessHost* plugin_host = FindNpapiPluginProcess(plugin_path);
   if (plugin_host)
     return plugin_host;
 
   webkit::WebPluginInfo info;
-  if (!webkit::npapi::PluginList::Singleton()->GetPluginInfoByPath(
-          plugin_path, &info)) {
+  if (!GetPluginList()->GetPluginInfoByPath(plugin_path, &info)) {
     return NULL;
   }
 
   // This plugin isn't loaded by any plugin process, so create a new process.
   scoped_ptr<PluginProcessHost> new_host(new PluginProcessHost());
   if (!new_host->Init(info, ui_locale_)) {
     NOTREACHED();  // Init is not expected to fail.
     return NULL;
   }
   return new_host.release();
@@ skipping 146 matching lines @@
 bool PluginService::GetPluginInfo(int render_process_id,
                                   int render_view_id,
                                   const content::ResourceContext& context,
                                   const GURL& url,
                                   const GURL& page_url,
                                   const std::string& mime_type,
                                   bool allow_wildcard,
                                   bool* use_stale,
                                   webkit::WebPluginInfo* info,
                                   std::string* actual_mime_type) {
-  webkit::npapi::PluginList* plugin_list =
-      webkit::npapi::PluginList::Singleton();
+  webkit::npapi::PluginList* plugin_list = GetPluginList();
   // GetPluginInfoArray may need to load the plugins, so we need to be
   // on the FILE thread.
   DCHECK(use_stale || BrowserThread::CurrentlyOn(BrowserThread::FILE));
   std::vector<webkit::WebPluginInfo> plugins;
   std::vector<std::string> mime_types;
   plugin_list->GetPluginInfoArray(
       url, mime_type, allow_wildcard, use_stale, &plugins, &mime_types);
   if (plugins.size() > 1 &&
       plugins.back().path ==
           FilePath(webkit::npapi::kDefaultPluginLibraryName)) {
@@ skipping 11 matching lines @@
                                              &plugins[i])) {
       *info = plugins[i];
       if (actual_mime_type)
         *actual_mime_type = mime_types[i];
       return true;
     }
   }
   return false;
 }
 
-void PluginService::GetPlugins(
-    const content::ResourceContext& context,
-    std::vector<webkit::WebPluginInfo>* plugins) {
-  // GetPlugins may need to load the plugins, so we need to be
-  // on the FILE thread.
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  webkit::npapi::PluginList* plugin_list =
-      webkit::npapi::PluginList::Singleton();
+void PluginService::RefreshPluginList() {
+  GetPluginList()->RefreshPlugins();
+}
+
+void PluginService::GetPlugins(const content::ResourceContext& context,
+    const PluginService::GetPluginsCallback& callback) {
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&PluginService::GetPluginsInternal, base::Unretained(this),
+          base::Unretained(&context),

[jam, 2011/09/21 00:04:51]

how do you know that the ResourceContext will still be around when we go to the
file thread? it seems that this can cause crashes when profiles go away

[Robert Sesek, 2011/09/21 01:13:44]

I was concerned about this too, but was wondering the best approach. Should I
make the ResourceContext extend from SupportsWeakPtr<T>?

[jam, 2011/09/21 06:31:46]

You'd have to check with Will Chan about this. It seems like a workaround
because one is unsure about lifetime. also, weakptr is not threadsafe to use on
threads other than the one it was created on.

it looks like there's only one caller of this function (RMF), so perhaps the
best approach for now is to have only one GetPlugins(callback) call, and have
RMF do its filtering when it gets the result.

[Bernhard Bauer, 2011/09/21 14:31:26]

Yes please! Then we can do the filtering on the IO thread and get rid of the
resource context map in ChromePluginServiceFilter.

+          MessageLoop::current()->message_loop_proxy(),
+          callback));
+}
+
+void PluginService::GetPlugins(const GetPluginsCallback& callback) {
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&PluginService::GetPluginsInternal, base::Unretained(this),
+          base::Unretained(static_cast<content::ResourceContext*>(NULL)),
+          MessageLoop::current()->message_loop_proxy(),
+          callback));
+}
+
+void PluginService::GetCachedPluginGroups(
+    std::vector<webkit::npapi::PluginGroup>* groups) {
+  GetPluginList()->GetPluginGroups(false, groups);
+}
+
+void PluginService::GetPluginGroups(const GetPluginGroupsCallback& callback) {
+  GetPlugins(base::Bind(&GetPluginsForGroupsCallback, callback));
+}
+
+void PluginService::GetPluginsInternal(const content::ResourceContext* context,
+     base::MessageLoopProxy* target_loop,
+     const PluginService::GetPluginsCallback& callback) {

[jam, 2011/09/21 00:04:51]

probably want to copy the
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); from line 425

[Robert Sesek, 2011/09/21 01:13:44]

No, this will be changing shortly anyways.

[jam, 2011/09/21 06:31:46]

On Windows this will still be in-process, so we still want the assert there to
ensure people don't do this heavy disk operation on UI/IO

+  // Load all the plugins synchronously.
+  webkit::npapi::PluginList* plugin_list = GetPluginList();
   std::vector<webkit::WebPluginInfo> all_plugins;
   plugin_list->GetPlugins(&all_plugins);
 
-  int child_process_id = -1;
-  int routing_id = MSG_ROUTING_NONE;
-  for (size_t i = 0; i < all_plugins.size(); ++i) {
-    if (!filter_ || filter_->ShouldUsePlugin(child_process_id,
-                                             routing_id,
-                                             &context,
-                                             GURL(),
-                                             GURL(),
-                                             &all_plugins[i])) {
-      plugins->push_back(all_plugins[i]);
+  if (context) {
+    // If there is a context, filter plugins.
+    std::vector<webkit::WebPluginInfo> plugins;
+
+    int child_process_id = -1;
+    int routing_id = MSG_ROUTING_NONE;
+    for (size_t i = 0; i < all_plugins.size(); ++i) {
+      if (!filter_ || filter_->ShouldUsePlugin(child_process_id,
+                                               routing_id,
+                                               context,
+                                               GURL(),
+                                               GURL(),
+                                               &all_plugins[i])) {
+        plugins.push_back(all_plugins[i]);
+      }
     }
+
+    target_loop->PostTask(FROM_HERE,
+        base::Bind(&RunGetPluginsCallback, callback, plugins));
+  } else {
+    // Otherwise, return all plugins.
+    target_loop->PostTask(FROM_HERE,
+        base::Bind(&RunGetPluginsCallback, callback, all_plugins));
   }
 }
 
+void PluginService::SetPluginListForTesting(
+    webkit::npapi::MockPluginList* plugin_list) {
+  plugin_list_ = plugin_list;
+}
+
+webkit::npapi::PluginList* PluginService::GetPluginList() {
+  if (plugin_list_)
+    return plugin_list_;
+  return webkit::npapi::PluginList::Singleton();
+}
+
 void PluginService::OnWaitableEventSignaled(
     base::WaitableEvent* waitable_event) {
 #if defined(OS_WIN)
   if (waitable_event == hkcu_event_.get()) {
     hkcu_key_.StartWatching();
   } else {
     hklm_key_.StartWatching();
   }
 
-  webkit::npapi::PluginList::Singleton()->RefreshPlugins();
+  GetPluginList()->RefreshPlugins();
   PurgePluginListCache(true);
 #else
   // This event should only get signaled on a Windows machine.
   NOTREACHED();
 #endif  // defined(OS_WIN)
 }
 
 void PluginService::Observe(int type,
                             const NotificationSource& source,
                             const NotificationDetails& details) {
@@ skipping 11 matching lines @@
   for (RenderProcessHost::iterator it = RenderProcessHost::AllHostsIterator();
        !it.IsAtEnd(); it.Advance()) {
     it.GetCurrentValue()->Send(new ViewMsg_PurgePluginListCache(reload_pages));
   }
 }
 
 void PluginService::RegisterPepperPlugins() {
   // TODO(abarth): It seems like the PepperPluginRegistry should do this work.
   PepperPluginRegistry::ComputeList(&ppapi_plugins_);
   for (size_t i = 0; i < ppapi_plugins_.size(); ++i) {
-    webkit::npapi::PluginList::Singleton()->RegisterInternalPlugin(
+    GetPluginList()->RegisterInternalPlugin(
         ppapi_plugins_[i].ToWebPluginInfo());
   }
 }
 
 // There should generally be very few plugins so a brute-force search is fine.
 PepperPluginInfo* PluginService::GetRegisteredPpapiPluginInfo(
     const FilePath& plugin_path) {
   PepperPluginInfo* info = NULL;
   for (size_t i = 0; i < ppapi_plugins_.size(); i++) {
     if (ppapi_plugins_[i].path == plugin_path) {
       info = &ppapi_plugins_[i];
       break;
     }
   }
   if (info)
     return info;
   // We did not find the plugin in our list. But wait! the plugin can also
   // be a latecomer, as it happens with pepper flash. This information
   // can be obtained from the PluginList singleton and we can use it to
   // construct it and add it to the list. This same deal needs to be done
   // in the renderer side in PepperPluginRegistry.
   webkit::WebPluginInfo webplugin_info;
-  if (!webkit::npapi::PluginList::Singleton()->GetPluginInfoByPath(
-      plugin_path, &webplugin_info))
+  if (!GetPluginList()->GetPluginInfoByPath(plugin_path, &webplugin_info))
     return NULL;
   PepperPluginInfo new_pepper_info;
   if (!MakePepperPluginInfo(webplugin_info, &new_pepper_info))
     return NULL;
   ppapi_plugins_.push_back(new_pepper_info);
   return &ppapi_plugins_[ppapi_plugins_.size() - 1];
 }
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX)
 // static
 void PluginService::RegisterFilePathWatcher(
     FilePathWatcher *watcher,
     const FilePath& path,
     FilePathWatcher::Delegate* delegate) {
   bool result = watcher->Watch(path, delegate);
   DCHECK(result);
 }
 #endif