PluginList: Unit tests and bugfixes

webkit/glue/plugins/plugin_list.h

 // Copyright (c) 2010 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.
 
 #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"
 #include "base/file_path.h"
 #include "base/linked_ptr.h"
 #include "base/lock.h"
 #include "third_party/npapi/bindings/nphostapi.h"
 #include "webkit/glue/plugins/plugin_group.h"
 #include "webkit/glue/plugins/webplugininfo.h"
 
 class GURL;
 
 namespace base {
 
 template <typename T>
 struct DefaultLazyInstanceTraits;
 
 }  // namespace base
 
+namespace plugin_test_internal {
+class TestablePluginList;
+}
+
 namespace NPAPI {
 
 #define kDefaultPluginLibraryName FILE_PATH_LITERAL("default_plugin")
 #define kGearsPluginLibraryName FILE_PATH_LITERAL("gears")
 
 class PluginInstance;
 
 // This struct holds entry points into a plugin.  The entry points are
 // slightly different between Win/Mac and Unixes.
 struct PluginEntryPoints {
@@ skipping 175 matching lines @@
   // Disable all plugins groups that are known to be outdated, according to
   // the information hardcoded in PluginGroup, to make sure that they can't
   // be loaded on a web page and instead show a UI to update to the latest
   // version.
   void DisableOutdatedPluginGroups();
 
   const DisabledPlugins& GetDisabledPlugins() const;
 
   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();
 
   // Creates PluginGroups for the static group definitions, and adds them to
   // the PluginGroup cache of this PluginList.
   void AddHardcodedPluginGroups();
 
   // Adds the given WebPluginInfo to its corresponding group, creating it if
   // necessary, and returns the group.
   // 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.
   // |visited_plugins| is updated with paths to all plugins that were considered
   //   (including those we didn't load)
   void LoadPluginsFromDir(const FilePath& path,
                           std::set<FilePath>* visited_plugins);
 
   // Returns true if we should load the given plugin, or false otherwise.
   // plugins is the list of plugins we have crawled in the current plugin
@@ skipping 88 matching lines @@
   mutable Lock lock_;
 
   friend struct base::DefaultLazyInstanceTraits<PluginList>;
 
   DISALLOW_COPY_AND_ASSIGN(PluginList);
 };
 
 }  // namespace NPAPI
 
 #endif  // WEBKIT_GLUE_PLUGINS_PLUGIN_LIST_H_