Don't prepend hardcoded plugin definitions to plugin groups.

BUG=98514
TEST=PluginListTest.*


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

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

3 files changed, 47 insertions, 33 deletions

diff --git a/webkit/plugins/npapi/plugin_list.cc b/webkit/plugins/npapi/plugin_list.cc
index e323ab6..e28aa88 100644
--- a/webkit/plugins/npapi/plugin_list.cc
+++ b/webkit/plugins/npapi/plugin_list.cc
@@ -323,21 +323,28 @@ bool PluginList::ParseMimeTypes(
 
 PluginList::PluginList()
     : plugins_need_refresh_(true),
-      group_definitions_(kGroupDefinitions),
-      num_group_definitions_(ARRAYSIZE_UNSAFE(kGroupDefinitions)),
       default_plugin_enabled_(false) {
   PlatformInit();
-  AddHardcodedPluginGroups(&plugin_groups_);
+  AddHardcodedPluginGroups(kGroupDefinitions,
+                           ARRAYSIZE_UNSAFE(kGroupDefinitions));
 }
 
 PluginList::PluginList(const PluginGroupDefinition* definitions,
                        size_t num_definitions)
     : plugins_need_refresh_(true),
-      group_definitions_(definitions),
-      num_group_definitions_(num_definitions),
       default_plugin_enabled_(false) {
   // Don't do platform-dependend initialization in unit tests.
-  AddHardcodedPluginGroups(&plugin_groups_);
+  AddHardcodedPluginGroups(definitions, num_definitions);
+}
+
+PluginGroup* PluginList::CreatePluginGroup(
+      const webkit::WebPluginInfo& web_plugin_info) const {
+  for (size_t i = 0; i < hardcoded_plugin_groups_.size(); ++i) {
+    const PluginGroup* group = hardcoded_plugin_groups_[i];
+    if (group->Match(web_plugin_info))
+      return new PluginGroup(*group);
+  }
+  return PluginGroup::FromWebPluginInfo(web_plugin_info);
 }
 
 void PluginList::LoadPluginsInternal(ScopedVector<PluginGroup>* plugin_groups) {
@@ -410,7 +417,6 @@ void PluginList::LoadPlugins() {
   }
 
   ScopedVector<PluginGroup> new_plugin_groups;
-  AddHardcodedPluginGroups(&new_plugin_groups);
   // Do the actual loading of the plugins.
   LoadPluginsInternal(&new_plugin_groups);
 
@@ -636,7 +642,7 @@ PluginGroup* PluginList::GetPluginGroup(
       }
     }
   }
-  PluginGroup* group = PluginGroup::FromWebPluginInfo(web_plugin_info);
+  PluginGroup* group = CreatePluginGroup(web_plugin_info);
   group->AddPlugin(web_plugin_info);
   return group;
 }
@@ -649,10 +655,12 @@ string16 PluginList::GetPluginGroupName(const std::string& identifier) {
   return string16();
 }
 
-void PluginList::AddHardcodedPluginGroups(ScopedVector<PluginGroup>* groups) {
-  for (size_t i = 0; i < num_group_definitions_; ++i) {
-    groups->push_back(
-        PluginGroup::FromPluginGroupDefinition(group_definitions_[i]));
+void PluginList::AddHardcodedPluginGroups(
+    const PluginGroupDefinition* group_definitions,
+    size_t num_group_definitions) {
+  for (size_t i = 0; i < num_group_definitions; ++i) {
+    hardcoded_plugin_groups_->push_back(
+        PluginGroup::FromPluginGroupDefinition(group_definitions[i]));
   }
 }
 
@@ -667,7 +675,7 @@ PluginGroup* PluginList::AddToPluginGroups(
     }
   }
   if (!group) {
-    group = PluginGroup::FromWebPluginInfo(web_plugin_info);
+    group = CreatePluginGroup(web_plugin_info);
     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
diff --git a/webkit/plugins/npapi/plugin_list.h b/webkit/plugins/npapi/plugin_list.h
index 6c7cdee..bb8fbd1 100644
--- a/webkit/plugins/npapi/plugin_list.h
+++ b/webkit/plugins/npapi/plugin_list.h
@@ -204,31 +204,37 @@ class PluginList {
   friend struct base::DefaultLazyInstanceTraits<PluginList>;
   FRIEND_TEST_ALL_PREFIXES(PluginGroupTest, PluginGroupDefinition);
 
-  // Constructors are private for singletons
+  // Constructors are private for singletons.
   PluginList();
 
-  // Creates PluginGroups for the static group definitions, and adds them to
-  // the list of PluginGroups.
-  void AddHardcodedPluginGroups(ScopedVector<PluginGroup>* groups);
+  // Creates PluginGroups for the hardcoded group definitions, and stores them
+  // in |hardcoded_plugin_groups_|.
+  void AddHardcodedPluginGroups(const PluginGroupDefinition* group_definitions,
+                                size_t num_group_definitions);
 
-  // Implements all IO dependant operations of the LoadPlugins method so that
-  // test classes can mock these out. Return value false means LoadPlugins
-  // should not go on with the loading.
+  // Creates a new PluginGroup either from a hardcoded group definition, or from
+  // the plug-in information.
+  // Caller takes ownership of the returned PluginGroup.
+  PluginGroup* CreatePluginGroup(
+      const webkit::WebPluginInfo& web_plugin_info) const;
+
+  // Implements all IO dependent operations of the LoadPlugins method so that
+  // test classes can mock these out.
   virtual void LoadPluginsInternal(ScopedVector<PluginGroup>* plugin_groups);
 
-  // Load all plugins from the default plugins directory
+  // Load all plugins from the default plugins directory.
   void LoadPlugins();
 
   // Load all plugins from a specific directory.
-  // |plugins| is updated with loaded plugin information.
+  // |plugin_groups| is updated with loaded plugin information.
   // |visited_plugins| is updated with paths to all plugins that were considered
-  //   (including those we didn't load)
+  // (including those we didn't load).
   void LoadPluginsFromDir(const FilePath& path,
                           ScopedVector<PluginGroup>* plugin_groups,
                           std::set<FilePath>* visited_plugins);
 
   // Returns true if we should load the given plugin, or false otherwise.
-  // plugins is the list of plugins we have crawled in the current plugin
+  // |plugins| is the list of plugins we have crawled in the current plugin
   // loading run.
   bool ShouldLoadPlugin(const webkit::WebPluginInfo& info,
                         ScopedVector<PluginGroup>* plugins);
@@ -288,13 +294,13 @@ class PluginList {
   // Holds information about internal plugins.
   std::vector<InternalPlugin> internal_plugins_;
 
-  // Hardcoded plugin group definitions.
-  const PluginGroupDefinition* const group_definitions_;
-  const size_t num_group_definitions_;
-
   // Holds the currently available plugin groups.
   ScopedVector<PluginGroup> plugin_groups_;
 
+  // Holds the hardcoded definitions of well-known plug-ins.
+  // This should only be modified during construction of the PluginList.
+  ScopedVector<PluginGroup> hardcoded_plugin_groups_;
+
   // Callback that is invoked whenever the PluginList will reload the plugins.
   base::Closure will_load_plugins_callback_;
 
diff --git a/webkit/plugins/npapi/plugin_list_unittest.cc b/webkit/plugins/npapi/plugin_list_unittest.cc
index 20faa446..6298cef 100644
--- a/webkit/plugins/npapi/plugin_list_unittest.cc
+++ b/webkit/plugins/npapi/plugin_list_unittest.cc
@@ -57,8 +57,8 @@ class PluginListTest : public testing::Test {
   }
 
   virtual void SetUp() {
-    plugin_list_.AddPluginToLoad(foo_plugin_);
     plugin_list_.AddPluginToLoad(bar_plugin_);
+    plugin_list_.AddPluginToLoad(foo_plugin_);
   }
 
   PluginGroup* AddToPluginGroups(const WebPluginInfo& plugin) {
@@ -115,11 +115,11 @@ TEST_F(PluginListTest, HardcodedGroups) {
   plugin_list_.GetPluginGroups(true, &groups);
   ASSERT_EQ(2u, groups.size());
   EXPECT_EQ(1u, groups[0].web_plugin_infos().size());
-  EXPECT_TRUE(groups[0].ContainsPlugin(FilePath(kFooPath)));
-  EXPECT_EQ(kFooIdentifier, groups[0].identifier());
+  EXPECT_TRUE(groups[0].ContainsPlugin(FilePath(kBarPath)));
+  EXPECT_EQ("bar.plugin", groups[0].identifier());
   EXPECT_EQ(1u, groups[1].web_plugin_infos().size());
-  EXPECT_TRUE(groups[1].ContainsPlugin(FilePath(kBarPath)));
-  EXPECT_EQ("bar.plugin", groups[1].identifier());
+  EXPECT_TRUE(groups[1].ContainsPlugin(FilePath(kFooPath)));
+  EXPECT_EQ(kFooIdentifier, groups[1].identifier());
 }
 
 }  // namespace npapi