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 8f11a5e0e25ca3d4551ed223ec9ab707390fdfd1..59d651fc814431d25566bc50a2636dd71ef1fc5f 100644
--- a/webkit/glue/plugins/plugin_list.h
+++ b/webkit/glue/plugins/plugin_list.h
@@ -5,8 +5,10 @@
 #ifndef WEBKIT_GLUE_PLUGINS_PLUGIN_LIST_H_
 #define WEBKIT_GLUE_PLUGINS_PLUGIN_LIST_H_
 
+#include <map>
 #include <set>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "base/basictypes.h"
@@ -26,6 +28,10 @@ struct DefaultLazyInstanceTraits;
 
 }  // namespace base
 
+namespace plugin_test_internal {
+class TestablePluginList;
+}
+
 namespace NPAPI {
 
 #define kDefaultPluginLibraryName FILE_PATH_LITERAL("default_plugin")
@@ -221,10 +227,10 @@ class PluginList {
 
   const DisabledGroups& GetDisabledGroups() const;
 
-  ~PluginList();
+  virtual ~PluginList();
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(PluginGroupTest, PluginGroupDefinition);
+  friend class plugin_test_internal::TestablePluginList;

[Bernhard Bauer, 2010/12/14 20:18:34]

Couldn't you make the necessary methods protected?

[Jakob Kummerow, 2010/12/15 18:03:27]

Done.
I have to say, though, that I find it ugly to split the non-public
methods/members into separate protected/private sections based solely on the
needs of the test class. I'm happy to revert this if you agree that having the
test class as a friend is cleaner.

[jam, 2010/12/15 19:43:39]

I agree that it's not as nice.  We shouldn't break encapsulation just for tests,
since those newly exposed members can now in the future be misused in non-test
code.  Having the test class be a friend ensures that the members aren't touched
by non-test code.

 
   // Constructors are private for singletons
   PluginList();
@@ -238,8 +244,13 @@ class PluginList {
   // Callers need to protect calls to this method by a lock themselves.
   PluginGroup* AddToPluginGroups(const WebPluginInfo& web_plugin_info);
 
-  // Load all plugins from the default plugins directory
-  void LoadPlugins(bool refresh);
+  // 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 PostInitPluginGroup(PluginGroup* group) { }

[Bernhard Bauer, 2010/12/14 20:18:34]

Nit: Maybe name it DidInitPluginGroup?

[danno, 2010/12/15 10:42:12]

How about ProcessGroupAfterInitialize
On 2010/12/14 20:18:34, Bernhard Bauer wrote:

[Jakob Kummerow, 2010/12/15 18:03:27]

Done.

 
   // Load all plugins from a specific directory.
   // |plugins| is updated with loaded plugin information.