Policy: Re-enabled plugin still disabled

webkit/glue/plugins/plugin_list.cc

 // 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.
 
 #include "webkit/glue/plugins/plugin_list.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
@@ skipping 271 matching lines @@
       plugins_need_refresh_(false),
       disable_outdated_plugins_(false),
       next_priority_(0) {
   PlatformInit();
   AddHardcodedPluginGroups();
 }
 
 bool PluginList::ShouldDisableGroup(const string16& group_name) {
   AutoLock lock(lock_);
   if (PluginGroup::IsPluginNameDisabledByPolicy(group_name)) {
-    disabled_groups_.insert(group_name);
+    disabled_groups_.insert(DisabledGroupsListElement(group_name, POLICY));
     return true;
   }
   return disabled_groups_.count(group_name) > 0;
 }
 
 void PluginList::LoadPlugins(bool refresh) {
   // Don't want to hold the lock while loading new plugins, so we don't block
   // other methods if they're called on other threads.
   std::vector<FilePath> extra_plugin_paths;
   std::vector<FilePath> extra_plugin_dirs;
   std::vector<PluginVersionInfo> internal_plugins;
   {
     AutoLock lock(lock_);
     if (plugins_loaded_ && !refresh && !plugins_need_refresh_)
       return;
 
     // Clear the refresh bit now, because it might get set again before we
     // reach the end of the method.
     plugins_need_refresh_ = false;
     extra_plugin_paths = extra_plugin_paths_;
     extra_plugin_dirs = extra_plugin_dirs_;
     internal_plugins = internal_plugins_;
   }
 
-  std::vector<WebPluginInfo> new_plugins;
+  std::vector<WebPluginInfo*> new_plugins;

[jam, 2010/12/15 18:25:50]

why is this change necessary?

[pastarmovj, 2010/12/15 21:56:04]

Before we got a copy of the plugins in PluginsList::plugins_ and in the
PluginGroup for that plugin. Therefore all code handling the state of plugin had
to make sure the state of these was kept consistent as both are used in
different contexts. However this tends to be not so trivial given the different
contexts these happen to occur. In order to resolve this problem it is better to
have only one instance of a WebPluginInfo per plugin. I haven't found any better
way than this though to do it yet.  Maybe using some sort of smart pointers can
be a better solution but still doesn't solve the problem who owns that element.
In general I see a long term solution in completely scraping the plugins_ member
in favor of using directly the groups. However this is a major refactoring that
is a bit outside the scope of that bugfix. 

I am open for other solutions here though.

   std::set<FilePath> visited_plugins;
 
   std::vector<FilePath> directories_to_scan;
   GetPluginDirectories(&directories_to_scan);
 
   // Load internal plugins first so that, if both an internal plugin and a
   // "discovered" plugin want to handle the same type, the internal plugin
   // will have precedence.
   for (size_t i = 0; i < internal_plugins.size(); ++i) {
     if (internal_plugins[i].path.value() == kDefaultPluginLibraryName)
@@ skipping 11 matching lines @@
 
   for (size_t i = 0; i < extra_plugin_dirs.size(); ++i) {
     LoadPluginsFromDir(extra_plugin_dirs[i], &new_plugins, &visited_plugins);
   }
 
   for (size_t i = 0; i < directories_to_scan.size(); ++i) {
     LoadPluginsFromDir(directories_to_scan[i], &new_plugins, &visited_plugins);
   }
 
 #if defined(OS_WIN)
-  LoadPluginsFromRegistry(&new_plugins, &visited_plugins);
+  LoadPluginsFromRegistry(&visited_plugins, &new_plugins);
 #endif
 
   // Load the default plugin last.
   if (webkit_glue::IsDefaultPluginEnabled())
     LoadPlugin(FilePath(kDefaultPluginLibraryName), &new_plugins);
 
   // Disable all of the plugins and plugin groups that are disabled by policy.
   // There's currenly a bug that makes it impossible to correctly re-enable
   // plugins or plugin-groups to their original, "pre-policy" state, so
   // plugins and groups are only changed to a more "safe" state after a policy
   // change, i.e. from enabled to disabled. See bug 54681.
   for (PluginGroup::PluginMap::iterator it = plugin_groups_.begin();
        it != plugin_groups_.end(); ++it) {
     PluginGroup* group = it->second;
     string16 group_name = group->GetGroupName();
     if (ShouldDisableGroup(group_name)) {
       group->Enable(false);
     }
 
     if (disable_outdated_plugins_) {
       group->DisableOutdatedPlugins();
     }
     if (!group->Enabled()) {
       AutoLock lock(lock_);
-      disabled_groups_.insert(group_name);
+      if (disabled_groups_.count(group_name) == 0)
+        disabled_groups_.insert(DisabledGroupsListElement(group_name, USER));
     }
   }
 
-  // Only update the data now since loading plugins can take a while.
   AutoLock lock(lock_);
-
   plugins_ = new_plugins;

[Bernhard Bauer, 2010/12/15 17:23:45]

What happens to the old pointers in |plugins_|? Do we leak them? What's the
reason we're now using pointers, BTW?

[pastarmovj, 2010/12/15 21:56:04]

About the reason for pointers please look at the response to John's question
above. 

As of the pointers being leaked - no, we don't leak then because the groups own
the objects behind these pointers. And because the groups are being built and
owned by the PluginList we can make sure the pointers never outlive the objects.
All functions that give information about plugins in the system only make copies
of these objects so they can't pass pointer that might get 0xDEADBEEF-ed while
in use.

   plugins_loaded_ = true;
 }
 
 void PluginList::LoadPlugin(const FilePath& path,
-                            std::vector<WebPluginInfo>* plugins) {
+                            std::vector<WebPluginInfo*>* plugins) {
   LOG_IF(ERROR, PluginList::DebugPluginLoading())
       << "Loading plugin " << path.value();
 
   WebPluginInfo plugin_info;
   const PluginEntryPoints* entry_points;
 
   if (!ReadPluginInfo(path, &plugin_info, &entry_points))
     return;
 
   if (!ShouldLoadPlugin(plugin_info, plugins))
@@ skipping 16 matching lines @@
 
   // Mark disabled plugins as such. (This has to happen before calling
   // |AddToPluginGroups(plugin_info)|.)
   if (disabled_plugins_.count(plugin_info.path)) {
     plugin_info.enabled = false;
   } else {
     plugin_info.enabled = true;
   }
 
   AutoLock lock(lock_);
-  plugins->push_back(plugin_info);
-  AddToPluginGroups(plugin_info);
+  AddToPluginGroups(plugin_info, plugins);
 }
 
 bool PluginList::SupportsType(const WebPluginInfo& info,
                               const std::string &mime_type,
                               bool allow_wildcard) {
   // Webkit will ask for a plugin to handle empty mime types.
   if (mime_type.empty())
     return false;
 
   for (size_t i = 0; i < info.mime_types.size(); ++i) {
@@ skipping 23 matching lines @@
   }
 
   return false;
 }
 
 
 void PluginList::GetPlugins(bool refresh, std::vector<WebPluginInfo>* plugins) {
   LoadPlugins(refresh);
 
   AutoLock lock(lock_);
-  *plugins = plugins_;
+  for (std::vector<WebPluginInfo*>::const_iterator it = plugins_.begin();
+       it != plugins_.end();
+       ++it) {
+    plugins->push_back(**it);
+  }
 }
 
 void PluginList::GetEnabledPlugins(bool refresh,
                                    std::vector<WebPluginInfo>* plugins) {
   LoadPlugins(refresh);
 
   plugins->clear();
   AutoLock lock(lock_);
-  for (std::vector<WebPluginInfo>::const_iterator it = plugins_.begin();
+  for (std::vector<WebPluginInfo*>::const_iterator it = plugins_.begin();
        it != plugins_.end();
        ++it) {
-    if (it->enabled)
-      plugins->push_back(*it);
+    if ((*it)->enabled)
+      plugins->push_back(**it);
   }
 }
 
 void PluginList::GetPluginInfoArray(
     const GURL& url,
     const std::string& mime_type,
     bool allow_wildcard,
     std::vector<WebPluginInfo>* info,
     std::vector<std::string>* actual_mime_types) {
   DCHECK(mime_type == StringToLowerASCII(mime_type));
   DCHECK(info);
 
   LoadPlugins(false);
   AutoLock lock(lock_);
   info->clear();
   if (actual_mime_types)
     actual_mime_types->clear();
 
   std::set<FilePath> visited_plugins;
 
   // Add in enabled plugins by mime type.
   WebPluginInfo default_plugin;
   for (size_t i = 0; i < plugins_.size(); ++i) {
-    if (plugins_[i].enabled &&
-        SupportsType(plugins_[i], mime_type, allow_wildcard)) {
-      FilePath path = plugins_[i].path;
+    if (plugins_[i]->enabled &&
+        SupportsType(*plugins_[i], mime_type, allow_wildcard)) {
+      FilePath path = plugins_[i]->path;
       if (path.value() != kDefaultPluginLibraryName &&
           visited_plugins.insert(path).second) {
-        info->push_back(plugins_[i]);
+        info->push_back(*plugins_[i]);
         if (actual_mime_types)
           actual_mime_types->push_back(mime_type);
       }
     }
   }
 
   // Add in enabled plugins by url.
   std::string path = url.path();
   std::string::size_type last_dot = path.rfind('.');
   if (last_dot != std::string::npos) {
     std::string extension = StringToLowerASCII(std::string(path, last_dot+1));
     std::string actual_mime_type;
     for (size_t i = 0; i < plugins_.size(); ++i) {
-      if (plugins_[i].enabled &&
-          SupportsExtension(plugins_[i], extension, &actual_mime_type)) {
-        FilePath path = plugins_[i].path;
+      if (plugins_[i]->enabled &&
+          SupportsExtension(*plugins_[i], extension, &actual_mime_type)) {
+        FilePath path = plugins_[i]->path;
         if (path.value() != kDefaultPluginLibraryName &&
             visited_plugins.insert(path).second) {
-          info->push_back(plugins_[i]);
+          info->push_back(*plugins_[i]);
           if (actual_mime_types)
             actual_mime_types->push_back(actual_mime_type);
         }
       }
     }
   }
 
   // Add in disabled plugins by mime type.
   for (size_t i = 0; i < plugins_.size(); ++i) {
-    if (!plugins_[i].enabled &&
-        SupportsType(plugins_[i], mime_type, allow_wildcard)) {
-      FilePath path = plugins_[i].path;
+    if (!plugins_[i]->enabled &&
+        SupportsType(*plugins_[i], mime_type, allow_wildcard)) {
+      FilePath path = plugins_[i]->path;
       if (path.value() != kDefaultPluginLibraryName &&
           visited_plugins.insert(path).second) {
-        info->push_back(plugins_[i]);
+        info->push_back(*plugins_[i]);
         if (actual_mime_types)
           actual_mime_types->push_back(mime_type);
       }
     }
   }
 
   // Add the default plugin at the end if it supports the mime type given,
   // and the default plugin is enabled.
   if (!plugins_.empty() && webkit_glue::IsDefaultPluginEnabled()) {
-    const WebPluginInfo& default_info = plugins_.back();
+    const WebPluginInfo& default_info = *plugins_.back();
     if (SupportsType(default_info, mime_type, allow_wildcard)) {
       info->push_back(default_info);
       if (actual_mime_types)
         actual_mime_types->push_back(mime_type);
     }
   }
 }
 
 bool PluginList::GetPluginInfo(const GURL& url,
                                const std::string& mime_type,
@@ skipping 22 matching lines @@
     }
   }
   return false;
 }
 
 bool PluginList::GetPluginInfoByPath(const FilePath& plugin_path,
                                      WebPluginInfo* info) {
   LoadPlugins(false);
   AutoLock lock(lock_);
   for (size_t i = 0; i < plugins_.size(); ++i) {
-    if (plugins_[i].path == plugin_path) {
-      *info = plugins_[i];
+    if (plugins_[i]->path == plugin_path) {
+      *info = *plugins_[i];
       return true;
     }
   }
 
   return false;
 }
 
 void PluginList::GetPluginGroups(
     bool load_if_necessary,
     std::vector<PluginGroup>* plugin_groups) {
   if (load_if_necessary)
     LoadPlugins(false);
+  AutoLock lock(lock_);
   plugin_groups->clear();
   for (PluginGroup::PluginMap::const_iterator it = plugin_groups_.begin();
        it != plugin_groups_.end(); ++it) {
     plugin_groups->push_back(*it->second);
   }
 }
 
 const PluginGroup* PluginList::GetPluginGroup(
     const WebPluginInfo& web_plugin_info) {
   AutoLock lock(lock_);
-  return AddToPluginGroups(web_plugin_info);
+  return AddToPluginGroups(web_plugin_info, NULL);
 }
 
 string16 PluginList::GetPluginGroupName(std::string identifier) {
   PluginGroup::PluginMap::iterator it = plugin_groups_.find(identifier);
   if (it == plugin_groups_.end()) {
     return string16();
   }
   return it->second->GetGroupName();
 }
 
 std::string PluginList::GetPluginGroupIdentifier(
     const WebPluginInfo& web_plugin_info) {
   AutoLock lock(lock_);
-  PluginGroup* group = AddToPluginGroups(web_plugin_info);
+  PluginGroup* group = AddToPluginGroups(web_plugin_info, NULL);
   return group->identifier();
 }
 
 void PluginList::AddHardcodedPluginGroups() {
   AutoLock lock(lock_);
   const PluginGroupDefinition* definitions = GetPluginGroupDefinitions();
   for (size_t i = 0; i < GetPluginGroupDefinitionsSize(); ++i) {
     PluginGroup* definition_group = PluginGroup::FromPluginGroupDefinition(
         definitions[i]);
     std::string identifier = definition_group->identifier();
     DCHECK(plugin_groups_.find(identifier) == plugin_groups_.end());
     plugin_groups_.insert(std::make_pair(identifier, definition_group));
   }
 }
 
 PluginGroup* PluginList::AddToPluginGroups(
-    const WebPluginInfo& web_plugin_info) {
+    const WebPluginInfo& web_plugin_info,
+    std::vector<WebPluginInfo*>* plugins) {
   PluginGroup* group = NULL;
   for (PluginGroup::PluginMap::iterator it = plugin_groups_.begin();
        it != plugin_groups_.end(); ++it) {
     if (it->second->Match(web_plugin_info))
       group = it->second;
   }
   if (!group) {
     group = PluginGroup::FromWebPluginInfo(web_plugin_info);
     std::string identifier = group->identifier();
     // If the identifier is not unique, use the full path. This means that we
     // probably won't be able to search for this group by identifier, but at
     // least it's going to be in the set of plugin groups, and if there
     // is already a plug-in with the same filename, it's probably going to
     // handle the same MIME types (and it has a higher priority), so this one
     // is not going to run anyway.
     if (plugin_groups_.find(identifier) != plugin_groups_.end())
       identifier = PluginGroup::GetLongIdentifier(web_plugin_info);
     DCHECK(plugin_groups_.find(identifier) == plugin_groups_.end());
     plugin_groups_.insert(std::make_pair(identifier, group));
   }
-  group->AddPlugin(web_plugin_info, next_priority_++);
+  WebPluginInfo* group_plugin_copy;
+  bool is_new_addition = group->AddPlugin(
+      web_plugin_info, next_priority_, &group_plugin_copy);
+  if (plugins)
+    plugins->push_back(group_plugin_copy);
+  if (is_new_addition) {
+    next_priority_++;
+    string16 group_name = group->GetGroupName();
+    if (!group->Enabled() && !disabled_groups_.count(group_name)) {
+      disabled_groups_.insert(DisabledGroupsListElement(
+          group_name,
+          PluginGroup::IsPluginNameDisabledByPolicy(group_name) ?
+              POLICY : USER));
+    } else if (group->Enabled() && disabled_groups_.count(group_name)) {
+      disabled_groups_.erase(group_name);
+    }
+    // We don't need to protect the flag here because it is protected from the
+    // callers of |AddToPluginGroups|.
+  }
   return group;
 }
 
 bool PluginList::EnablePlugin(const FilePath& filename) {
   AutoLock lock(lock_);
 
   bool did_enable = false;
 
-  std::set<FilePath>::iterator entry = disabled_plugins_.find(filename);
-  if (entry == disabled_plugins_.end())
+  DisabledPlugins::iterator entry =
+      disabled_plugins_.find(filename);
+  if (entry == disabled_plugins_.end() || entry->second == POLICY)
     return did_enable;  // Early exit if plugin not in disabled list.
 
   disabled_plugins_.erase(entry);  // Remove from disabled list.
 
   // Set enabled flags if necessary.
-  for (std::vector<WebPluginInfo>::iterator it = plugins_.begin();
+  for (std::vector<WebPluginInfo*>::iterator it = plugins_.begin();
        it != plugins_.end();
        ++it) {
-    if (it->path == filename) {
-      DCHECK(!it->enabled);  // Should have been disabled.
-      it->enabled = true;
+    if ((*it)->path == filename) {
+      DCHECK(!(*it)->enabled);  // Should have been disabled.
+      (*it)->enabled = true;
+      PluginGroup* group = AddToPluginGroups(**it, NULL);
+      bool group_was_enabled = group->Enabled();
+      group->RefreshEnabledState();
+      if (group_was_enabled && group->Enabled()) {
+        DisabledGroups::iterator entry =
+            disabled_groups_.find(group->GetGroupName());
+        if (entry != disabled_groups_.end())
+          disabled_groups_.erase(entry);
+      }
       did_enable = true;
     }
   }
 
   return did_enable;
 }
 
-bool PluginList::DisablePlugin(const FilePath& filename) {
+bool PluginList::DisablePlugin(const FilePath& filename, bool policy_disabled) {
   AutoLock lock(lock_);
 
   bool did_disable = false;
 
-  if (disabled_plugins_.find(filename) != disabled_plugins_.end())
+  DisabledPlugins::iterator entry =
+      disabled_plugins_.find(filename);
+  if (entry != disabled_plugins_.end()) {
+    if ((entry->second == POLICY && !policy_disabled) ||
+        (entry->second == USER && policy_disabled))
+      entry->second = POLICY_AND_USER;
     return did_disable;  // Early exit if plugin already in disabled list.
+  }
 
-  disabled_plugins_.insert(filename);  // Add to disabled list.
+  // Add to disabled list.
+  disabled_plugins_.insert(
+      DisabledPluginsListElement(filename, policy_disabled ? POLICY : USER));
 
   // Unset enabled flags if necessary.
-  for (std::vector<WebPluginInfo>::iterator it = plugins_.begin();
+  for (std::vector<WebPluginInfo*>::iterator it = plugins_.begin();
        it != plugins_.end();
        ++it) {
-    if (it->path == filename) {
-      DCHECK(it->enabled);  // Should have been enabled.
-      it->enabled = false;
+    if ((*it)->path == filename) {
+      DCHECK((*it)->enabled);  // Should have been enabled.
+      (*it)->enabled = false;
+      PluginGroup* group = AddToPluginGroups(**it, NULL);
+      group->RefreshEnabledState();
+      if (!group->Enabled()) {
+        if (!disabled_groups_.count(group->GetGroupName())) {
+          disabled_groups_.insert(
+              DisabledGroupsListElement(group->GetGroupName(),
+                                        policy_disabled ? POLICY : USER));
+        }
+      }
       did_disable = true;
     }
   }
 
   return did_disable;
 }
 
 bool PluginList::EnableGroup(bool enable, const string16& group_name) {
   bool did_change = false;
   {
     AutoLock lock(lock_);
 
-    std::set<string16>::iterator entry = disabled_groups_.find(group_name);
+    DisabledGroups::iterator entry =
+        disabled_groups_.find(group_name);
     if (enable) {
-      if (entry == disabled_groups_.end())
+      if (entry == disabled_groups_.end() || entry->second == POLICY)
         return did_change;  // Early exit if group not in disabled list.
       disabled_groups_.erase(entry);  // Remove from disabled list.
     } else {
       if (entry != disabled_groups_.end())
         return did_change;  // Early exit if group already in disabled list.
-      disabled_groups_.insert(group_name);
+      disabled_groups_.insert(
+          DisabledGroupsListElement(
+              group_name,
+              PluginGroup::IsPluginNameDisabledByPolicy(group_name) ?
+                  POLICY : USER));
     }
   }
 
   for (PluginGroup::PluginMap::iterator it = plugin_groups_.begin();
        it != plugin_groups_.end(); ++it) {
     if (it->second->GetGroupName() == group_name) {
       if (it->second->Enabled() != enable) {
         it->second->Enable(enable);
         did_change = true;
         break;
       }
     }
   }
 
   return did_change;
 }
 
 void PluginList::DisableOutdatedPluginGroups() {
   disable_outdated_plugins_ = true;
 }
 
+const PluginList::DisabledPlugins&  PluginList::GetDisabledPlugins() const {
+  AutoLock lock(lock_);
+  return disabled_plugins_;
+}
+
+const PluginList::DisabledGroups& PluginList::GetDisabledGroups() const {
+  AutoLock lock(lock_);
+  return disabled_groups_;
+}
+
 PluginList::~PluginList() {
   Shutdown();
 }
 
 void PluginList::Shutdown() {
   // TODO
   // Note: plugin_groups_ contains simple pointers of type PluginGroup*, but
   // since this singleton lives until the process is destroyed, no explicit
   // cleanup is necessary.
   // However, when running on Valgrind, we need to do the cleanup to keep the
   // memory tree green.
 #if defined(OS_POSIX)
   if (RUNNING_ON_VALGRIND) {
     STLDeleteContainerPairSecondPointers(plugin_groups_.begin(),
                                          plugin_groups_.end());
   }
 #endif
 }
 
 }  // namespace NPAPI