Clean up PluginGroup and related code.

webkit/glue/plugins/plugin_group.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_GROUP_H_
 #define WEBKIT_GLUE_PLUGINS_PLUGIN_GROUP_H_
 #pragma once
 
 #include <map>
 #include <set>
+#include <string>
 #include <vector>
 
 #include "base/gtest_prod_util.h"
 #include "base/scoped_ptr.h"
 #include "base/string16.h"
 
 class DictionaryValue;
 class FilePath;
 class Version;
 struct WebPluginInfo;
 
 namespace NPAPI {
   class PluginList;
 };
 
-template <typename T>
-class linked_ptr;
-
 // Hard-coded definitions of plugin groups.
 struct PluginGroupDefinition {
   const char* identifier;  // Unique identifier for this group.
   const char* name;  // Name of this group.
   const char* name_matcher;  // Substring matcher for the plugin name.
   const char* version_matcher_low;  // Matchers for the plugin version.
   const char* version_matcher_high;
   const char* min_version;  // Minimum secure version.
   const char* update_url;  // Location of latest secure version.
 };
 
 // A PluginGroup can match a range of versions of a specific plugin (as defined
 // by matching a substring of its name).
 // It contains all WebPluginInfo structs (at least one) matching its definition.
 // In addition, it knows about a security "baseline", i.e. the minimum version
 // of a plugin that is needed in order not to exhibit known security
 // vulnerabilities.
 
 class PluginGroup {
  public:
   // Used by about:plugins to disable Reader plugin when internal PDF viewer is
   // enabled.
   static const char* kAdobeReader8GroupName;
   static const char* kAdobeReader9GroupName;
 
-  typedef std::map<std::string, linked_ptr<PluginGroup> > PluginMap;
+  typedef std::map<std::string, PluginGroup*> PluginMap;

[Bernhard Bauer, 2010/12/03 16:13:28]

Do we need this typedef outside of PluginGroup or can we make it private?

[Jakob Kummerow, 2010/12/06 18:21:12]

Done. (Made private.)

 
   // Creates a PluginGroup from a PluginGroupDefinition.
   static PluginGroup* FromPluginGroupDefinition(
       const PluginGroupDefinition& definition);
 
+  PluginGroup(const PluginGroup& other);

[Bernhard Bauer, 2010/12/03 16:13:28]

Nit: make the single-argument constructor explicit.

[Jakob Kummerow, 2010/12/06 18:21:12]

Nope. Let me quote the style guide:
"We require all single argument constructors to be explicit. [...]
The exception is copy constructors, which, in the rare cases when we allow them,
should probably not be explicit."
In fact, it doesn't compile if this copy constructor is explicit.
The only alternative would be to never store plain PluginGroups (as opposed to
pointers) in std::vectors or similar containers, and get rid of this constructor
altogether, but then we'd still have the same threading/concurrency problems
this cleanup is intended to get rid of.

+
   ~PluginGroup();
 
-  // Creates a PluginGroup from a WebPluginInfo -- when no hard-coded
-  // definition is found.
+  PluginGroup& operator=(const PluginGroup& other);
+
+  // Creates a PluginGroup from a WebPluginInfo. Should not be called externally
+  // except by PluginList and unit tests.

[Bernhard Bauer, 2010/12/03 16:13:28]

PluginList is already a friend, so just declare this method private. 

Also, please explain that the caller takes ownership of the returned object.

[Jakob Kummerow, 2010/12/06 18:21:12]

Done.
This required adding several more friend classes (which is why I didn't make the
method private before).
Done.

   static PluginGroup* FromWebPluginInfo(const WebPluginInfo& wpi);
 
-  // Find a plugin group matching |info| in the list of hardcoded plugins and
-  // returns a copy of it if found, or a new group matching exactly this plugin
-  // otherwise.
-  // The caller should take ownership of the return PluginGroup.
-  static PluginGroup* CopyOrCreatePluginGroup(const WebPluginInfo& info);
-
   // Configures the set of plugin name patterns for disabling plugins via
   // enterprise configuration management.
   static void SetPolicyDisabledPluginPatterns(const std::set<string16>& set);
 
   // Tests to see if a plugin is on the blacklist using its name as
   // the lookup key.
   static bool IsPluginNameDisabledByPolicy(const string16& plugin_name);
 
   // Tests to see if a plugin is on the blacklist using its path as
   // the lookup key.
   static bool IsPluginPathDisabledByPolicy(const FilePath& plugin_path);
 
-  // Find the PluginGroup matching a Plugin in a list of plugin groups. Returns
-  // NULL if no matching PluginGroup is found.
-  static PluginGroup* FindGroupMatchingPlugin(
-      const std::map<std::string, linked_ptr<PluginGroup> >& plugin_groups,
-      const WebPluginInfo& plugin);
-
-  // Creates a copy of this plugin group.
-  PluginGroup* Copy() {
-    return new PluginGroup(group_name_, name_matcher_, version_range_low_str_,
-                           version_range_high_str_, min_version_str_,
-                           update_url_, identifier_);
-  }
-
   // Returns true if the given plugin matches this group.
   bool Match(const WebPluginInfo& plugin) const;
 
   // Adds the given plugin to this group. Provide the position of the
   // plugin as given by PluginList so we can display its priority.
   void AddPlugin(const WebPluginInfo& plugin, int position);
 
   // Enables/disables this group. This enables/disables all plugins in the
   // group.
   void Enable(bool enable);
@@ skipping 23 matching lines @@
 
   // Returns true if the highest-priority plugin in this group has known
   // security problems.
   bool IsVulnerable() const;
 
   // Disables all plugins in this group that are older than the
   // minimum version.
   void DisableOutdatedPlugins();
 
  private:
+  friend class NPAPI::PluginList;
   FRIEND_TEST_ALL_PREFIXES(PluginGroupTest, PluginGroupDefinition);
 
   static const PluginGroupDefinition* GetPluginGroupDefinitions();
   static size_t GetPluginGroupDefinitionsSize();
 
+  // Generates the (short) identifier string for the given plugin.
+  static std::string GetIdentifier(const WebPluginInfo& wpi);
+
+  // Generates the long identifier (based on the full file path) for the given
+  // plugin, to be called when the short identifier is not unique.
+  static std::string GetLongIdentifier(const WebPluginInfo& wpi);
+
+  // Find the PluginGroup matching a Plugin in a list of plugin groups. Returns
+  // NULL if no matching PluginGroup is found.
+  static PluginGroup* FindGroupMatchingPlugin(
+      const PluginMap* plugin_groups,
+      const WebPluginInfo& plugin);
+
   PluginGroup(const string16& group_name,
               const string16& name_matcher,
               const std::string& version_range_low,
               const std::string& version_range_high,
               const std::string& min_version,
               const std::string& update_url,
               const std::string& identifier);
 
   Version* CreateVersionFromString(const string16& version_string);
 
@@ skipping 15 matching lines @@
   scoped_ptr<Version> version_range_low_;
   scoped_ptr<Version> version_range_high_;
   string16 description_;
   std::string update_url_;
   bool enabled_;
   std::string min_version_str_;
   scoped_ptr<Version> min_version_;
   scoped_ptr<Version> version_;
   std::vector<WebPluginInfo> web_plugin_infos_;
   std::vector<int> web_plugin_positions_;
-
-  DISALLOW_COPY_AND_ASSIGN(PluginGroup);
 };
 
 #endif  // WEBKIT_GLUE_PLUGINS_PLUGIN_GROUP_H_