Relocate plugin list fetching to PluginService

We fetch the plugin list from three places.  Previously, each location had
custom code to proxy the query to the file thread.  This change moves
the query to the PluginService, which then internally manages posting to
the file thread and calling back.

I experimented with some different approaches to handling the lifetimes
of the requests and responses.  The approach now is simple:
- The PluginService plugin methods are called and respond on the IO
  thread.  Two of the three consumers of the plugin lists are already on
  the IO thread, so they don't need any complicated thread handling.
- None of the callers ever need to cancel their requests: one is a
  singleton, and the other two always wait (in terms of holding a ref
  on the object) for the requests to complete.  This makes lifetime
  management a lot simpler than it would otherwise be.

With this change in place, I can then look at refactoring the PluginService
implementation on Linux to do more complicated plugin loading as needed in
bug 17863.

BUG=17863

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

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

8 files changed, 138 insertions, 71 deletions

diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc
index 5839171..82cdeac 100644
--- a/chrome/browser/metrics/metrics_service.cc
+++ b/chrome/browser/metrics/metrics_service.cc
@@ -174,6 +174,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/load_notification_details.h"
 #include "chrome/browser/memory_details.h"
+#include "chrome/browser/plugin_service.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "chrome/browser/search_engines/template_url_model.h"
@@ -284,35 +285,39 @@ class MetricsMemoryDetails : public MemoryDetails {
   DISALLOW_EVIL_CONSTRUCTORS(MetricsMemoryDetails);
 };
 
-class MetricsService::GetPluginListTaskComplete : public Task {
+// This object and its static functions manage the calls to and from
+// the plugin service.  It lives on the IO thread.  It only lives long
+// enough to handle the callback, then it deletes itself.
+class GetPluginListClient : public PluginService::GetPluginListClient {
  public:
-  explicit GetPluginListTaskComplete(
-      const std::vector<WebPluginInfo>& plugins) : plugins_(plugins) { }
-  virtual void Run() {
-    g_browser_process->metrics_service()->OnGetPluginListTaskComplete(plugins_);
+  // Call GetPluginList on a GetPluginListClient.  Used to proxy this call
+  // from the UI thread to the IO thread.
+  static void CallGetPluginList(GetPluginListClient* client) {
+    DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
+    PluginService::GetInstance()->GetPluginList(false, client);
   }
 
- private:
-  std::vector<WebPluginInfo> plugins_;
-};
-
-class MetricsService::GetPluginListTask : public Task {
- public:
-  explicit GetPluginListTask(MessageLoop* callback_loop)
-      : callback_loop_(callback_loop) {}
-
-  virtual void Run() {
-    std::vector<WebPluginInfo> plugins;
-    NPAPI::PluginList::Singleton()->GetPlugins(false, &plugins);
-
-    callback_loop_->PostTask(
-        FROM_HERE, new GetPluginListTaskComplete(plugins));
+  // Callback from the PluginService when the plugin list has been retrieved.
+  virtual void OnGetPluginList(const std::vector<WebPluginInfo>& plugins) {
+    DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
+    // Forward back to the UI thread.
+    ChromeThread::PostTask(
+        ChromeThread::UI, FROM_HERE,
+        NewRunnableFunction(&GetPluginListClient::GetPluginListComplete,
+                            plugins));
+    delete this;
   }
 
- private:
-  MessageLoop* callback_loop_;
+  // A function to proxy the plugin list back from the IO thread to the UI
+  // thread to call the metrics service.
+  // |plugins| is intentionally pass by value.
+  static void GetPluginListComplete(const std::vector<WebPluginInfo> plugins) {
+    DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+    g_browser_process->metrics_service()->OnGetPluginListComplete(plugins);
+  }
 };
 
+
 // static
 void MetricsService::RegisterPrefs(PrefService* local_state) {
   DCHECK(IsSingleThreaded());
@@ -745,7 +750,7 @@ void MetricsService::InitializeMetricsState() {
   ScheduleNextStateSave();
 }
 
-void MetricsService::OnGetPluginListTaskComplete(
+void MetricsService::OnGetPluginListComplete(
     const std::vector<WebPluginInfo>& plugins) {
   DCHECK(state_ == PLUGIN_LIST_REQUESTED);
   plugins_ = plugins;
@@ -826,8 +831,10 @@ void MetricsService::StartRecording() {
 
     // Make sure the plugin list is loaded before the inital log is sent, so
     // that the main thread isn't blocked generating the list.
-    g_browser_process->file_thread()->message_loop()->PostDelayedTask(FROM_HERE,
-        new GetPluginListTask(MessageLoop::current()),
+    ChromeThread::PostDelayedTask(
+        ChromeThread::IO, FROM_HERE,
+        NewRunnableFunction(&GetPluginListClient::CallGetPluginList,
+                            new GetPluginListClient),
         kInitialInterlogDuration * 1000 / 2);
   }
 }
diff --git a/chrome/browser/metrics/metrics_service.h b/chrome/browser/metrics/metrics_service.h
index 2386827..b66d36e 100644
--- a/chrome/browser/metrics/metrics_service.h
+++ b/chrome/browser/metrics/metrics_service.h
@@ -118,7 +118,7 @@ class MetricsService : public NotificationObserver,
   void RecordBreakpadHasDebugger(bool has_debugger);
 
   // Callback to let us knew that the plugin list is warmed up.
-  void OnGetPluginListTaskComplete(const std::vector<WebPluginInfo>& plugins);
+  void OnGetPluginListComplete(const std::vector<WebPluginInfo>& plugins);
 
   // Save any unsent logs into a persistent store in a pref.  We always do this
   // at shutdown, but we can do it as we reduce the list as well.
@@ -146,9 +146,6 @@ class MetricsService : public NotificationObserver,
   // Maintain a map of histogram names to the sample stats we've sent.
   typedef std::map<std::string, Histogram::SampleSet> LoggedSampleMap;
 
-  class GetPluginListTask;
-  class GetPluginListTaskComplete;
-
   // When we start a new version of Chromium (different from our last run), we
   // need to discard the old crash stats so that we don't attribute crashes etc.
   // in the old version to the current version (via current logs).
diff --git a/chrome/browser/plugin_service.cc b/chrome/browser/plugin_service.cc
index ba5821e..578e23e 100644
--- a/chrome/browser/plugin_service.cc
+++ b/chrome/browser/plugin_service.cc
@@ -126,6 +126,44 @@ const FilePath& PluginService::GetChromePluginDataDir() {
   return chrome_plugin_data_dir_;
 }
 
+// Call the GetPluginListClient back with a list of plugings.
+// |plugins| is intentionally pass-by-value so it's copied from the File thread
+// back to the IO thread.
+static void CallGetPluginListClient(PluginService::GetPluginListClient* client,
+                                    const std::vector<WebPluginInfo> plugins) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
+
+  client->OnGetPluginList(plugins);
+}
+
+// Go out to disk (well, NPAPI::PluginList) to get the plugin list.
+// Called on the file thread.
+static void GetPluginListOnFileThread(
+    bool refresh,
+    PluginService::GetPluginListClient* client) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+
+  std::vector<WebPluginInfo> plugins;
+  NPAPI::PluginList::Singleton()->GetPlugins(refresh, &plugins);
+
+  // Call the client back on the IO thread.
+  ChromeThread::PostTask(
+      ChromeThread::IO, FROM_HERE,
+      NewRunnableFunction(&CallGetPluginListClient,
+                          client,
+                          plugins));
+}
+
+void PluginService::GetPluginList(bool refresh,
+                                  GetPluginListClient* client) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
+
+  // Forward the request on to the file thread.
+  ChromeThread::PostTask(
+      ChromeThread::FILE, FROM_HERE,
+      NewRunnableFunction(&GetPluginListOnFileThread, refresh, client));
+}
+
 const std::wstring& PluginService::GetUILocale() {
   return ui_locale_;
 }
diff --git a/chrome/browser/plugin_service.h b/chrome/browser/plugin_service.h
index 7ab39e2..fba9b04 100644
--- a/chrome/browser/plugin_service.h
+++ b/chrome/browser/plugin_service.h
@@ -53,6 +53,19 @@ class PluginService
   void SetChromePluginDataDir(const FilePath& data_dir);
   const FilePath& GetChromePluginDataDir();
 
+  // An interface for GetPluginList callbacks.  Note that this is
+  // called directly by the PluginService on the IO thread, so the
+  // implementor of this interface must outlive the request.
+  class GetPluginListClient {
+   public:
+    // Called when the request completes.
+    virtual void OnGetPluginList(const std::vector<WebPluginInfo>& plugins) = 0;
+  };
+
+  // Start a query for the list of plugins.
+  // If |refresh| is true, refreshes the list.
+  void GetPluginList(bool refresh, GetPluginListClient* client);
+
   // Gets the browser's UI locale.
   const std::wstring& GetUILocale();
 
diff --git a/chrome/browser/renderer_host/buffered_resource_handler.cc b/chrome/browser/renderer_host/buffered_resource_handler.cc
index f3ca82f..a72c54d 100644
--- a/chrome/browser/renderer_host/buffered_resource_handler.cc
+++ b/chrome/browser/renderer_host/buffered_resource_handler.cc
@@ -10,6 +10,7 @@
 #include "base/logging.h"
 #include "base/string_util.h"
 #include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/plugin_service.h"
 #include "chrome/browser/renderer_host/download_throttling_resource_handler.h"
 #include "chrome/browser/renderer_host/resource_dispatcher_host.h"
 #include "chrome/browser/renderer_host/resource_dispatcher_host_request_info.h"
@@ -395,10 +396,10 @@ bool BufferedResourceHandler::ShouldWaitForPlugins() {
       ResourceDispatcherHost::InfoForRequest(request_);
   host_->PauseRequest(info->child_id(), info->request_id(), true);
 
-  // Schedule plugin loading on the file thread.
-  ChromeThread::PostTask(
-      ChromeThread::FILE, FROM_HERE,
-      NewRunnableMethod(this, &BufferedResourceHandler::LoadPlugins));
+  // Schedule plugin loading.
+  this->AddRef();  // Balanced in OnGetPluginList.
+  PluginService::GetInstance()->GetPluginList(false, this);
+
   return true;
 }
 
@@ -466,23 +467,18 @@ bool BufferedResourceHandler::ShouldDownload(bool* need_plugin_list) {
       GURL(), type, allow_wildcard, &info, NULL);
 }
 
-void BufferedResourceHandler::LoadPlugins() {
-  std::vector<WebPluginInfo> plugins;
-  NPAPI::PluginList::Singleton()->GetPlugins(false, &plugins);
-
-  ChromeThread::PostTask(
-      ChromeThread::IO, FROM_HERE,
-      NewRunnableMethod(this, &BufferedResourceHandler::OnPluginsLoaded));
-}
-
-void BufferedResourceHandler::OnPluginsLoaded() {
+void BufferedResourceHandler::OnGetPluginList(
+    const std::vector<WebPluginInfo>& plugins) {
   wait_for_plugins_ = false;
-  if (!request_)
-    return;
 
-  ResourceDispatcherHostRequestInfo* info =
-      ResourceDispatcherHost::InfoForRequest(request_);
-  host_->PauseRequest(info->child_id(), info->request_id(), false);
-  if (!CompleteResponseStarted(info->request_id(), false))
-    host_->CancelRequest(info->child_id(), info->request_id(), false);
+  if (request_) {
+    ResourceDispatcherHostRequestInfo* info =
+        ResourceDispatcherHost::InfoForRequest(request_);
+    host_->PauseRequest(info->child_id(), info->request_id(), false);
+    if (!CompleteResponseStarted(info->request_id(), false))
+      host_->CancelRequest(info->child_id(), info->request_id(), false);
+  }
+
+  // Drop the reference added before the GetPluginList call.
+  this->Release();
 }
diff --git a/chrome/browser/renderer_host/buffered_resource_handler.h b/chrome/browser/renderer_host/buffered_resource_handler.h
index 6bc34e1..3bed83a7 100644
--- a/chrome/browser/renderer_host/buffered_resource_handler.h
+++ b/chrome/browser/renderer_host/buffered_resource_handler.h
@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "chrome/browser/plugin_service.h"
 #include "chrome/browser/renderer_host/resource_handler.h"
 
 class MessageLoop;
@@ -14,7 +15,8 @@ class ResourceDispatcherHost;
 class URLRequest;
 
 // Used to buffer a request until enough data has been received.
-class BufferedResourceHandler : public ResourceHandler {
+class BufferedResourceHandler : public ResourceHandler,
+                                public PluginService::GetPluginListClient {
  public:
   BufferedResourceHandler(ResourceHandler* handler,
                           ResourceDispatcherHost* host,
@@ -63,11 +65,8 @@ class BufferedResourceHandler : public ResourceHandler {
   // loaded.
   bool ShouldDownload(bool* need_plugin_list);
 
-  // Called on the file thread to load the list of plugins.
-  void LoadPlugins();
-
   // Called on the IO thread once the list of plugins has been loaded.
-  void OnPluginsLoaded();
+  void OnGetPluginList(const std::vector<WebPluginInfo>& plugins);
 
   scoped_refptr<ResourceHandler> real_handler_;
   scoped_refptr<ResourceResponse> response_;
diff --git a/chrome/browser/renderer_host/resource_message_filter.cc b/chrome/browser/renderer_host/resource_message_filter.cc
index 8006960..afc5007 100644
--- a/chrome/browser/renderer_host/resource_message_filter.cc
+++ b/chrome/browser/renderer_host/resource_message_filter.cc
@@ -582,22 +582,32 @@ void ResourceMessageFilter::OnLoadFont(LOGFONT font) {
 
 void ResourceMessageFilter::OnGetPlugins(bool refresh,
                                          IPC::Message* reply_msg) {
-  ChromeThread::PostTask(
-      ChromeThread::FILE, FROM_HERE,
-      NewRunnableMethod(
-          this, &ResourceMessageFilter::OnGetPluginsOnFileThread, refresh,
-          reply_msg));
+  // Start the query to the plugin service, but only if we don't have another
+  // query already processing.
+  if (pending_getpluginlist_.empty()) {
+    PluginService::GetInstance()->GetPluginList(refresh, this);
+    this->AddRef();
+  }
+
+  // Put reply_msg into the queue of waiting responses.
+  pending_getpluginlist_.push(std::make_pair(refresh, reply_msg));
 }
 
-void ResourceMessageFilter::OnGetPluginsOnFileThread(
-    bool refresh, IPC::Message* reply_msg) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
-  std::vector<WebPluginInfo> plugins;
-  NPAPI::PluginList::Singleton()->GetPlugins(refresh, &plugins);
-  ViewHostMsg_GetPlugins::WriteReplyParams(reply_msg, plugins);
-  ChromeThread::PostTask(
-      ChromeThread::IO, FROM_HERE,
-      NewRunnableMethod(this, &ResourceMessageFilter::Send, reply_msg));
+void ResourceMessageFilter::OnGetPluginList(
+    const std::vector<WebPluginInfo>& plugins) {
+  IPC::Message* msg = pending_getpluginlist_.front().second;
+  ViewHostMsg_GetPlugins::WriteReplyParams(msg, plugins);
+  Send(msg);
+
+  pending_getpluginlist_.pop();
+  if (!pending_getpluginlist_.empty()) {
+    // We have more pending requests; start the next one.
+    PluginService::GetInstance()->GetPluginList(
+        pending_getpluginlist_.front().first, this);
+  } else {
+    // Drop the reference grabbed in OnGetPlugins().
+    this->Release();
+  }
 }
 
 void ResourceMessageFilter::OnGetPluginPath(const GURL& url,
diff --git a/chrome/browser/renderer_host/resource_message_filter.h b/chrome/browser/renderer_host/resource_message_filter.h
index 53c78dd..974abfc 100644
--- a/chrome/browser/renderer_host/resource_message_filter.h
+++ b/chrome/browser/renderer_host/resource_message_filter.h
@@ -24,6 +24,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/net/resolve_proxy_msg_helper.h"
 #include "chrome/browser/renderer_host/resource_dispatcher_host.h"
+#include "chrome/browser/plugin_service.h"
 #include "chrome/common/nacl_types.h"
 #include "chrome/common/notification_registrar.h"
 #include "chrome/common/transport_dib.h"
@@ -73,7 +74,8 @@ struct ViewHostMsg_DidPrintPage_Params;
 class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter,
                               public ResourceDispatcherHost::Receiver,
                               public NotificationObserver,
-                              public ResolveProxyMsgHelper::Delegate {
+                              public ResolveProxyMsgHelper::Delegate,
+                              public PluginService::GetPluginListClient {
  public:
   // Create the filter.
   // Note:  because the lifecycle of the ResourceMessageFilter is not
@@ -152,7 +154,7 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter,
   void OnGetScreenInfo(gfx::NativeViewId window, IPC::Message* reply);
 #endif
   void OnGetPlugins(bool refresh, IPC::Message* reply_msg);
-  void OnGetPluginsOnFileThread(bool refresh, IPC::Message* reply_msg);
+  virtual void OnGetPluginList(const std::vector<WebPluginInfo>& plugins);
   void OnGetPluginPath(const GURL& url,
                        const GURL& policy_url,
                        const std::string& mime_type,
@@ -398,6 +400,11 @@ class ResourceMessageFilter : public IPC::ChannelProxy::MessageFilter,
   // A callback to create a routing id for the associated renderer process.
   scoped_ptr<CallbackWithReturnValue<int>::Type> next_route_id_callback_;
 
+  // A queue of pending GetPluginList calls.  The bool is the |refresh|
+  // flag used to make the call, and the IPC::Message is the destination
+  // of the result of the call once the call completes.
+  std::queue<std::pair<bool, IPC::Message*> > pending_getpluginlist_;
+
   DISALLOW_COPY_AND_ASSIGN(ResourceMessageFilter);
 };