Policy: Re-enabled plugin still disabled

webkit/glue/plugins/plugin_list.h

Index: webkit/glue/plugins/plugin_list.h
diff --git a/webkit/glue/plugins/plugin_list.h b/webkit/glue/plugins/plugin_list.h
index 734cc6d7286693456b0704c2b5711190cba81e1c..f5bf9b767c92a602dc639433b729a7bbf9aee044 100644
--- a/webkit/glue/plugins/plugin_list.h
+++ b/webkit/glue/plugins/plugin_list.h
@@ -71,6 +71,12 @@ struct PluginVersionInfo {
 // This object is thread safe.
 class PluginList {
  public:
+  // Enum of possible reasons for plugin or group being disabled. Used in the
+  // |disabled_plugins_| and |disabled_groups_| maps.
+  enum DisabledElementReason { USER = 0, POLICY, POLICY_AND_USER };

[danno, 2010/12/14 09:43:39]

How about PluginDisabledReason?

[pastarmovj, 2010/12/15 14:44:51]

Done.

+  typedef std::map<FilePath, DisabledElementReason> DisabledPluginsList;
+  typedef std::map<string16, DisabledElementReason> DisabledGroupsList;

[danno, 2010/12/14 09:43:39]

This is a map, so the list name is misleading. consider renaming to just
DisabledPlugins and DisabledGroups.

[pastarmovj, 2010/12/15 14:44:51]

Done.

+
   // Gets the one instance of the PluginList.
   static PluginList* Singleton();
 
@@ -182,8 +188,7 @@ class PluginList {
   std::string GetPluginGroupIdentifier(const WebPluginInfo& web_plugin_info);
 
   // Load a specific plugin with full path.
-  void LoadPlugin(const FilePath& filename,
-                  std::vector<WebPluginInfo>* plugins);
+  void LoadPlugin(const FilePath& filename);
 
   // Enable a specific plugin, specified by path. Returns |true| iff a plugin
   // currently in the plugin list was actually enabled as a result; regardless
@@ -195,8 +200,9 @@ class PluginList {
   // Disable a specific plugin, specified by path. Returns |true| iff a plugin
   // currently in the plugin list was actually disabled as a result; regardless
   // of return value, if a plugin is found in the future with the given name, it
-  // will be disabled.
-  bool DisablePlugin(const FilePath& filename);
+  // will be disabled. The boolean flag should be set to true if the operation

[danno, 2010/12/14 09:43:39]

... if the plugin is disabled by a policy and not a user.

[pastarmovj, 2010/12/15 14:44:51]

Done.

+  // has been trigerred by a policy and not by the user.
+  bool DisablePlugin(const FilePath& filename, bool policy_disabled);
 
   // Enable/disable a plugin group, specified by group_name.  Returns |true| iff
   // a plugin currently in the plugin list was actually enabled/disabled as a
@@ -211,6 +217,10 @@ class PluginList {
   // version.
   void DisableOutdatedPluginGroups();
 
+  void GetDisabledPlugins(DisabledPluginsList* disabled_plugins);
+
+  void GetDisabledGroups(DisabledGroupsList* disabled_groups);

[danno, 2010/12/14 09:43:39]

Do these copy? If so, why (probably should document in a comment). Alternatively
(probably better) why not just return a const reference?

[pastarmovj, 2010/12/15 14:44:51]

Done.

+
   ~PluginList();
 
  private:
@@ -231,19 +241,20 @@ class PluginList {
   // Load all plugins from the default plugins directory
   void LoadPlugins(bool refresh);
 
+  // Populates the |plugins_| vector if is has been marked as dirty.

[danno, 2010/12/14 09:43:39]

"it" missing?

[pastarmovj, 2010/12/15 14:44:51]

Done. s/is/it/

+  void RebuildPluginsList();
+
   // 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.
   // plugins is the list of plugins we have crawled in the current plugin
   // loading run.
-  bool ShouldLoadPlugin(const WebPluginInfo& info,
-                        std::vector<WebPluginInfo>* plugins);
+  bool ShouldLoadPlugin(const WebPluginInfo& info);
 
   // Return whether a plug-in group with the given name should be disabled,
   // either because it already is on the list of disabled groups, or because it
@@ -286,7 +297,7 @@ class PluginList {
 
   // Loads plugins registered under HKCU\Software\MozillaPlugins and
   // HKLM\Software\MozillaPlugins.
-  void LoadPluginsFromRegistry(std::vector<WebPluginInfo>* plugins,
+  void LoadPluginsFromRegistry(std::vector<WebPluginInfo*>* plugins,
                                std::set<FilePath>* visited_plugins);
 #endif
 
@@ -300,7 +311,8 @@ class PluginList {
   bool plugins_need_refresh_;
 
   // Contains information about the available plugins.
-  std::vector<WebPluginInfo> plugins_;
+  std::vector<WebPluginInfo*> plugins_;

[danno, 2010/12/14 09:43:39]

I see that you don't explicitly manage the memory of the WebPluginInfos
explicitly with this change (I see no STLDeleteContainerPointers in the
destructor). If you are relying on the the objects staying alive due to some
side-effect of ownership somewhere else, it needs to be clearly documented. 

+  bool plugins_vector_is_dirty_;

[danno, 2010/12/14 09:43:39]

Please document when this actually happens.

[pastarmovj, 2010/12/15 14:44:51]

Obsoleted,

 
   // Extra plugin paths that we want to search when loading.
   std::vector<FilePath> extra_plugin_paths_;
@@ -311,11 +323,15 @@ class PluginList {
   // 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, DisabledElementReason> DisabledPluginsListElement;
   // Path names of plugins to disable (the default is to enable them all).
-  std::set<FilePath> disabled_plugins_;
+  DisabledPluginsList disabled_plugins_;
 
+  typedef std::pair<string16, DisabledElementReason> DisabledGroupsListElement;
   // Group names to disable (the default is to enable them all).
-  std::set<string16> disabled_groups_;
+  DisabledGroupsList disabled_groups_;
 
   bool disable_outdated_plugins_;