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>
@@ skipping 10 matching lines @@
 
 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 178 matching lines @@
   // 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;
 
   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.
   // Callers need to protect calls to this method by a lock themselves.
   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.
   // |visited_plugins| is updated with paths to all plugins that were considered
   //   (including those we didn't load)
   void LoadPluginsFromDir(const FilePath& path,
                           std::vector<WebPluginInfo*>* plugins,
                           std::set<FilePath>* visited_plugins);
 
   // Returns true if we should load the given plugin, or false otherwise.
@@ skipping 44 matching lines @@
   // Loads plugins registered under HKCU\Software\MozillaPlugins and
   // HKLM\Software\MozillaPlugins.
   void LoadPluginsFromRegistry(std::set<FilePath>* visited_plugins,
                                std::vector<WebPluginInfo*>* plugins);
 #endif
 
   //
   // 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_;
 
   // Extra plugin directories that we want to search when loading.
   std::vector<FilePath> extra_plugin_dirs_;
 
   // Holds information about internal plugins.
   std::vector<PluginVersionInfo> internal_plugins_;
 
   // 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).
   DisabledGroups disabled_groups_;
 
   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
   // accessed on multiple threads.
   mutable Lock lock_;
 
   friend struct base::DefaultLazyInstanceTraits<PluginList>;
 
   DISALLOW_COPY_AND_ASSIGN(PluginList);
 };
 
 }  // namespace NPAPI
 
 #endif  // WEBKIT_GLUE_PLUGINS_PLUGIN_LIST_H_