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 148 matching lines @@
 }
 
 bool PluginList::PluginsLoaded() {
   AutoLock lock(lock_);
   return plugins_loaded_;
 }
 
 void PluginList::RefreshPlugins() {
   AutoLock lock(lock_);
   plugins_need_refresh_ = true;
+  plugins_vector_is_dirty_ = true;
 }
 
 void PluginList::AddExtraPluginPath(const FilePath& plugin_path) {
   // Chrome OS only loads plugins from /opt/google/chrome/plugins.
 #if !defined(OS_CHROMEOS)
   AutoLock lock(lock_);
   extra_plugin_paths_.push_back(plugin_path);
 #endif
 }
 
@@ skipping 94 matching lines @@
 
     info->mime_types.push_back(mime_type);
   }
 
   return true;
 }
 
 PluginList::PluginList()
     : plugins_loaded_(false),
       plugins_need_refresh_(false),
+      plugins_vector_is_dirty_(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::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)
       continue;
-    LoadPlugin(internal_plugins[i].path, &new_plugins);
+    LoadPlugin(internal_plugins[i].path);
   }
 
   for (size_t i = 0; i < extra_plugin_paths.size(); ++i) {
     const FilePath& path = extra_plugin_paths[i];
     if (visited_plugins.find(path) != visited_plugins.end())
       continue;
-    LoadPlugin(path, &new_plugins);
+    LoadPlugin(path);
     visited_plugins.insert(path);
   }
 
   for (size_t i = 0; i < extra_plugin_dirs.size(); ++i) {
-    LoadPluginsFromDir(extra_plugin_dirs[i], &new_plugins, &visited_plugins);
+    LoadPluginsFromDir(extra_plugin_dirs[i], &visited_plugins);
   }
 
   for (size_t i = 0; i < directories_to_scan.size(); ++i) {
-    LoadPluginsFromDir(directories_to_scan[i], &new_plugins, &visited_plugins);
+    LoadPluginsFromDir(directories_to_scan[i], &visited_plugins);
   }
 
 #if defined(OS_WIN)
   LoadPluginsFromRegistry(&new_plugins, &visited_plugins);
 #endif
 
   // Load the default plugin last.
   if (webkit_glue::IsDefaultPluginEnabled())
-    LoadPlugin(FilePath(kDefaultPluginLibraryName), &new_plugins);
+    LoadPlugin(FilePath(kDefaultPluginLibraryName));
 
   // 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(0 == disabled_groups_.count(group_name))

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

use == 0 formatting (reverse order). compiler gives warning/error on inadvertent
assignment. 

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

Done.

+        disabled_groups_.insert(DisabledGroupsListElement(group_name, USER));
     }
   }
 
   // Only update the data now since loading plugins can take a while.
+  RebuildPluginsList();

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

You also acquire the lock inside of RebuildPluginList. Do you really need to
re-acquire it below?

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

I have to for the plugins_loaded_ flag. However this code has/will change anyway
and RebuildPluginsList won't get called.

+  //plugins_ = new_plugins;
   AutoLock lock(lock_);
-
-  plugins_ = new_plugins;
   plugins_loaded_ = true;
 }
 
-void PluginList::LoadPlugin(const FilePath& path,
-                            std::vector<WebPluginInfo>* plugins) {
+void PluginList::RebuildPluginsList() {
+  AutoLock lock(lock_);
+  if(!plugins_vector_is_dirty_) return;
+  plugins_vector_is_dirty_ = false;
+
+  std::vector<WebPluginInfo*> new_plugins;
+  new_plugins.resize(this->next_priority_);
+  for (PluginGroup::PluginMap::iterator it = plugin_groups_.begin();
+       it != plugin_groups_.end(); ++it) {
+    std::vector<WebPluginInfo>& group_plugins = it->second->GetPlugins();
+    std::vector<int>::iterator itprio =
+        it->second->GetPluginPositions().begin();
+    for (std::vector<WebPluginInfo>::iterator itp = group_plugins.begin();
+         itp != group_plugins.end();
+         ++itp,++itprio) {
+      new_plugins[*itprio] = &(*itp);

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

Whoa. Taking the address of a dereferenced iterator makes my eyeballs itch, and
has an odd "smell". Are you sure that the lifecycles of the WebPluginInfos in
group_plugins outlive plugns_ always? If so, it needs to be clearly documented
in the the header for the plugin_ member. I suspect there was a good reason for
copying them in the old version.

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

Cool right ;)

+    }
+  }
+  plugins_ = new_plugins;
+}
+
+void PluginList::LoadPlugin(const FilePath& path) {

[Bernhard Bauer, 2010/12/14 04:21:49]

Drive-by: Removing the |plugins| parameter may seem like a good idea, but it's
not. 

As your CL stands right now, if multiple threads call  LoadPlugins at the same
time, we will possibly end up with duplicates in the plugin list. We don't use a
lock around the whole LoadPlugins method for performance reasons, so we
construct the new plugins array separately, and then only swap it out at the end
(hence the extra |plugins| parameter).

Jakob, for AddToPluginGroups we should probably add an additional parameter as
well. Sorry I didn't catch that in the first CL.

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

Done.

   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))
+  if (!ShouldLoadPlugin(plugin_info))
     return;
 
   if (path.value() != kDefaultPluginLibraryName
 #if defined(OS_WIN) && !defined(NDEBUG)
       && path.BaseName().value() != L"npspy.dll"  // Make an exception for NPSPY
 #endif
       ) {
     for (size_t i = 0; i < plugin_info.mime_types.size(); ++i) {
       // TODO: don't load global handlers for now.
       // WebKit hands to the Plugin before it tries
       // to handle mimeTypes on its own.
       const std::string &mime_type = plugin_info.mime_types[i].mime_type;
       if (mime_type == "*" )
         return;
     }
   }
 
   // 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);
 }
 
 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;
 
@@ skipping 24 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) {
@@ skipping 54 matching lines @@
     // 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_++);
+  if(group->AddPlugin(web_plugin_info, next_priority_++)){
+    // We don't need to protect the flag here because it is protected from the
+    // callers of |AddToPluginGroups|.
+    plugins_vector_is_dirty_ = true;
+  }
   return group;
 }
 
 bool PluginList::EnablePlugin(const FilePath& filename) {
+  RebuildPluginsList();
   AutoLock lock(lock_);
 
   bool did_enable = false;
 
-  std::set<FilePath>::iterator entry = disabled_plugins_.find(filename);
-  if (entry == disabled_plugins_.end())
+  DisabledPluginsList::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;
+      AddToPluginGroups(**it)->RefreshEnabledState();
       did_enable = true;
     }
   }
 
   return did_enable;
 }
 
-bool PluginList::DisablePlugin(const FilePath& filename) {
+bool PluginList::DisablePlugin(const FilePath& filename, bool policy_disabled) {
+  RebuildPluginsList();
   AutoLock lock(lock_);
 
   bool did_disable = false;
 
-  if (disabled_plugins_.find(filename) != disabled_plugins_.end())
+  DisabledPluginsList::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;
+      AddToPluginGroups(**it)->RefreshEnabledState();
       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);
+    DisabledGroupsList::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;
 }
 
+void PluginList::GetDisabledPlugins(DisabledPluginsList* disabled_plugins) {
+  AutoLock lock(lock_);
+  *disabled_plugins = disabled_plugins_;
+}
+
+void PluginList::GetDisabledGroups(DisabledGroupsList* disabled_groups) {
+  AutoLock lock(lock_);
+  *disabled_groups = 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