4 files changed, 89 insertions, 58 deletions

diff --git a/webkit/plugins/npapi/plugin_group.cc b/webkit/plugins/npapi/plugin_group.cc
index 7d45aa7..1bf6284 100644
--- a/webkit/plugins/npapi/plugin_group.cc
+++ b/webkit/plugins/npapi/plugin_group.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -239,52 +239,49 @@ void PluginGroup::AddPlugin(const WebPluginInfo& plugin) {
     }
   }
   web_plugin_infos_.push_back(plugin);
-  // If the group is disabled disable new plugins in it too. This should cover
-  // the case where a plugin has been upgraded that has been disabled and should
-  // stay disabled after the upgrade.
-  if (!enabled_)
-    DisablePlugin(web_plugin_infos_.back().path);
   UpdateActivePlugin(web_plugin_infos_.back());
-  RefreshEnabledState();
 }
 
 bool PluginGroup::RemovePlugin(const FilePath& filename) {
+  bool did_remove = false;
+  ResetGroupState();
   for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
     if (web_plugin_infos_[i].path == filename) {
       web_plugin_infos_.erase(web_plugin_infos_.begin() + i);
-      return true;
+      did_remove = true;
     }
+    UpdateActivePlugin(web_plugin_infos_[i]);
   }
-  return false;
+  return did_remove;
 }
 
 bool PluginGroup::EnablePlugin(const FilePath& filename) {
+  bool did_enable = false;
+  ResetGroupState();
   for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
-    if (web_plugin_infos_[i].path == filename) {
-      bool did_enable = Enable(&web_plugin_infos_[i],
-                               WebPluginInfo::USER_ENABLED);
-      RefreshEnabledState();
-      return did_enable;
-    }
+    if (web_plugin_infos_[i].path == filename)
+      did_enable = Enable(&web_plugin_infos_[i], WebPluginInfo::USER_ENABLED);
+    UpdateActivePlugin(web_plugin_infos_[i]);
   }
-  return false;
+  return did_enable;
 }
 
 bool PluginGroup::DisablePlugin(const FilePath& filename) {
+  bool did_disable = false;
+  ResetGroupState();
   for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
     if (web_plugin_infos_[i].path == filename) {
       // We are only called for user intervention however we should respect a
       // policy that might as well be active on this plugin.
-      bool did_disable = Disable(
+      did_disable = Disable(
           &web_plugin_infos_[i],
           IsPluginNameDisabledByPolicy(web_plugin_infos_[i].name) ?
               WebPluginInfo::USER_DISABLED_POLICY_DISABLED :
               WebPluginInfo::USER_DISABLED);
-      RefreshEnabledState();
-      return did_disable;
     }
+    UpdateActivePlugin(web_plugin_infos_[i]);
   }
-  return false;
+  return did_disable;
 }
 
 string16 PluginGroup::GetGroupName() const {
@@ -422,56 +419,49 @@ bool PluginGroup::IsEmpty() const {
 }
 
 void PluginGroup::DisableOutdatedPlugins() {
-  bool first_enabled = true;
-
+  ResetGroupState();
   for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
     scoped_ptr<Version> version(
         CreateVersionFromString(web_plugin_infos_[i].version));
     if (version.get()) {
-      bool plugin_is_outdated = false;
       for (size_t j = 0; j < version_ranges_.size(); ++j) {
         if (IsPluginOutdated(*version, version_ranges_[j])) {
           Disable(&web_plugin_infos_[i], WebPluginInfo::USER_DISABLED);
-          plugin_is_outdated = true;
           break;
         }
       }
-      if (!plugin_is_outdated && first_enabled) {
-        first_enabled = false;
-        UpdateDescriptionAndVersion(web_plugin_infos_[i]);
-      }
     }
+    UpdateActivePlugin(web_plugin_infos_[i]);
   }
 }
 
 bool PluginGroup::EnableGroup(bool enable) {
-  bool enabled_plugin_exists = false;
   bool group_disabled_by_policy = IsPluginNameDisabledByPolicy(group_name_);
   // We can't enable groups disabled by policy
   if (group_disabled_by_policy && enable)
     return false;
 
+  ResetGroupState();
   for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
     bool policy_disabled =
         IsPluginNameDisabledByPolicy(web_plugin_infos_[i].name);
     if (enable && !policy_disabled) {
       Enable(&web_plugin_infos_[i], WebPluginInfo::USER_ENABLED);
-      enabled_plugin_exists = true;
     } else {
       Disable(&web_plugin_infos_[i],
               policy_disabled || group_disabled_by_policy ?
                   WebPluginInfo::POLICY_DISABLED :
                   WebPluginInfo::USER_DISABLED);
     }
+    UpdateActivePlugin(web_plugin_infos_[i]);
   }
-  enabled_ = enabled_plugin_exists;
   return enabled_ == enable;
 }
 
 void PluginGroup::EnforceGroupPolicy() {
-  bool enabled_plugin_exists = false;
   bool group_disabled_by_policy = IsPluginNameDisabledByPolicy(group_name_);
 
+  ResetGroupState();
   for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
     bool policy_disabled =
         IsPluginNameDisabledByPolicy(web_plugin_infos_[i].name) |
@@ -483,22 +473,15 @@ void PluginGroup::EnforceGroupPolicy() {
     // ...here would a else if (policy_enabled) { ... } be then.
     } else {
       Enable(&web_plugin_infos_[i], WebPluginInfo::POLICY_UNMANAGED);
-      if (IsPluginEnabled(web_plugin_infos_[i]))
-        enabled_plugin_exists = true;
     }
+    UpdateActivePlugin(web_plugin_infos_[i]);
   }
-  enabled_ = enabled_plugin_exists;
 }
 
-void PluginGroup::RefreshEnabledState() {
-  bool enabled_plugin_exists = false;
-  for (size_t i = 0; i < web_plugin_infos_.size(); ++i) {
-    if (IsPluginEnabled(web_plugin_infos_[i])) {
-      enabled_plugin_exists = true;
-      break;
-    }
-  }
-  enabled_ = enabled_plugin_exists;
+void PluginGroup::ResetGroupState() {
+  enabled_ = false;
+  description_.clear();
+  version_.reset();
 }
 
 bool PluginGroup::Enable(WebPluginInfo* plugin,
diff --git a/webkit/plugins/npapi/plugin_group.h b/webkit/plugins/npapi/plugin_group.h
index c1e77b8..9e0250c 100644
--- a/webkit/plugins/npapi/plugin_group.h
+++ b/webkit/plugins/npapi/plugin_group.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -232,15 +232,17 @@ class PluginGroup {
   // enabled one, or if all plugins are disabled, simply the first one.
   void UpdateActivePlugin(const WebPluginInfo& plugin);
 
-  // Refreshes the enabled flag based on the state of its plugins.
-  void RefreshEnabledState();
+  // Resets the group state to its default value (as if the group was empty).
+  // After calling this method, calling |UpdateActivePlugin| with all plugins
+  // in a row will correctly set the group state.
+  void ResetGroupState();
 
   // Enables the plugin if not already enabled and if policy allows it to.
-  // Returns true on success.
+  // Returns true on success. Does not update the group state.
   static bool Enable(WebPluginInfo* plugin, int reason);
 
   // Disables the plugin if not already disabled and if policy allows it to.
-  // Returns true on success.
+  // Returns true on success. Does not update the group state.
   static bool Disable(WebPluginInfo* plugin, int reason);
 
   // Returns a non-const vector of all plugins in the group. This is only used
diff --git a/webkit/plugins/npapi/plugin_group_unittest.cc b/webkit/plugins/npapi/plugin_group_unittest.cc
index d83e2cd..3d37786 100644
--- a/webkit/plugins/npapi/plugin_group_unittest.cc
+++ b/webkit/plugins/npapi/plugin_group_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -49,25 +49,25 @@ static const PluginGroupDefinition kPluginDefNotVulnerable = {
     "myplugin-latest", "MyPlugin", "MyPlugin", NULL, 0, "http://latest" };
 
 // name, path, version, desc.
-static WebPluginInfo kPluginNoVersion = WebPluginInfo(
+static const WebPluginInfo kPluginNoVersion = WebPluginInfo(
     ASCIIToUTF16("MyPlugin"), FilePath(FILE_PATH_LITERAL("myplugin.so.2.0.43")),
     ASCIIToUTF16(""), ASCIIToUTF16("MyPlugin version 2.0.43"));
-static WebPluginInfo kPlugin2043 = WebPluginInfo(
+static const WebPluginInfo kPlugin2043 = WebPluginInfo(
     ASCIIToUTF16("MyPlugin"), FilePath(FILE_PATH_LITERAL("myplugin.so.2.0.43")),
     ASCIIToUTF16("2.0.43"), ASCIIToUTF16("MyPlugin version 2.0.43"));
-static WebPluginInfo kPlugin3043 = WebPluginInfo(
+static const WebPluginInfo kPlugin3043 = WebPluginInfo(
     ASCIIToUTF16("MyPlugin"), FilePath(FILE_PATH_LITERAL("myplugin.so.3.0.43")),
     ASCIIToUTF16("3.0.43"), ASCIIToUTF16("MyPlugin version 3.0.43"));
-static WebPluginInfo kPlugin3044 = WebPluginInfo(
+static const WebPluginInfo kPlugin3044 = WebPluginInfo(
     ASCIIToUTF16("MyPlugin"), FilePath(FILE_PATH_LITERAL("myplugin.so.3.0.44")),
     ASCIIToUTF16("3.0.44"), ASCIIToUTF16("MyPlugin version 3.0.44"));
-static WebPluginInfo kPlugin3045 = WebPluginInfo(
+static const WebPluginInfo kPlugin3045 = WebPluginInfo(
     ASCIIToUTF16("MyPlugin"), FilePath(FILE_PATH_LITERAL("myplugin.so.3.0.45")),
      ASCIIToUTF16("3.0.45"), ASCIIToUTF16("MyPlugin version 3.0.45"));
-static WebPluginInfo kPlugin3045r = WebPluginInfo(
+static const WebPluginInfo kPlugin3045r = WebPluginInfo(
     ASCIIToUTF16("MyPlugin"), FilePath(FILE_PATH_LITERAL("myplugin.so.3.0.45")),
      ASCIIToUTF16("3.0r45"), ASCIIToUTF16("MyPlugin version 3.0r45"));
-static WebPluginInfo kPlugin4043 = WebPluginInfo(
+static const WebPluginInfo kPlugin4043 = WebPluginInfo(
     ASCIIToUTF16("MyPlugin"), FilePath(FILE_PATH_LITERAL("myplugin.so.4.0.43")),
      ASCIIToUTF16("4.0.43"), ASCIIToUTF16("MyPlugin version 4.0.43"));
 
@@ -266,5 +266,26 @@ TEST(PluginGroupTest, IsVulnerable) {
   EXPECT_TRUE(PluginGroup(*group).RequiresAuthorization());
 }
 
+TEST(PluginGroupTest, MultipleVersions) {
+  scoped_ptr<PluginGroup> group(
+      PluginGroupTest::CreatePluginGroup(kPluginDef3));
+  group->AddPlugin(kPlugin3044);
+  group->AddPlugin(kPlugin3043);
+  EXPECT_EQ(kPlugin3044.desc, group->description());
+  EXPECT_FALSE(group->IsVulnerable());
+
+  group->DisablePlugin(kPlugin3044.path);
+  EXPECT_EQ(kPlugin3043.desc, group->description());
+  EXPECT_TRUE(group->IsVulnerable());
+
+  EXPECT_TRUE(group->EnableGroup(false));
+  EXPECT_EQ(kPlugin3044.desc, group->description());
+  EXPECT_FALSE(group->IsVulnerable());
+
+  EXPECT_TRUE(group->RemovePlugin(kPlugin3044.path));
+  EXPECT_EQ(kPlugin3043.desc, group->description());
+  EXPECT_TRUE(group->IsVulnerable());
+}
+
 }  // namespace npapi
 }  // namespace webkit
diff --git a/webkit/plugins/npapi/plugin_list_unittest.cc b/webkit/plugins/npapi/plugin_list_unittest.cc
index acb6c87..2420120 100644
--- a/webkit/plugins/npapi/plugin_list_unittest.cc
+++ b/webkit/plugins/npapi/plugin_list_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -40,6 +40,8 @@ class PluginListWithoutFileIO : public PluginList {
   }
 
   virtual const PluginGroupDefinition* GetPluginGroupDefinitions() OVERRIDE {
+    if (hardcoded_definitions_.empty())
+      return NULL;
     return &hardcoded_definitions_.front();
   }
   virtual size_t GetPluginGroupDefinitionsSize() OVERRIDE {
@@ -261,5 +263,28 @@ TEST_F(PluginListTest, HardcodedGroups) {
   EXPECT_EQ("bar.plugin", groups[1].identifier());
 }
 
+TEST_F(PluginListTest, UpdatedPlugin) {
+  // Test that a disabled plugin stays disabled when it's updated between
+  // browser restarts, i.e. its path and version change, but it stays in the
+  // same plugin group.
+
+  // Create a pristine PluginList.
+  plugin_test_internal::PluginListWithoutFileIO plugin_list;
+
+  // The preferences will mark the "MyPlugin" group as disabled.
+  string16 myPluginName(ASCIIToUTF16("MyPlugin"));
+  EXPECT_TRUE(plugin_list.EnableGroup(false, myPluginName));
+
+  WebPluginInfo plugin_3045(myPluginName,
+                            FilePath(FILE_PATH_LITERAL("/myplugin.3.0.45")),
+                            ASCIIToUTF16("3.0.45"),
+                            ASCIIToUTF16("MyPlugin version 3.0.45"));
+  plugin_list.AddPluginToLoad(plugin_3045);
+  std::vector<WebPluginInfo> plugins;
+  plugin_list.GetPlugins(true, &plugins);
+  ASSERT_EQ(1u, plugins.size());
+  ASSERT_EQ(WebPluginInfo::USER_DISABLED_POLICY_UNMANAGED, plugins[0].enabled);
+}
+
 }  // namespace npapi
 }  // namespace webkit