Policy: Re-enabled plugin still disabled

webkit/plugins/npapi/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_PLUGINS_NPAPI_PLUGIN_GROUP_H_
 #define WEBKIT_PLUGINS_NPAPI_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 TableModelArrayControllerTest;
 class PluginExceptionsTableModelTest;
 class Version;
 
 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
   // 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
+  // in the group before.
+  bool AddPlugin(const WebPluginInfo& plugin, int priority);
 
-  bool IsEmpty() const;
+  // 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.
-  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_; }

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

we shouldn't be returning a non-const reference to the internal data structure.

[pastarmovj, 2010/12/21 20:31:19]

I probably don't need this version anymore. I will remove it if this is the case
or replace it with a proper manipulator function for the aim I have.

[pastarmovj, 2010/12/23 13:00:19]

Done.

+
+  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 plugins
+  // (with priority 0).

[Bernhard Bauer, 2010/12/21 19:07:39]

Comment is outdated.

[pastarmovj, 2010/12/23 13:00:19]

Done.

+  bool IsEmpty() const;
+
+  // Set plugin placeholder status
+  void SetPluginIsPlaceholder(const WebPluginInfo& plugin, bool status);
+
+  // Set plugin placeholder status

[Bernhard Bauer, 2010/12/21 19:07:39]

Comment is wrong. Also, it doesn't really tell you anything you wouldn't know
from the method name (if it was correct, that is).

[pastarmovj, 2010/12/23 13:00:19]

Done.

+  bool IsPluginPlaceholder(const WebPluginInfo& plugin);
+
   // 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 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_;
+  std::vector<int> web_plugin_priority_;

[Bernhard Bauer, 2010/12/21 19:07:39]

Nit: You should call it priorities. It's confusing otherwise.

Also, we probably could get fully rid of them, but I'd say we should do that in
a separate CL when we have proper test coverage in place so that we know we're
not breaking anything. That okay with you, John?

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

i think we can do without this.

[pastarmovj, 2010/12/23 13:00:19]

I have removed it for now but I am not sure I have cleaned all the UI traces of
that one. Should I look into some localized files for the IDS_PLUGIN_PRIORITY or
is the generated_resources the only one that I had to clean?

+
+  // Set to true for every entry in |web_plugin_infos_| that is only a
+  // placeholder.
+  std::vector<bool> is_plugin_placeholder_;

[Bernhard Bauer, 2010/12/21 19:07:39]

I'm still not exactly sure what the invariants for this are. Is there ever going
to be more than one placeholder? And what's the difference between a group
without plugins and one with a placeholder?

[pastarmovj, 2010/12/21 20:31:19]

In a group one can have more than one placeholder, but only one per distinctive
plugin (different file). From the perspective of an user there is no difference
between an empty group and one with only placeholders - both appear empty and
are not returned from the PluginList::GetPluginGroups call. The concept is not
much different than the disabledgroups/plugins lists we had before but
implemented clean in terms only of existing obejcts and reducing the chance for
these getting out of sync (as it happened before - plugins/groups couldn't get
properly disabled/enabled separately or didn't update the groups status
correctly).

 };
 
 }  // namespace npapi
 }  // namespace webkit
 
 #endif  // WEBKIT_PLUGINS_NPAPI_PLUGIN_GROUP_H_