Policy: Re-enabled plugin still disabled

webkit/plugins/npapi/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_PLUGINS_NPAPI_PLUGIN_LIST_H_
 #define WEBKIT_PLUGINS_NPAPI_PLUGIN_LIST_H_
 
+#include <map>
 #include <set>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/file_path.h"
 #include "base/linked_ptr.h"
 #include "base/lock.h"
 #include "third_party/npapi/bindings/nphostapi.h"
 #include "webkit/plugins/npapi/plugin_group.h"
 #include "webkit/plugins/npapi/webplugininfo.h"
 
@@ skipping 76 matching lines @@
 
   // Same as above, but specifies a directory in which to search for plugins.
   void AddExtraPluginDir(const FilePath& plugin_dir);
 
   // Get the ordered list of directories from which to load plugins
   void GetPluginDirectories(std::vector<FilePath>* plugin_dirs);
 
   // Register an internal plugin with the specified plugin information and
   // function pointers.  An internal plugin must be registered before it can
   // be loaded using PluginList::LoadPlugin().
-  void RegisterInternalPlugin(const FilePath& path);

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

why take this out?  it was convenient for internal plugins.

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

As already mentioned in my comment to
http://codereview.chromium.org/5699005/diff/142001/chrome/browser/automation/...
this function was only used in one unit test. (And after Evan's CL even there
not anymore). It doesn't contribute to the interface it only can be source of
confusion. All plugins use the full fledged 5 param version (and have to because
they need to supply the entry points array).

   void RegisterInternalPlugin(const FilePath& filename,
                               const std::string& name,
                               const std::string& description,
                               const std::string& mime_type,
                               const PluginEntryPoints& entry_points);
 
   // Deprecated version of the above.
   // TODO(evan): remove when NaCl is fixed.
   void RegisterInternalPlugin(const PluginVersionInfo& info);
 
@@ skipping 63 matching lines @@
   // Beware: when calling this from the Browser process, the group that the
   // returned pointer points to might disappear suddenly. This happens when
   // |RefreshPlugins()| is called and then |LoadPlugins()| is triggered by a
   // call to |GetPlugins()|, |GetEnabledPlugins()|, |GetPluginInfoArray()|,
   // |GetPluginInfoByPath()|, or |GetPluginGroups(true, _)|. It is the caller's
   // responsibility to make sure this doesn't happen.
   const PluginGroup* GetPluginGroup(const WebPluginInfo& web_plugin_info);
 
   // 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);

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

nit: while you're at it, this should be const std::string&

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

Done.

 
   // 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);
 
   // 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.
+  // will be enabled.
   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);
 
   // 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.
+  // the given name, it will be enabled/disabled.
   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();
 
-  ~PluginList();
+  virtual ~PluginList();
 
- private:
-  FRIEND_TEST_ALL_PREFIXES(PluginGroupTest, PluginGroupDefinition);
-
+ protected:
   // 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();
+  // Load all plugins from the default plugins directory
+  virtual void LoadPlugins(bool refresh);
+
+  // Allows tests to perform custom post-initialization tasks on newly created
+  // PluginGroups.
+  virtual void ProcessGroupAfterInitialize(PluginGroup* group) { }
 
   // 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);
+  bool plugins_loaded_;
+
+  // Holds the currently available plugin groups.
+  PluginGroup::PluginMap plugin_groups_;

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

does this really need to be a map?  we used to use a vector before because then
ordering is a way of signifying who gets first dibs.  this way we can put the
plugin installer plugin (i.e. the one that catches all mime-types) at the end. 
we would also put internal plugins first, i.e. the pdf.

I've looked at the usage of this, and almost all places just iterate over it. 
for the other (2) places, they could iterate manually. so i think given that
performance is not an issue, we should use a vector as before.

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

Copied from plugin_group.cc question about the typedef:
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]

Sorry I don't follow here.  if AddToPluginGroups finds that it wants to insert a
group with an identifier that already exists (I'm actually not sure I understand
when this situation can happen?), why shouldn't we just set the new group's
identifier() to its GetLongIdentifier()?

+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(PluginGroupTest, PluginGroupDefinition);
+
+  // Creates PluginGroups for the static group definitions, and adds them to
+  // the PluginGroup cache of this PluginList.
+  void AddHardcodedPluginGroups();
 
   // 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);
 
   // 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,
-                           const std::string &mime_type,
-                           bool allow_wildcard);
+  // Returns true if the plugin supports |mime_type|. |mime_type| should be all
+  // lower case.
+  bool SupportsType(const WebPluginInfo& plugin,
+                    const std::string& mime_type,
+                    bool allow_wildcard);
 
-  // Returns true if the given WebPluginInfo supports a given file extension.
-  // extension should be all lower case.
-  // If mime_type is not NULL, it will be set to the mime type if found.
-  // The mime type which corresponds to the extension is optionally returned
-  // back.
-  static bool SupportsExtension(const WebPluginInfo& info,
-                                const std::string &extension,
-                                std::string* actual_mime_type);
-
+  // Returns true if the given plugin supports a given file extension.
+  // |extension| should be all lower case. If |mime_type| is not NULL, it will
+  // be set to the MIME type if found. The MIME type which corresponds to the
+  // extension is optionally returned back.
+  bool SupportsExtension(const WebPluginInfo& plugin,
+                         const std::string& extension,
+                         std::string* actual_mime_type);
   //
   // Platform functions
   //
 
   // Do any initialization.
   void PlatformInit();
 
   //
   // 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,
                                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_;
-
   // 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_;
 
-  // Path names of plugins to disable (the default is to enable them all).
-  std::set<FilePath> disabled_plugins_;
-
-  // Group names to disable (the default is to enable them all).
-  std::set<string16> disabled_groups_;
-
   bool disable_outdated_plugins_;
 
-  // Holds the currently available plugin groups.
-  PluginGroup::PluginMap plugin_groups_;
+  std::vector<FilePath> prematurely_disabled_plugins_;

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

comment please

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

Done.

+  std::vector<string16> prematurely_disabled_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
 }  // namespace webkit
 
 #endif  // WEBKIT_PLUGINS_NPAPI_PLUGIN_LIST_H_