PluginList: Unit tests and bugfixes

webkit/glue/plugins/plugin_list.h

Index: webkit/glue/plugins/plugin_list.h
diff --git a/webkit/glue/plugins/plugin_list.h b/webkit/glue/plugins/plugin_list.h
index 890984fb72ac64b351479dcb29bedab4afc219ca..42a828cb7e8b0338e7805700ade85ea95d811399 100644
--- a/webkit/glue/plugins/plugin_list.h
+++ b/webkit/glue/plugins/plugin_list.h
@@ -28,6 +28,10 @@ struct DefaultLazyInstanceTraits;
 
 }  // namespace base
 
+namespace plugin_test_internal {
+class TestablePluginList;
+}
+
 namespace NPAPI {
 
 #define kDefaultPluginLibraryName FILE_PATH_LITERAL("default_plugin")
@@ -226,15 +230,17 @@ class PluginList {
 
   virtual ~PluginList();
 
- private:
-  FRIEND_TEST_ALL_PREFIXES(PluginGroupTest, PluginGroupDefinition);
-
-  // Constructors are private for singletons
+ protected:
+  // Constructors are private for singletons.

[jam, 2010/12/15 19:41:57]

nit: this comment doesn't make sense now

   PluginList();
 
-  // Creates PluginGroups for the static group definitions, and adds them to
-  // the PluginGroup cache of this PluginList.
-  void AddHardcodedPluginGroups();
+  // Load all plugins from the default plugins directory.
+  // Declared virtual so that it can be overridden for tests.
+  virtual void LoadPlugins(bool refresh);
+
+  // Allows tests to perform custom post-initialization tasks on newly created
+  // PluginGroups.
+  virtual void ProcessGroupAfterInitialize(PluginGroup* group) { }
 
   // Adds the given WebPluginInfo to its corresponding group, creating it if
   // necessary, and returns the group.
@@ -242,8 +248,21 @@ class PluginList {
   PluginGroup* AddToPluginGroups(const WebPluginInfo& web_plugin_info,
                                  std::vector<WebPluginInfo*>* plugins);
 
-  // Load all plugins from the default plugins directory
-  void LoadPlugins(bool refresh);
+  bool plugins_loaded_;
+
+  // Contains information about the available plugins.
+  std::vector<WebPluginInfo*> plugins_;

[jam, 2010/12/15 19:41:57]

i actually don't like moving member variables to be protected so that test
classes can derive from this object and access them.  this isn't a pattern that
i've seen in our tests, and breaks encapsulation since now non-test classes can
just derive from pluginlist and modify the private members.  you can just have
protected getters for these objects if you want to inspect them.

+
+  // Path names of plugins to disable (the default is to enable them all).
+  DisabledPlugins disabled_plugins_;
+
+  // Holds the currently available plugin groups.
+  PluginGroup::PluginMap plugin_groups_;
+
+ private:
+  // Creates PluginGroups for the static group definitions, and adds them to
+  // the PluginGroup cache of this PluginList.
+  void AddHardcodedPluginGroups();
 
   // Load all plugins from a specific directory.
   // |plugins| is updated with loaded plugin information.
@@ -308,14 +327,9 @@ class PluginList {
   // Internals
   //
 
-  bool plugins_loaded_;
-
   // If true, we reload plugins even if they've been loaded already.
   bool plugins_need_refresh_;
 
-  // Contains information about the available plugins.
-  std::vector<WebPluginInfo*> plugins_;
-
   // Extra plugin paths that we want to search when loading.
   std::vector<FilePath> extra_plugin_paths_;
 
@@ -328,8 +342,6 @@ class PluginList {
   // An element of the disabled list saves the reason for being disabled in the
   // second element of the pair.
   typedef std::pair<FilePath, PluginDisabledReason> DisabledPluginsListElement;
-  // Path names of plugins to disable (the default is to enable them all).
-  DisabledPlugins disabled_plugins_;
 
   typedef std::pair<string16, PluginDisabledReason> DisabledGroupsListElement;
   // Group names to disable (the default is to enable them all).
@@ -337,9 +349,6 @@ class PluginList {
 
   bool disable_outdated_plugins_;
 
-  // Holds the currently available plugin groups.
-  PluginGroup::PluginMap plugin_groups_;
-
   int next_priority_;
 
   // Need synchronization for the above members since this object can be