Policy: Re-enabled plugin still disabled

webkit/plugins/npapi/plugin_list.cc

Index: webkit/plugins/npapi/plugin_list.cc
diff --git a/webkit/plugins/npapi/plugin_list.cc b/webkit/plugins/npapi/plugin_list.cc
index a54a122d19797b96b7a693ede6a1b78d05a0ff38..9e11aa19924ff28808b1d928a6c1cac28a3ed31d 100644
--- a/webkit/plugins/npapi/plugin_list.cc
+++ b/webkit/plugins/npapi/plugin_list.cc
@@ -14,7 +14,6 @@
 #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"
@@ -253,6 +252,7 @@ bool PluginList::CreateWebPluginInfo(const PluginVersionInfo& pvi,
   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;
@@ -285,20 +285,11 @@ 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.
@@ -358,31 +349,33 @@ void PluginList::LoadPlugins(bool refresh) {
     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);
-    }
   }
 
-  // Only update the data now since loading plugins can take a while.
   AutoLock lock(lock_);
-
-  plugins_ = new_plugins;
+  // 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);
+    }
+  }
   plugins_loaded_ = true;
 }
 
@@ -390,7 +383,6 @@ 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;
 
@@ -415,61 +407,22 @@ void PluginList::LoadPlugin(const FilePath& path,
     }
   }
 
-  // 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);
+  plugins->push_back(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,
@@ -478,11 +431,14 @@ void PluginList::GetEnabledPlugins(bool 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]);
+    }
   }
 }
 
@@ -505,15 +461,21 @@ void PluginList::GetPluginInfoArray(
 
   // 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);
+        }
       }
     }
   }
@@ -524,42 +486,63 @@ void PluginList::GetPluginInfoArray(
   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);
+      }
     }
   }
 }
@@ -597,10 +580,15 @@ 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;
+      }
     }
   }
 
@@ -612,6 +600,7 @@ void PluginList::GetPluginGroups(
     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) {
@@ -647,6 +636,7 @@ void PluginList::AddHardcodedPluginGroups() {
   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));
@@ -658,11 +648,14 @@ PluginGroup* PluginList::AddToPluginGroups(
   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
@@ -675,88 +668,60 @@ PluginGroup* PluginList::AddToPluginGroups(
     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.
+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);
+}
 
-  // 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;
-    }
+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);
   }
-
-  return did_enable;
+  // No such group yet; add one as a placeholder.
+  PluginGroup* group = AddPlaceholderToPluginGroups(filename, name);
+  return group->EnablePlugin(filename);
 }
 
-bool PluginList::DisablePlugin(const FilePath& filename) {
+bool PluginList::DisablePlugin(const FilePath& filename, const string16& name) {
   AutoLock lock(lock_);
-
-  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;
-    }
+  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() == group_name) {
-      if (it->second->Enabled() != enable) {
-        it->second->Enable(enable);
-        did_change = true;
-        break;
-      }
+    if (it->second->GetGroupName().find(group_name) != string16::npos) {
+      group = it->second;
+      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() {