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

content/browser/plugin_service.h

 // 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.
 
 // This class responds to requests from renderers for the list of plugins, and
 // also a proxy object for plugin instances.
 
 #ifndef CONTENT_BROWSER_PLUGIN_SERVICE_H_
 #define CONTENT_BROWSER_PLUGIN_SERVICE_H_
 #pragma once
 
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/callback.h"
 #include "base/memory/scoped_vector.h"
 #include "base/memory/singleton.h"
 #include "base/synchronization/waitable_event_watcher.h"
 #include "build/build_config.h"
 #include "content/browser/plugin_process_host.h"
 #include "content/browser/ppapi_plugin_process_host.h"
 #include "content/browser/ppapi_broker_process_host.h"
 #include "content/common/content_export.h"
 #include "content/common/notification_observer.h"
 #include "content/common/notification_registrar.h"
 #include "googleurl/src/gurl.h"
 #include "ipc/ipc_channel_handle.h"
+#include "webkit/plugins/npapi/plugin_group.h"

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

nit: can you get away with forward declaring?

 #include "webkit/plugins/webplugininfo.h"
 
 #if defined(OS_WIN)
 #include "base/memory/scoped_ptr.h"
 #include "base/win/registry.h"
 #endif
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX)
 #include "base/files/file_path_watcher.h"
 #endif
 
 struct PepperPluginInfo;
 class PluginDirWatcherDelegate;
 
+namespace base {
+class MessageLoopProxy;
+}
+
 namespace content {
 class ResourceContext;
 class PluginServiceFilter;
 }
 
+namespace webkit {
+namespace npapi {
+class MockPluginList;
+class PluginList;
+}
+}
+
 // This must be created on the main thread but it's only called on the IO/file
-// thread.
+// thread. This is an asynchronous wrapper around the PluginList interface for
+// querying plugin information, and this should instead of that class in the
+// content layer and above.

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

above->below? (see http://dev.chromium.org/developers/content-module)

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

I guess it depends if you're looking up or down ;). Got suggestions for better
wording?

[jam, 2011/09/21 17:20:15]

(big nit!) It's been my experience that we use down, for example that's why i
made the diagram in the wiki above that way. A rough check with a few others
here confirmed that that's our convention.

 class CONTENT_EXPORT PluginService
     : public base::WaitableEventWatcher::Delegate,
       public NotificationObserver {
  public:
   struct OverriddenPlugin {
     int render_process_id;
     int render_view_id;
     GURL url;  // If empty, the override applies to all urls in render_view.
     webkit::WebPluginInfo plugin;
   };
 
+  typedef base::Callback<void(std::vector<webkit::WebPluginInfo>)>

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

const ref (and below)

+      GetPluginsCallback;
+  typedef base::Callback<void(std::vector<webkit::npapi::PluginGroup>)>
+      GetPluginGroupsCallback;
+
   // Returns the PluginService singleton.
   static PluginService* GetInstance();
 
   // Starts watching for changes in the list of installed plug-ins.
   void StartWatchingPlugins();
 
   // Gets the browser's UI locale.
   const std::string& GetUILocale();
 
   // Returns the plugin process host corresponding to the plugin process that
@@ skipping 38 matching lines @@
                      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);
 
-  // Returns a list of all plug-ins available to the resource context. Must be
-  // called on the FILE thread.
+  // Marks the plugin list as dirty and will cause the plugins to be reloaded
+  // on the next access through GetPlugins() or GetPluginGroups().
+  void RefreshPluginList();
+
+  // Asynchronously loads plugins on the FILE thread and then calls back to the

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

nit: i think it's better not to mention file thread here, since on mac/win we'll
be going to a different process

+  // provided function on the calling MessageLoop on completion.
   void GetPlugins(const content::ResourceContext& context,
-                  std::vector<webkit::WebPluginInfo>* plugins);
+                  const GetPluginsCallback& callback);
+  // Variant of above, but does not perform filtering based on |context|.
+  void GetPlugins(const GetPluginsCallback& callback);
+
+  // Returns the list of cached plugin groups. This can be called from any
+  // thread, but note that the data can be stale.
+  void GetCachedPluginGroups(std::vector<webkit::npapi::PluginGroup>* groups);
+
+  // Gets the list of plugin groups, reloading them from disk if marked as dirty
+  // with RefreshPluginList().
+  void GetPluginGroups(const GetPluginGroupsCallback& callback);
 
   // Tells all the renderer processes to throw away their cache of the plugin
   // list, and optionally also reload all the pages with plugins.
   // NOTE: can only be called on the UI thread.
   static void PurgePluginListCache(bool reload_pages);
 
   void set_filter(content::PluginServiceFilter* filter) {
     filter_ = filter;
   }
 
+  // Sets the PluginList for testing environments. Be sure to change back to
+  // NULL at the end of the test!
+  void SetPluginListForTesting(webkit::npapi::MockPluginList* plugin_list);

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

nit: can you make this private and friend the test class, so we can be assured
that other code doesn't call it?

actually, since this is only used by the extension code, why did you move this
stuff here instead of keeping it where it was?

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

I personally hate friending tests in shared components. Since it takes a
MockPluginList, I think this is pretty safe. There's also a PRESUBMIT when you
check in code that calls a "ForTesting" method.
The extension code needs to "load" fake plugins, which is part of the PluginList
interface. Since the PluginService just uses the Singleton(), there needs to be
a way to inject. They previously did this just through the injector in their
code, but now that the extension code uses PluginService, the injector needed to
move here.

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

oh I didn't know about that presubmit check. I'm curious, why do you hate
friending tests?
so in the test, the extensions code calls PS. currently you call into PS from
the test to override the returned list. why not have the test call into the
extensions code to override the list, and if that's set, it doesn't even call
PS? it seems nicer if testing code is localized as much as possible to the code
that needs that override.

+
  private:
   friend struct DefaultSingletonTraits<PluginService>;
 
   // Creates the PluginService object, but doesn't actually build the plugin
   // list yet.  It's generated lazily.
   PluginService();
   virtual ~PluginService();
 
+  // Returns the PluginList, either the singleton or the override.
+  webkit::npapi::PluginList* GetPluginList();
+
   // base::WaitableEventWatcher::Delegate implementation.
   virtual void OnWaitableEventSignaled(base::WaitableEvent* waitable_event);
 
   // NotificationObserver implementation
   virtual void Observe(int type, const NotificationSource& source,
                        const NotificationDetails& details);
 
   void RegisterPepperPlugins();
 
   PepperPluginInfo* GetRegisteredPpapiPluginInfo(const FilePath& plugin_path);
 
+  // Function that is run on the FILE thread to load the plugins synchronously.
+  void GetPluginsInternal(const content::ResourceContext* context,
+                          base::MessageLoopProxy* target_loop,
+                          const GetPluginsCallback& callback);
+
   // Helper so we can do the plugin lookup on the FILE thread.
   void GetAllowedPluginForOpenChannelToPlugin(
       int render_process_id,
       int render_view_id,
       const GURL& url,
       const GURL& page_url,
       const std::string& mime_type,
       PluginProcessHost::Client* client,
       const content::ResourceContext* resource_context);
 
@@ skipping 31 matching lines @@
   scoped_refptr<PluginDirWatcherDelegate> file_watcher_delegate_;
 #endif
 
   std::vector<PepperPluginInfo> ppapi_plugins_;
 
   // Weak pointer; outlives us.
   content::PluginServiceFilter* filter_;
 
   std::set<PluginProcessHost::Client*> pending_plugin_clients_;
 
+  // Alternate plugin list to use for testing environments. When NULL, defaults
+  // to the singleton.
+  webkit::npapi::PluginList* plugin_list_;
+
   DISALLOW_COPY_AND_ASSIGN(PluginService);
 };
 
 DISABLE_RUNNABLE_METHOD_REFCOUNT(PluginService);
 
 #endif  // CONTENT_BROWSER_PLUGIN_SERVICE_H_