Policy: Re-enabled plugin still disabled

webkit/plugins/npapi/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/plugins/npapi/plugin_list.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/string_split.h"
 #include "base/string_util.h"
 #include "base/sys_string_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "googleurl/src/gurl.h"
-#include "net/base/mime_util.h"
 #include "webkit/glue/webkit_glue.h"
 #include "webkit/plugins/npapi/plugin_constants_win.h"
 #include "webkit/plugins/npapi/plugin_lib.h"
 #include "webkit/plugins/plugin_switches.h"
 
 #if defined(OS_POSIX)
 #include "base/stl_util-inl.h"
 #include "base/third_party/valgrind/valgrind.h"
 #endif  // defined(OS_POSIX)
 
@@ skipping 218 matching lines @@
     LOG_IF(ERROR, PluginList::DebugPluginLoading())
         << "Plugin " << pvi.product_name << " has no MIME types, skipping";
     return false;
   }
 
   info->name = WideToUTF16(pvi.product_name);
   info->desc = WideToUTF16(pvi.file_description);
   info->version = WideToUTF16(pvi.file_version);
   info->path = pvi.path;
   info->enabled = true;
+  info->reason = WebPluginInfo::USER;
 
   for (size_t i = 0; i < mime_types.size(); ++i) {
     WebPluginMimeType mime_type;
     mime_type.mime_type = StringToLowerASCII(mime_types[i]);
     if (file_extensions.size() > i)
       base::SplitString(file_extensions[i], ',', &mime_type.file_extensions);
 
     if (descriptions.size() > i) {
       mime_type.description = descriptions[i];
 
@@ skipping 12 matching lines @@
     info->mime_types.push_back(mime_type);
   }
 
   return true;
 }
 
 PluginList::PluginList()
     : plugins_loaded_(false),
       plugins_need_refresh_(false),
       disable_outdated_plugins_(false),
-      next_priority_(0) {
+      next_priority_(1) {
   PlatformInit();
   AddHardcodedPluginGroups();
 }
 
-bool PluginList::ShouldDisableGroup(const string16& group_name) {
-  AutoLock lock(lock_);
-  if (PluginGroup::IsPluginNameDisabledByPolicy(group_name)) {
-    disabled_groups_.insert(group_name);
-    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;
@@ skipping 39 matching lines @@
 
 #if defined(OS_WIN)
   LoadPluginsFromRegistry(&new_plugins, &visited_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 (PluginGroup::IsPluginNameDisabledByPolicy(group_name))
+      group->EnableGroup(false);
 
-    if (disable_outdated_plugins_) {
+    if (disable_outdated_plugins_)
       group->DisableOutdatedPlugins();
-    }
-    if (!group->Enabled()) {
-      AutoLock lock(lock_);
-      disabled_groups_.insert(group_name);
+  }
+
+  AutoLock lock(lock_);
+  // Clean up the plugins that has disappeared from the groups.

[Bernhard Bauer, 2010/12/21 19:07:39]

Nit: have

[pastarmovj, 2010/12/23 13:00:19]

Done.

+  for (PluginGroup::PluginMap::const_iterator group = plugin_groups_.begin();
+       group != plugin_groups_.end(); ++group) {
+    std::vector<WebPluginInfo>& gr_plugins = group->second->GetPlugins();

[Bernhard Bauer, 2010/12/21 19:07:39]

Can you use a const reference here? If not, that is a sign that you're violating
encapsulation; you're basically directly mucking around with a member variable
of another object.

[pastarmovj, 2010/12/21 20:31:19]

I can and will do it no problem. I only read from the plugins.

[pastarmovj, 2010/12/23 13:00:19]

Done.

+    for (size_t i = 0; i < gr_plugins.size(); ++i) {
+      bool plugin_found = false;
+      for (size_t j = 0; j < new_plugins.size(); ++j)
+        if (gr_plugins[i].path == new_plugins[j].path) {
+          plugin_found = true;
+          break;
+        }
+      if (!plugin_found)
+        group->second->SetPluginIsPlaceholder(gr_plugins[i], true);
     }
   }
-
-  // Only update the data now since loading plugins can take a while.
-  AutoLock lock(lock_);
-
-  plugins_ = new_plugins;
   plugins_loaded_ = true;
 }
 
 void PluginList::LoadPlugin(const FilePath& path,
                             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))
     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_);
+  AddToPluginGroups(plugin_info);
   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;
-
-  for (size_t i = 0; i < info.mime_types.size(); ++i) {
-    const WebPluginMimeType& mime_info = info.mime_types[i];
-    if (net::MatchesMimeType(mime_info.mime_type, mime_type)) {
-      if (!allow_wildcard && mime_info.mime_type == "*") {
-        continue;
-      }
-      return true;
-    }
-  }
-  return false;
-}
-
-bool PluginList::SupportsExtension(const WebPluginInfo& info,
-                                   const std::string &extension,
-                                   std::string* actual_mime_type) {
-  for (size_t i = 0; i < info.mime_types.size(); ++i) {
-    const WebPluginMimeType& mime_type = info.mime_types[i];
-    for (size_t j = 0; j < mime_type.file_extensions.size(); ++j) {
-      if (mime_type.file_extensions[j] == extension) {
-        if (actual_mime_type)
-          *actual_mime_type = mime_type.mime_type;
-        return true;
-      }
-    }
-  }
-
-  return false;
-}
-
-
 void PluginList::GetPlugins(bool refresh, std::vector<WebPluginInfo>* plugins) {
   LoadPlugins(refresh);
 
   AutoLock lock(lock_);
-  *plugins = plugins_;
+  for (PluginGroup::PluginMap::const_iterator group = plugin_groups_.begin();
+       group != plugin_groups_.end(); ++group) {
+    const std::vector<WebPluginInfo>& gr_plugins = group->second->GetPlugins();
+    for (size_t i = 0; i < gr_plugins.size(); ++i)
+      if (!group->second->IsPluginPlaceholder(gr_plugins[i]))
+        plugins->push_back(gr_plugins[i]);
+  }
 }
 
 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();
-       it != plugins_.end();
-       ++it) {
-    if (it->enabled)
-      plugins->push_back(*it);
+  for (PluginGroup::PluginMap::const_iterator group = plugin_groups_.begin();
+       group != plugin_groups_.end(); ++group) {
+    const std::vector<WebPluginInfo>& gr_plugins = group->second->GetPlugins();
+    for (size_t i = 0; i < gr_plugins.size(); ++i) {
+      if (!group->second->IsPluginPlaceholder(gr_plugins[i]) &&
+          WebPluginInfoUtils::IsEnabled(gr_plugins[i]))
+        plugins->push_back(gr_plugins[i]);
+    }
   }
 }
 
 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 (path.value() != kDefaultPluginLibraryName &&
-          visited_plugins.insert(path).second) {
-        info->push_back(plugins_[i]);
-        if (actual_mime_types)
-          actual_mime_types->push_back(mime_type);
+  for (PluginGroup::PluginMap::const_iterator group = plugin_groups_.begin();
+       group != plugin_groups_.end(); ++group) {
+    const std::vector<WebPluginInfo>& plugins = group->second->GetPlugins();
+    for (size_t i = 0; i < plugins.size(); ++i) {
+      if (!group->second->IsPluginPlaceholder(plugins[i]) &&
+          WebPluginInfoUtils::IsEnabled(plugins[i]) &&
+          WebPluginInfoUtils::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]);
+          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 (path.value() != kDefaultPluginLibraryName &&
-            visited_plugins.insert(path).second) {
-          info->push_back(plugins_[i]);
-          if (actual_mime_types)
-            actual_mime_types->push_back(actual_mime_type);
+    for (PluginGroup::PluginMap::const_iterator group = plugin_groups_.begin();
+         group != plugin_groups_.end(); ++group) {
+      const std::vector<WebPluginInfo>& plugins = group->second->GetPlugins();
+      for (size_t i = 0; i < plugins.size(); ++i) {
+        if (!group->second->IsPluginPlaceholder(plugins[i]) &&
+            WebPluginInfoUtils::IsEnabled(plugins[i]) &&
+            WebPluginInfoUtils::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]);
+            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 (path.value() != kDefaultPluginLibraryName &&
-          visited_plugins.insert(path).second) {
-        info->push_back(plugins_[i]);
-        if (actual_mime_types)
-          actual_mime_types->push_back(mime_type);
+  for (PluginGroup::PluginMap::const_iterator group = plugin_groups_.begin();
+       group != plugin_groups_.end(); ++group) {
+    const std::vector<WebPluginInfo>& plugins = group->second->GetPlugins();
+    for (size_t i = 0; i < plugins.size(); ++i) {
+      if (!group->second->IsPluginPlaceholder(plugins[i]) &&
+          !WebPluginInfoUtils::IsEnabled(plugins[i]) &&
+          WebPluginInfoUtils::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]);
+          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();
-    if (SupportsType(default_info, mime_type, allow_wildcard)) {
-      info->push_back(default_info);
-      if (actual_mime_types)
-        actual_mime_types->push_back(mime_type);
+  for (PluginGroup::PluginMap::const_iterator group = plugin_groups_.begin();
+       group != plugin_groups_.end(); ++group) {
+#if defined(OS_WIN)
+    if (group->first.compare(WideToUTF8(kDefaultPluginLibraryName)) == 0) {
+#else
+    if (group->first.compare(kDefaultPluginLibraryName) == 0) {
+#endif
+      DCHECK_NE(0U, group->second->GetPlugins().size());
+      const WebPluginInfo& default_info = group->second->GetPlugins().front();
+      if (WebPluginInfoUtils::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,
                                bool allow_wildcard,
                                WebPluginInfo* info,
                                std::string* actual_mime_type) {
   DCHECK(info);
@@ skipping 17 matching lines @@
       return true;
     }
   }
   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];
-      return true;
+  for (PluginGroup::PluginMap::const_iterator group = plugin_groups_.begin();
+       group != plugin_groups_.end(); ++group) {
+    const std::vector<WebPluginInfo>& plugins = group->second->GetPlugins();
+    for (size_t i = 0; i < plugins.size(); ++i) {
+      if (!group->second->IsPluginPlaceholder(plugins[i]) &&
+          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) {
     if (!it->second->IsEmpty())
       plugin_groups->push_back(*it->second);
   }
 }
 
 const PluginGroup* PluginList::GetPluginGroup(
     const WebPluginInfo& web_plugin_info) {
@@ skipping 15 matching lines @@
   PluginGroup* group = AddToPluginGroups(web_plugin_info);
   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]);
+    ProcessGroupAfterInitialize(definition_group);
     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) {
   PluginGroup* group = NULL;
   for (PluginGroup::PluginMap::iterator it = plugin_groups_.begin();
        it != plugin_groups_.end(); ++it) {
-    if (it->second->Match(web_plugin_info))
+    if (it->second->Match(web_plugin_info)) {
       group = it->second;
+      break;
+    }
   }
   if (!group) {
     group = PluginGroup::FromWebPluginInfo(web_plugin_info);
+    ProcessGroupAfterInitialize(group);
     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_++);
+  bool is_new_addition = group->AddPlugin(web_plugin_info, next_priority_);
+  if (is_new_addition)
+    next_priority_++;
   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())
-    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();
-       it != plugins_.end();
-       ++it) {
-    if (it->path == filename) {
-      DCHECK(!it->enabled);  // Should have been disabled.
-      it->enabled = true;
-      did_enable = true;
-    }
-  }
-
-  return did_enable;
+PluginGroup* PluginList::AddPlaceholderToPluginGroups(
+    const FilePath& filename, const string16& name) {
+  WebPluginInfo plugin_info;

[jam, 2010/12/21 19:57:42]

if I understand correctly, AddPlaceholderToPluginGroups is used to allow
disabling a plugin based on its filename, before we have the list of groups
ready, right?  If that's the case, do we really need to call AddToPluginGroups? 
why can't we just keep a vector of disabled FilePaths, and check those when
creating the real plugin groups?  it seems that this would simplify things a
little since we wouldn't need |name| or empty groups?

[pastarmovj, 2010/12/21 20:31:19]

It is not so simple to keep this list of disabled plugins in sync every time
they change. which happens from the UI/policy changes/(and soon file appearing
and disappering). The code used to do that before and it failed because the
connections were not that simple at all - a lot of possible cause-effects can
occur that were too difficult to enumerate and change all too often. The
implementation now hides this complexity at much lower price than the code
needed before to keep such an array for plugins _and_ groups.

+  plugin_info.path = filename;
+  plugin_info.name = name;
+  return AddToPluginGroups(plugin_info);
 }
 
-bool PluginList::DisablePlugin(const FilePath& filename) {
+bool PluginList::EnablePlugin(const FilePath& filename, const string16& name) {
   AutoLock lock(lock_);
+  for (PluginGroup::PluginMap::iterator it = plugin_groups_.begin();
+       it != plugin_groups_.end(); ++it) {
+    if (it->second->ContainsPlugin(filename))
+      return it->second->EnablePlugin(filename);
+  }
+  // No such group yet; add one as a placeholder.
+  PluginGroup* group = AddPlaceholderToPluginGroups(filename, name);
+  return group->EnablePlugin(filename);
+}
 
-  bool did_disable = false;
-
-  if (disabled_plugins_.find(filename) != disabled_plugins_.end())
-    return did_disable;  // Early exit if plugin already in disabled list.
-
-  disabled_plugins_.insert(filename);  // Add to disabled list.
-
-  // Unset enabled flags if necessary.
-  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;
-      did_disable = true;
-    }
+bool PluginList::DisablePlugin(const FilePath& filename, const string16& name) {
+  AutoLock lock(lock_);
+  for (PluginGroup::PluginMap::iterator it = plugin_groups_.begin();
+       it != plugin_groups_.end(); ++it) {
+    if (it->second->ContainsPlugin(filename))
+      return it->second->DisablePlugin(filename);
   }
-
-  return did_disable;
+  // No such group yet; add one as a placeholder.
+  PluginGroup* group = AddPlaceholderToPluginGroups(filename, name);
+  return group->DisablePlugin(filename);
 }
 
 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);
-    if (enable) {
-      if (entry == disabled_groups_.end())
-        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);
+  AutoLock lock(lock_);
+  PluginGroup* group = NULL;
+  for (PluginGroup::PluginMap::iterator it = plugin_groups_.begin();
+       it != plugin_groups_.end(); ++it) {
+    if (it->second->GetGroupName().find(group_name) != string16::npos) {
+      group = it->second;
+      break;
     }
   }
-
-  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;
-      }
-    }
+  if (!group) {
+    group = PluginGroup::CreateEmptyGroup(group_name);
+    plugin_groups_.insert(std::make_pair(UTF16ToUTF8(group_name), group));
   }
 
-  return did_change;
+  return group->EnableGroup(enable);
 }
 
 void PluginList::DisableOutdatedPluginGroups() {
   disable_outdated_plugins_ = true;
 }
 
 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
 }  // namespace webkit