Policy: Re-enabled plugin still disabled

webkit/glue/plugins/plugin_list.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_LIST_H_
 #define WEBKIT_GLUE_PLUGINS_PLUGIN_LIST_H_
 
 #include <set>
 #include <string>
 #include <vector>
@@ skipping 53 matching lines @@
 // The PluginList is responsible for loading our NPAPI based plugins. It does
 // so in whatever manner is appropriate for the platform. On Windows, it loads
 // plugins from a known directory by looking for DLLs which start with "NP",
 // and checking to see if they are valid NPAPI libraries. On the Mac, it walks
 // the machine-wide and user plugin directories and loads anything that has
 // the correct types. On Linux, it walks the plugin directories as well
 // (e.g. /usr/lib/browser-plugins/).
 // 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();
 
   // Returns true if we're in debug-plugin-loading mode. This is controlled
   // by a command line switch.
   static bool DebugPluginLoading();
 
   static const PluginGroupDefinition* GetPluginGroupDefinitions();
   static size_t GetPluginGroupDefinitionsSize();
 
@@ skipping 91 matching lines @@
   // Returns the name of the PluginGroup with the given identifier.
   // If no such group exists, an empty string is returned.
   string16 GetPluginGroupName(std::string identifier);
 
   // Returns the identifier string of the PluginGroup corresponding to the given
   // WebPluginInfo. If no such group exists, it is created and added to the
   // cache.
   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
   // of return value, if a plugin is found in the future with the given name, it
   // will be enabled. Note that plugins are enabled by default as far as
   // |PluginList| is concerned.
   bool EnablePlugin(const FilePath& filename);
 
   // 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
   // result; regardless of return value, if a plugin is found in the future with
   // the given name, it will be enabled/disabled. Note that plugins are enabled
   // by default as far as |PluginList| is concerned.
   bool EnableGroup(bool enable, const string16& name);
 
   // Disable all plugins groups that are known to be outdated, according to
   // the information hardcoded in PluginGroup, to make sure that they can't
   // be loaded on a web page and instead show a UI to update to the latest
   // 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:
   FRIEND_TEST_ALL_PREFIXES(PluginGroupTest, PluginGroupDefinition);
 
   // Constructors are private for singletons
   PluginList();
 
   // Creates PluginGroups for the static group definitions, and adds them to
   // the PluginGroup cache of this PluginList.
   void AddHardcodedPluginGroups();
 
   // Adds the given WebPluginInfo to its corresponding group, creating it if
   // necessary, and returns the group.
   // Callers need to protect calls to this method by a lock themselves.
   PluginGroup* AddToPluginGroups(const WebPluginInfo& web_plugin_info);
 
   // 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
   // is blacklisted by a policy. In the latter case, add the plugin group to the
   // list of disabled groups as well.
   bool ShouldDisableGroup(const string16& group_name);
 
   // Returns true if the given WebPluginInfo supports "mime-type".
   // mime_type should be all lower case.
   static bool SupportsType(const WebPluginInfo& info,
@@ skipping 22 matching lines @@
   //
   // Command-line switches
   //
 
 #if defined(OS_WIN)
   // true if we shouldn't load the new WMP plugin.
   bool dont_load_new_wmp_;
 
   // 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
 
   //
   // Internals
   //
 
   bool plugins_loaded_;
 
   // If true, we reload plugins even if they've been loaded already.
   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_;
 
   // Extra plugin directories that we want to search when loading.
   std::vector<FilePath> extra_plugin_dirs_;
 
   // 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_;
 
   // Holds the currently available plugin groups.
   PluginGroup::PluginMap plugin_groups_;
 
   int next_priority_;
 
   // Need synchronization for the above members since this object can be
   // accessed on multiple threads.
   Lock lock_;
 
   friend struct base::DefaultLazyInstanceTraits<PluginList>;
 
   DISALLOW_COPY_AND_ASSIGN(PluginList);
 };
 
 }  // namespace NPAPI
 
 #endif  // WEBKIT_GLUE_PLUGINS_PLUGIN_LIST_H_