Policy: Re-enabled plugin still disabled

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;
 };
-
+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
   // to match everything iff |version_matcher_high| is also empty.
   const char* version_matcher_low;
   // Matcher for highest version matched by this range (exclusive). May be empty
   // to match anything higher than |version_matcher_low|.
   const char* version_matcher_high;
   const char* min_version;  // Minimum secure version.
 };
@@ skipping 46 matching lines @@
   PluginGroup& operator=(const PluginGroup& other);
 
   // 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);
-
   // 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
+  // inn the group before.

[Bernhard Bauer, 2010/12/20 22:30:28]

Nit: "in".

[pastarmovj, 2010/12/21 16:53:02]

Done.

+  bool AddPlugin(const WebPluginInfo& plugin, int priority);
+
+  // Enables a plugin given its filename. The function returns true if the
+  // plugin could be enabled. Plugins might not get enabled if they are disabled
+  // by policy or are already in the disabled state.
+  bool EnablePlugin(const FilePath& filename);
+
+  // Disables a plugin given its filename. The function returns true if the
+  // plugin could be disabled. Plugins might not get enabled if they are enabled
+  // by policy or are already in the enabled state.

[Bernhard Bauer, 2010/12/20 22:30:28]

You could group these comments ("the two following functions enable or disable
...").

[pastarmovj, 2010/12/21 16:53:02]

Done.

+  bool DisablePlugin(const FilePath& filename);
 
   // Enables/disables this group. This enables/disables all plugins in the
   // group.
-  void Enable(bool enable);
+  bool EnableGroup(bool enable);
 
   // Returns whether the plugin group is enabled or not.
   bool Enabled() const { return enabled_; }
 
   // Returns a unique identifier for this group, if one is defined, or the empty
   // string otherwise.
   const std::string& identifier() const { return identifier_; }
 
   // Returns this group's name, or the filename without extension if the name
   // is empty.
   string16 GetGroupName() const;
 
+  const std::vector<WebPluginInfo>& GetPlugins() const;
+
+  std::vector<WebPluginInfo>& GetPlugins() { return web_plugin_infos_; }
+
+  bool ContainsPlugin(const FilePath& path) const;
+
   // Returns the description of the highest-priority plug-in in the group.
   const string16& description() const { return description_; }
 
   // Returns a DictionaryValue with data to display in the UI.
   DictionaryValue* GetDataForUI() const;
 
   // Returns a DictionaryValue with data to save in the preferences.
   DictionaryValue* GetSummary() const;
 
   // Returns the update URL.
   std::string GetUpdateURL() const { return update_url_; }
 
   // Returns true if the highest-priority plugin in this group has known
   // security problems.
   bool IsVulnerable() const;
 
+  // Check if the group has no plugins or only non-existing pluigns

[Bernhard Bauer, 2010/12/20 22:30:28]

Nit: plugins

[pastarmovj, 2010/12/21 16:53:02]

Done.

+  // (with priority 0).

[Bernhard Bauer, 2010/12/20 22:30:28]

This priority 0 thing is definitely a step in the wrong direction. Like John
said, if we want to have placeholder groups, we can store that state in the
group itself.

[pastarmovj, 2010/12/21 16:53:02]

I moved the priority back into the group class and introduced a placeholder flag
separately from it so that we don't mix two meanings in the same variable.

[jam, 2010/12/21 19:57:42]

I still don't think we need a priority at all.  Can you please explain why it's
needed?  The only example I've heard so far is because both internal and
external flash are in the same group.  There are two solutions to that: either
put them in separate groups, and have the internal one first, or add the
internal one before the external one in a group.  that would reduce the need to
have priority, which seems redundant since the same data can (and used to be)
taken from the ordering of the plugin list.

+  bool IsEmpty() const;
+
   // Disables all plugins in this group that are older than the
   // minimum version.
   void DisableOutdatedPlugins();
 
   // Parse a version string as used by a plug-in. This method is more lenient
   // in accepting weird version strings than Version::GetFromString().
   static Version* CreateVersionFromString(const string16& version_string);
 
  private:
   typedef std::map<std::string, PluginGroup*> PluginMap;
 
   friend class NPAPI::PluginList;
+  friend class plugin_test_internal::PluginListWithoutFileIO;
   friend class PluginGroupTest;
   friend class TableModelArrayControllerTest;
   friend class PluginExceptionsTableModelTest;
 
   // 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);
 
   // Creates a PluginGroup from a PluginGroupDefinition. The caller takes
   // ownership of the created PluginGroup.
   static PluginGroup* FromPluginGroupDefinition(
       const PluginGroupDefinition& definition);
 
   // Creates a PluginGroup from a WebPluginInfo. The caller takes ownership of
   // the created PluginGroup.
   static PluginGroup* FromWebPluginInfo(const WebPluginInfo& wpi);
 
+  // Creates an empty PluginGroup from a given name. The caller takes
+  // ownership of the created PluginGroup.
+  static PluginGroup* CreateEmptyGroup(const string16& name);
+
   // Returns |true| if |version| is contained in [low, high) of |range|.
   static bool IsVersionInRange(const Version& version,
                                const VersionRange& range);
 
   // Returns |true| iff |plugin_version| is both contained in |version_range|
   // and declared outdated (== vulnerable) by it.
   static bool IsPluginOutdated(const Version& plugin_version,
                                const VersionRange& version_range);
 
   PluginGroup(const string16& group_name,
               const string16& name_matcher,
               const std::string& update_url,
               const std::string& identifier);
 
   void InitFrom(const PluginGroup& other);
 
   // Set the description and version for this plugin group from the
   // given plug-in.
   void UpdateDescriptionAndVersion(const WebPluginInfo& plugin);
 
   // Updates the active plugin in the group. The active plugin is the first
   // 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();
+
   static std::set<string16>* policy_disabled_plugin_patterns_;
 
   std::string identifier_;
   string16 group_name_;
   string16 name_matcher_;
   string16 description_;
   std::string update_url_;
   bool enabled_;
   std::vector<VersionRange> version_ranges_;
   scoped_ptr<Version> version_;
   std::vector<WebPluginInfo> web_plugin_infos_;
-  std::vector<int> web_plugin_positions_;
 };
 
 #endif  // WEBKIT_GLUE_PLUGINS_PLUGIN_GROUP_H_