PluginList: Unit tests and bugfixes

webkit/glue/plugins/plugin_list_unittest.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 "base/utf_string_conversions.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace plugin_test_internal {
+
+// This class basically clones PluginList, but without any filesystem IO. There

[danno, 2010/12/15 10:42:12]

How about just: 
// A PluginList for tests that avoids file system IO. There...

[Jakob Kummerow, 2010/12/15 18:03:27]

Done.

+// is also no reason to use |lock_| (but it doesn't hurt either).
+class TestablePluginList : public NPAPI::PluginList {

[Bernhard Bauer, 2010/12/14 20:18:34]

Nit: MockPluginList or something.

[danno, 2010/12/15 10:42:12]

It's a bit more than a mock. The current name is a bit curious, however, since I
really hope that the PluginList is testable in the first place :-). How about
PluginListWithoutFileIO?

[Jakob Kummerow, 2010/12/15 18:03:27]

Done.

[Bernhard Bauer, 2010/12/17 18:54:18]

My suggestions for the color of the bikeshed: {Dummy,Fake}PluginList, and blue.

+ public:
+  void LoadDummyPlugin(WebPluginInfo* plugin) {
+    if (disabled_plugins_.count(plugin->path)) {
+      plugin->enabled = false;
+    } else {
+      plugin->enabled = true;
+    }
+    AddToPluginGroups(*plugin);
+  }
+
+  void LoadPlugins(bool refresh) {

[Bernhard Bauer, 2010/12/14 20:18:34]

Nit: Please add a comment "NPAPI::PluginList methods:" so that it's clear you're
overriding these methods. You might also want to declare them as virtual for
consistency.

[Jakob Kummerow, 2010/12/15 18:03:27]

Done.

+    if (plugins_loaded_ && !refresh)
+      return;
+
+    for (size_t i = 0; i < plugins_to_load_.size(); ++i)
+      LoadDummyPlugin(&plugins_to_load_[i]);
+
+    plugins_loaded_ = true;
+  }
+
+  void PostInitPluginGroup(PluginGroup* group) {
+    group->plugin_list_ = this;

[Bernhard Bauer, 2010/12/14 20:18:34]

Please use a setter and add a comment that this is only for unit tests.

[Jakob Kummerow, 2010/12/15 18:03:27]

Done.

+  }
+
+  void CreatePluginGroup(const PluginGroupDefinition& definition) {
+    PluginGroup* group = PluginGroup::FromPluginGroupDefinition(definition);
+    PostInitPluginGroup(group);
+    plugin_groups_.insert(std::make_pair(group->identifier(), group));
+  }
+
+  std::vector<WebPluginInfo> plugins_to_load_;
+};
+
+}  // namespace plugin_test_internal
+
+namespace {
+
+bool equals(const WebPluginInfo& a, const WebPluginInfo& b,

[danno, 2010/12/15 10:42:12]

Equals

[Jakob Kummerow, 2010/12/15 18:03:27]

Done.

+            bool care_about_enabled_status) {
+  return (a.name == b.name &&
+          a.path == b.path &&
+          a.version == b.version &&
+          a.desc == b.desc &&
+          (!care_about_enabled_status || a.enabled == b.enabled));
+}
+
+bool contains(const std::vector<WebPluginInfo>& list,

[danno, 2010/12/15 10:42:12]

Contains

[Jakob Kummerow, 2010/12/15 18:03:27]

Done.

+              const WebPluginInfo& plugin,
+              bool care_about_enabled_status) {
+  for (size_t i = 0; i < list.size(); ++i) {

[danno, 2010/12/15 10:42:12]

Nit: how about using container iterators here? 

[Jakob Kummerow, 2010/12/15 18:03:27]

Done.

+    if (equals(list[i], plugin, care_about_enabled_status))
+      return true;
+  }
+  return false;
+}
+
+}  // namespace
+
+namespace NPAPI {
+
+class PluginListTest : public testing::Test {
+ public:
+  PluginListTest()
+      : foo_plugin_(ASCIIToUTF16("Foo Plugin"),
+                    FilePath(FILE_PATH_LITERAL("/plugins/foo.plugin")),
+                    ASCIIToUTF16("1.2.3"),
+                    ASCIIToUTF16("foo")),
+        bar_plugin_(ASCIIToUTF16("Bar Plugin"),
+                    FilePath(FILE_PATH_LITERAL("/plugins/bar.plugin")),
+                    ASCIIToUTF16("2.3.4"),
+                    ASCIIToUTF16("bar")) {
+  }
+
+  virtual void SetUp() {
+    bar_plugin_.enabled = false;
+    plugin_list_.DisablePlugin(bar_plugin_.path, false);
+    plugin_list_.plugins_to_load_.push_back(foo_plugin_);
+    plugin_list_.plugins_to_load_.push_back(bar_plugin_);
+  }
+
+ protected:
+  plugin_test_internal::TestablePluginList plugin_list_;
+  WebPluginInfo foo_plugin_;
+  WebPluginInfo bar_plugin_;
+};
+
+TEST_F(PluginListTest, GetPlugins) {
+  std::vector<WebPluginInfo> plugins;
+  plugin_list_.GetPlugins(false, &plugins);
+  EXPECT_EQ(size_t(2), plugins.size());
+  EXPECT_TRUE(contains(plugins, foo_plugin_, true));
+  EXPECT_TRUE(contains(plugins, bar_plugin_, true));
+}
+
+TEST_F(PluginListTest, GetEnabledPlugins) {
+  std::vector<WebPluginInfo> plugins;
+  plugin_list_.GetEnabledPlugins(false, &plugins);
+  EXPECT_EQ(size_t(1), plugins.size());
+  EXPECT_TRUE(contains(plugins, foo_plugin_, true));
+}
+
+TEST_F(PluginListTest, GetPluginGroup) {
+  const PluginGroup* foo_group = plugin_list_.GetPluginGroup(foo_plugin_);
+  EXPECT_EQ(foo_group->GetGroupName(), foo_plugin_.name);
+  EXPECT_TRUE(foo_group->Enabled());
+  // The second request should return a pointer to the same instance.
+  const PluginGroup* foo_group2 = plugin_list_.GetPluginGroup(foo_plugin_);
+  EXPECT_EQ(foo_group, foo_group2);
+  const PluginGroup* bar_group = plugin_list_.GetPluginGroup(bar_plugin_);
+  EXPECT_FALSE(bar_group->Enabled());
+}
+
+TEST_F(PluginListTest, EnableDisablePlugin) {
+  // Disable "foo" plugin.
+  plugin_list_.DisablePlugin(foo_plugin_.path, false);
+  std::vector<WebPluginInfo> plugins;
+  plugin_list_.GetEnabledPlugins(false, &plugins);
+  EXPECT_FALSE(contains(plugins, foo_plugin_, false));
+  const PluginGroup* foo_group = plugin_list_.GetPluginGroup(foo_plugin_);
+  EXPECT_FALSE(foo_group->Enabled());
+  // Enable "bar" plugin.
+  plugin_list_.EnablePlugin(bar_plugin_.path);
+  plugin_list_.GetEnabledPlugins(false, &plugins);
+  EXPECT_TRUE(contains(plugins, bar_plugin_, false));
+  const PluginGroup* bar_group = plugin_list_.GetPluginGroup(bar_plugin_);
+  EXPECT_TRUE(bar_group->Enabled());
+}
+
+TEST_F(PluginListTest, EnableGroup) {
+  // Disable "foo" plugin group.
+  const PluginGroup* foo_group = plugin_list_.GetPluginGroup(foo_plugin_);
+  EXPECT_TRUE(foo_group->Enabled());
+  EXPECT_TRUE(plugin_list_.EnableGroup(false, foo_group->GetGroupName()));
+  EXPECT_FALSE(foo_group->Enabled());
+  std::vector<WebPluginInfo> plugins;
+  plugin_list_.GetEnabledPlugins(false, &plugins);
+  EXPECT_FALSE(contains(plugins, foo_plugin_, false));
+  // Enable "bar" plugin group.
+  const PluginGroup* bar_group = plugin_list_.GetPluginGroup(bar_plugin_);
+  EXPECT_FALSE(bar_group->Enabled());
+  plugin_list_.EnableGroup(true, bar_group->GetGroupName());
+  EXPECT_TRUE(bar_group->Enabled());
+  plugin_list_.GetEnabledPlugins(false, &plugins);
+  EXPECT_TRUE(contains(plugins, bar_plugin_, false));
+}
+
+TEST_F(PluginListTest, EmptyGroup) {
+  std::vector<PluginGroup> groups;
+  plugin_list_.GetPluginGroups(false, &groups);
+  for (size_t i = 0; i < groups.size(); ++i) {
+    EXPECT_GE(groups[i].GetPlugins().size(), size_t(1));

[Bernhard Bauer, 2010/12/14 20:18:34]

Nit: 1u instead of size_t(1)?

[Jakob Kummerow, 2010/12/15 18:03:27]

Done.

+  }
+}
+
+TEST_F(PluginListTest, DisableOutdated) {
+  VersionRangeDefinition version_range[] = {
+      { "0", "4", "3.0.44" },
+      { "4", "5", "" }
+  };
+  PluginGroupDefinition plugin_def = {
+      "myplugin-34", "MyPlugin 3/4", "MyPlugin", version_range,
+      arraysize(version_range), "http://latest" };
+  WebPluginInfo plugin_3043(ASCIIToUTF16("MyPlugin"),
+                            FilePath(FILE_PATH_LITERAL("/myplugin.3.0.43")),
+                            ASCIIToUTF16("3.0.43"),
+                            ASCIIToUTF16("MyPlugin version 3.0.43"));
+  WebPluginInfo plugin_3045(ASCIIToUTF16("MyPlugin"),
+                            FilePath(FILE_PATH_LITERAL("/myplugin.3.0.45")),
+                            ASCIIToUTF16("3.0.45"),
+                            ASCIIToUTF16("MyPlugin version 3.0.45"));
+  plugin_list_.plugins_to_load_.clear();
+  plugin_list_.plugins_to_load_.push_back(plugin_3043);
+  plugin_list_.plugins_to_load_.push_back(plugin_3045);
+  plugin_list_.CreatePluginGroup(plugin_def);
+  const PluginGroup* group_3043 = plugin_list_.GetPluginGroup(plugin_3043);
+  const PluginGroup* group_3045 = plugin_list_.GetPluginGroup(plugin_3045);
+  EXPECT_EQ(group_3043, group_3045);
+  EXPECT_EQ(plugin_3043.desc, group_3043->description());
+  EXPECT_TRUE(group_3043->IsVulnerable());
+  const_cast<PluginGroup*>(group_3043)->DisableOutdatedPlugins();
+  EXPECT_EQ(plugin_3045.desc, group_3043->description());
+  EXPECT_FALSE(group_3043->IsVulnerable());
+}
+
+}  // namespace NPAPI