Display the description of the active plugin in a plugin group.

The active plug-in is the one that would actually be loaded, i.e. the first enabled plug-in, or if all plugins are disabled, the very first plug-in in the group.

BUG=48572,51728
TEST=PluginGroupTest.PluginGroupDescription

Review URL: http://codereview.chromium.org/3142004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@55732 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 98 insertions, 39 deletions

diff --git a/chrome/common/plugin_group.cc b/chrome/common/plugin_group.cc
index fb8e251..5c9814e 100644
--- a/chrome/common/plugin_group.cc
+++ b/chrome/common/plugin_group.cc
@@ -136,7 +136,7 @@ PluginGroup::PluginGroup(const string16& group_name,
   }
   update_url_ = update_url;
   enabled_ = false;
-  max_version_.reset(Version::GetVersionFromString("0"));
+  version_.reset(Version::GetVersionFromString("0"));
 }
 
 PluginGroup* PluginGroup::FromPluginGroupDefinition(
@@ -225,18 +225,10 @@ bool PluginGroup::Match(const WebPluginInfo& plugin) const {
           version_range_high_->CompareTo(*plugin_version) > 0);
 }
 
-void PluginGroup::AddPlugin(const WebPluginInfo& plugin, int position) {
-  web_plugin_infos_.push_back(plugin);
-  // The position of this plugin relative to the global list of plugins.
-  web_plugin_positions_.push_back(position);
+void PluginGroup::UpdateDescriptionAndVersion(const WebPluginInfo& plugin) {
   description_ = plugin.desc;
 
-  // A group is enabled if any of the files are enabled.
-  if (plugin.enabled) {
-    enabled_ = true;
-  }
-
-  // update max_version_. Remove spaces and ')' from the version string,
+  // Update |version_|. Remove spaces and ')' from the version string,
   // Replace any instances of 'r', ',' or '(' with a dot.
   std::wstring version = UTF16ToWide(plugin.version);
   RemoveChars(version, L") ", &version);
@@ -244,12 +236,29 @@ void PluginGroup::AddPlugin(const WebPluginInfo& plugin, int position) {
   std::replace(version.begin(), version.end(), ',', '.');
   std::replace(version.begin(), version.end(), '(', '.');
 
-  scoped_ptr<Version> plugin_version(
-      Version::GetVersionFromString(version));
-  if (plugin_version.get() != NULL) {
-    if (plugin_version->CompareTo(*(max_version_)) > 0) {
-      max_version_.reset(plugin_version.release());
+  if (Version* new_version = Version::GetVersionFromString(version))
+    version_.reset(new_version);
+  else
+    version_.reset(Version::GetVersionFromString("0"));
+}
+
+void PluginGroup::AddPlugin(const WebPluginInfo& plugin, int position) {
+  web_plugin_infos_.push_back(plugin);
+  // The position of this plugin relative to the global list of plugins.
+  web_plugin_positions_.push_back(position);
+
+  // A group is enabled if any of the files are enabled.
+  if (plugin.enabled) {
+    if (!enabled_) {
+      // If this is the first enabled plugin, use its description.
+      enabled_ = true;
+      UpdateDescriptionAndVersion(plugin);
     }
+  } else {
+    // If this is the first plugin and it's disabled,
+    // use its description for now.
+    if (description_.empty())
+      UpdateDescriptionAndVersion(plugin);
   }
 }
 
@@ -264,7 +273,7 @@ DictionaryValue* PluginGroup::GetDataForUI() const {
   DictionaryValue* result = new DictionaryValue();
   result->SetString("name", group_name_);
   result->SetString("description", description_);
-  result->SetString("version", max_version_->GetString());
+  result->SetString("version", version_->GetString());
   result->SetString("update_url", update_url_);
   result->SetBoolean("critical", IsVulnerable());
 
@@ -290,7 +299,7 @@ DictionaryValue* PluginGroup::GetDataForUI() const {
       plugin_file->SetString("enabledMode", "disabledByPolicy");
     } else {
       plugin_file->SetString("enabledMode",
-                        web_plugin.enabled ? "enabled" : "disabledByUser");
+                             web_plugin.enabled ? "enabled" : "disabledByUser");
     }
     plugin_file->SetInteger("priority", priority);
 
@@ -325,10 +334,10 @@ DictionaryValue* PluginGroup::GetDataForUI() const {
 
 // Returns true if the latest version of this plugin group is vulnerable.
 bool PluginGroup::IsVulnerable() const {
-  if (min_version_.get() == NULL || max_version_->GetString() == "0") {
+  if (min_version_.get() == NULL || version_->GetString() == "0") {
     return false;
   }
-  return max_version_->CompareTo(*min_version_) < 0;
+  return version_->CompareTo(*min_version_) < 0;
 }
 
 void PluginGroup::Enable(bool enable) {
diff --git a/chrome/common/plugin_group.h b/chrome/common/plugin_group.h
index b22b64f..8e24722 100644
--- a/chrome/common/plugin_group.h
+++ b/chrome/common/plugin_group.h
@@ -88,7 +88,10 @@ class PluginGroup {
   void Enable(bool enable);
 
   // Returns this group's name
-  const string16 GetGroupName() const { return group_name_; }
+  const string16& GetGroupName() const { return group_name_; }
+
+  // Returns the description of highest-priority plug-in in the group.
+  const string16& description() const { return description_; }
 
   // Returns a DictionaryValue with data to display in the UI.
   DictionaryValue* GetDataForUI() const;
@@ -116,6 +119,10 @@ class PluginGroup {
               const std::string& min_version,
               const std::string& update_url);
 
+  // Set the description and version for this plugin group from the
+  // given plug-in.
+  void UpdateDescriptionAndVersion(const WebPluginInfo& plugin);
+
   static std::set<string16>* policy_disabled_puglins_;
 
   string16 group_name_;
@@ -129,7 +136,7 @@ class PluginGroup {
   bool enabled_;
   std::string min_version_str_;
   scoped_ptr<Version> min_version_;
-  scoped_ptr<Version> max_version_;
+  scoped_ptr<Version> version_;
   std::vector<WebPluginInfo> web_plugin_infos_;
   std::vector<int> web_plugin_positions_;
 
diff --git a/chrome/common/plugin_group_unittest.cc b/chrome/common/plugin_group_unittest.cc
index 07373c3..879ad70 100644
--- a/chrome/common/plugin_group_unittest.cc
+++ b/chrome/common/plugin_group_unittest.cc
@@ -26,19 +26,24 @@ static const PluginGroupDefinition kPluginDefNotVulnerable = {
 
 // name, path, version, desc, mime_types, enabled.
 static WebPluginInfo kPlugin2043 = {
-    ASCIIToUTF16("MyPlugin"), FilePath(), ASCIIToUTF16("2.0.43"), string16(),
+    ASCIIToUTF16("MyPlugin"), FilePath(), ASCIIToUTF16("2.0.43"),
+    ASCIIToUTF16("MyPlugin version 2.0.43"),
     std::vector<WebPluginMimeType>(), true };
 static WebPluginInfo kPlugin3043 = {
-    ASCIIToUTF16("MyPlugin"), FilePath(), ASCIIToUTF16("3.0.43"), string16(),
+    ASCIIToUTF16("MyPlugin"), FilePath(), ASCIIToUTF16("3.0.43"),
+    ASCIIToUTF16("MyPlugin version 3.0.43"),
     std::vector<WebPluginMimeType>(), true };
 static WebPluginInfo kPlugin3044 = {
-    ASCIIToUTF16("MyPlugin"), FilePath(), ASCIIToUTF16("3.0.44"), string16(),
+    ASCIIToUTF16("MyPlugin"), FilePath(), ASCIIToUTF16("3.0.44"),
+    ASCIIToUTF16("MyPlugin version 3.0.44"),
     std::vector<WebPluginMimeType>(), true };
 static WebPluginInfo kPlugin3045 = {
-    ASCIIToUTF16("MyPlugin"), FilePath(), ASCIIToUTF16("3.0.45"), string16(),
+    ASCIIToUTF16("MyPlugin"), FilePath(), ASCIIToUTF16("3.0.45"),
+    ASCIIToUTF16("MyPlugin version 3.0.45"),
     std::vector<WebPluginMimeType>(), true };
 static WebPluginInfo kPlugin4043 = {
-    ASCIIToUTF16("MyPlugin"), FilePath(), ASCIIToUTF16("4.0.43"), string16(),
+    ASCIIToUTF16("MyPlugin"), FilePath(), ASCIIToUTF16("4.0.43"),
+    ASCIIToUTF16("MyPlugin version 4.0.43"),
     std::vector<WebPluginMimeType>(), true };
 
 class PluginGroupTest : public testing::Test {
@@ -52,18 +57,6 @@ TEST(PluginGroupTest, PluginGroupMatch) {
   EXPECT_FALSE(group->IsVulnerable());
 }
 
-TEST(PluginGroupTest, PluginGroupMatchMultipleFiles) {
-  scoped_ptr<PluginGroup> group(PluginGroup::FromPluginGroupDefinition(
-      kPluginDef3));
-  EXPECT_TRUE(group->Match(kPlugin3043));
-  group->AddPlugin(kPlugin3043, 0);
-  EXPECT_TRUE(group->IsVulnerable());
-
-  EXPECT_TRUE(group->Match(kPlugin3045));
-  group->AddPlugin(kPlugin3045, 1);
-  EXPECT_FALSE(group->IsVulnerable());
-}
-
 TEST(PluginGroupTest, PluginGroupMatchCorrectVersion) {
   scoped_ptr<PluginGroup> group(PluginGroup::FromPluginGroupDefinition(
       kPluginDef3));
@@ -77,6 +70,56 @@ TEST(PluginGroupTest, PluginGroupMatchCorrectVersion) {
   EXPECT_TRUE(group->Match(kPlugin4043));
 }
 
+TEST(PluginGroupTest, PluginGroupDescription) {
+  string16 desc3043(ASCIIToUTF16("MyPlugin version 3.0.43"));
+  string16 desc3045(ASCIIToUTF16("MyPlugin version 3.0.45"));
+  WebPluginInfo plugin3043(kPlugin3043);
+  WebPluginInfo plugin3045(kPlugin3045);
+
+  {
+    scoped_ptr<PluginGroup> group(PluginGroup::FromPluginGroupDefinition(
+        kPluginDef3));
+    EXPECT_TRUE(group->Match(plugin3043));
+    group->AddPlugin(plugin3043, 0);
+    EXPECT_EQ(desc3043, group->description());
+    EXPECT_TRUE(group->IsVulnerable());
+    EXPECT_TRUE(group->Match(plugin3045));
+    group->AddPlugin(plugin3045, 1);
+    EXPECT_EQ(desc3043, group->description());
+    EXPECT_TRUE(group->IsVulnerable());
+  }
+
+  {
+    // Disable the first plugin.
+    plugin3043.enabled = false;
+    scoped_ptr<PluginGroup> group(PluginGroup::FromPluginGroupDefinition(
+        kPluginDef3));
+    EXPECT_TRUE(group->Match(plugin3043));
+    group->AddPlugin(plugin3043, 0);
+    EXPECT_EQ(desc3043, group->description());
+    EXPECT_TRUE(group->IsVulnerable());
+    EXPECT_TRUE(group->Match(plugin3045));
+    group->AddPlugin(plugin3045, 1);
+    EXPECT_EQ(desc3045, group->description());
+    EXPECT_FALSE(group->IsVulnerable());
+  }
+
+  {
+    // Disable the second plugin.
+    plugin3045.enabled = false;
+    scoped_ptr<PluginGroup> group(PluginGroup::FromPluginGroupDefinition(
+        kPluginDef3));
+    EXPECT_TRUE(group->Match(plugin3043));
+    group->AddPlugin(plugin3043, 1);
+    EXPECT_EQ(desc3043, group->description());
+    EXPECT_TRUE(group->IsVulnerable());
+    EXPECT_TRUE(group->Match(plugin3045));
+    group->AddPlugin(plugin3045, 0);
+    EXPECT_EQ(desc3043, group->description());
+    EXPECT_TRUE(group->IsVulnerable());
+  }
+}
+
 TEST(PluginGroupTest, PluginGroupDefinition) {
   const PluginGroupDefinition* definitions =
       PluginGroup::GetPluginGroupDefinitions();