Policy: Re-enabled plugin still disabled

webkit/plugins/npapi/plugin_group.h

Index: webkit/plugins/npapi/plugin_group.h
diff --git a/webkit/plugins/npapi/plugin_group.h b/webkit/plugins/npapi/plugin_group.h
index 308fee79a8185e542bd1121ffa7fee34f5c23747..ea317d4ea5bc003152508c0491f2edcfb300c60a 100644
--- a/webkit/plugins/npapi/plugin_group.h
+++ b/webkit/plugins/npapi/plugin_group.h
@@ -14,6 +14,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/scoped_ptr.h"
 #include "base/string16.h"
+#include "webkit/plugins/npapi/webplugininfo.h"
 
 class DictionaryValue;
 class FilePath;
@@ -25,8 +26,9 @@ namespace webkit {
 namespace npapi {
 
 class PluginList;
-struct WebPluginInfo;
-
+namespace plugin_test_internal {
+class PluginListWithoutFileIO;
+}
 // Hard-coded version ranges for plugin groups.
 struct VersionRangeDefinition {
   // Matcher for lowest version matched by this range (inclusive). May be empty
@@ -93,22 +95,31 @@ class PluginGroup {
   // 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);
-
   // 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);
+  // Adds the given plugin to this group.
+  // Retuns true if plugin has been added and false if it was already contained
+  // in the group before.
+  bool AddPlugin(const WebPluginInfo& plugin, int priority);
 
-  bool IsEmpty() const;
+  // Removes a plugin from the group by its path.
+  bool RemovePlugin(const FilePath& filename);
+
+  // The two following functions enable/disable a plugin given its filename. The
+  // function returns true if the plugin could be enabled/disabled. Plugins
+  // might not get enabled/disabled if they are controlled by policy or are
+  // already in the wanted state.
+  bool EnablePlugin(const FilePath& filename);
+  bool DisablePlugin(const FilePath& filename);
+
+  // Enables/disables this group. This enables/disables all plugins in the
+  // group.
+  bool EnableGroup(bool enable);
 
   // Enables/disables this group. This enables/disables all plugins in the

[jam, 2011/01/19 20:22:09]

nit: comment is copy and pasted from above

[pastarmovj, 2011/01/19 23:39:17]

Fixed. Sorry about that.

   // group.
-  void Enable(bool enable);
+  void EnforceGroupPolicy();
 
   // Returns whether the plugin group is enabled or not.
   bool Enabled() const { return enabled_; }
@@ -121,6 +132,10 @@ class PluginGroup {
   // is empty.
   string16 GetGroupName() const;
 
+  const std::vector<WebPluginInfo>& GetPlugins() const;

[jam, 2011/01/19 20:22:09]

simple getters should just be named in unix_hacker style, i.e.
web_plugin_infos() { return web_plugin_infos_; }

[pastarmovj, 2011/01/19 23:39:17]

Done.

+
+  bool ContainsPlugin(const FilePath& path) const;

[jam, 2011/01/19 20:22:09]

nit: small comment to match the rest of the header please

[pastarmovj, 2011/01/19 23:39:17]

Done.

+
   // Returns the description of the highest-priority plug-in in the group.
   const string16& description() const { return description_; }
 
@@ -137,6 +152,10 @@ class PluginGroup {
   // security problems.
   bool IsVulnerable() const;
 
+  // Check if the group has no plugins. Could happen after a reload if the plug-
+  // in has disappeared from the pc (or in the process of updating).
+  bool IsEmpty() const;

[jam, 2011/01/19 20:22:09]

this isn't needed anymore right?

[pastarmovj, 2011/01/19 23:39:17]

It is needed only for actually checking which groups should be pruned. It is a
shortcut to web_plugin_infos()->size() == 0.

+
   // Disables all plugins in this group that are older than the
   // minimum version.
   void DisableOutdatedPlugins();
@@ -149,6 +168,7 @@ class PluginGroup {
   typedef std::map<std::string, PluginGroup*> PluginMap;

[jam, 2011/01/19 20:22:09]

this typedef isn't used by PluginGroup, so it should be removed from here

[pastarmovj, 2011/01/19 23:39:17]

The problem is that we want to differentiate different groups based on their
identifiers (which mostly coincide with names but in cases of collisions don't
see. PluginGroup::AddToPluginGroups). Therefore we have this maps there.

[jam, 2011/01/20 00:24:25]

I don't understand what you mean.  AddToPluginGroups is in PluginList, not
PluginGroup.  I don't see any code in PluginGroup using PluginMap typedef, so
I'm saying the typedef doesn't belong in this file, it should be moved to
PluginList's header.

 
   friend class PluginList;
+  friend class plugin_test_internal::PluginListWithoutFileIO;
   friend class PluginGroupTest;
   friend class ::TableModelArrayControllerTest;
   friend class ::PluginExceptionsTableModelTest;
@@ -193,6 +213,17 @@ class PluginGroup {
   // enabled one, or if all plugins are disabled, simply the first one.
   void UpdateActivePlugin(const WebPluginInfo& plugin);
 
+  // Refreshes the enabled flag based on the state of its plugins.
+  void RefreshEnabledState();
+
+  // Enables the plugin if not already enabled and if policy allows it to.
+  // Returns true on success.
+  static bool Enable(WebPluginInfo* plugin, int reason);
+
+  // Disables the plugin if not already disabled and if policy allows it to.
+  // Returns true on success.
+  static bool Disable(WebPluginInfo* plugin, int reason);
+
   static std::set<string16>* policy_disabled_plugin_patterns_;
 
   std::string identifier_;
@@ -204,7 +235,6 @@ class PluginGroup {
   std::vector<VersionRange> version_ranges_;
   scoped_ptr<Version> version_;
   std::vector<WebPluginInfo> web_plugin_infos_;
-  std::vector<int> web_plugin_positions_;
 };
 
 }  // namespace npapi