[2] Adding a method to PluginFinder to get a PluginInstaller by WebPluginInfo.

chrome/browser/plugin_finder.cc

Index: chrome/browser/plugin_finder.cc
diff --git a/chrome/browser/plugin_finder.cc b/chrome/browser/plugin_finder.cc
index 59a5052950cf60ac6ac7d8b85a1689af4106de23..871d3988b6eb6126cdf8cc046aa8db8f23a368de 100644
--- a/chrome/browser/plugin_finder.cc
+++ b/chrome/browser/plugin_finder.cc
@@ -8,6 +8,7 @@
 #include "base/json/json_reader.h"
 #include "base/message_loop.h"
 #include "base/stl_util.h"
+#include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/plugin_installer.h"
@@ -129,12 +130,16 @@ PluginInstaller* PluginFinder::CreateInstaller(
   DCHECK(success);
   bool display_url = false;
   plugin_dict->GetBoolean("displayurl", &display_url);
+  string16 group_name_matcher;
+  success = plugin_dict->GetString("group_name_matcher", &group_name_matcher);
+  DCHECK(success);
 
   PluginInstaller* installer = new PluginInstaller(identifier,
                                                    name,
                                                    display_url,
                                                    GURL(url),
-                                                   GURL(help_url));
+                                                   GURL(help_url),
+                                                   group_name_matcher);
   const ListValue* versions = NULL;
   if (plugin_dict->GetList("versions", &versions)) {
     for (ListValue::const_iterator it = versions->begin();
@@ -161,3 +166,56 @@ PluginInstaller* PluginFinder::CreateInstaller(
   installers_[identifier] = installer;
   return installer;
 }
+
+PluginInstaller* PluginFinder::GetPluginInstaller(
+    const webkit::WebPluginInfo& plugin) {

[ibraaaa, 2012/08/22 14:44:13]

We can use a hash_map<plugin name, PluginInstaller*> to cache found installers
here. This may make search faster. However, we would need to save a 2 copies of
each PluginInstaller. What is your take on this?

[Bernhard Bauer, 2012/08/22 15:11:30]

Hm, if the PluginInstaller is owned by |installers_| (i.e. the map using
identifier as a key), we could simply use weak pointers in the second map?

[ibraaaa, 2012/08/22 17:04:54]

I think just making the second map have a pointer to the same object owned by
|installers_| would work fine also provided that we don't destruct both maps in
the destructor since they both point to the same thing. What do you think?

On the other hand, I liked the weak pointer thing, it was nice to read through,
knowledge++ :)

On 2012/08/22 15:11:30, Bernhard Bauer wrote:

[Bernhard Bauer, 2012/08/22 19:45:57]

Yes, exactly. Conceptually, we can say that |installers_| owns the
PluginInstaller objects, so we destroy those during destruction, whereas the
other map only has raw pointers, so we don't need to do anything.

+  for (DictionaryValue::Iterator plugin_it(*plugin_list_);
+      plugin_it.HasNext(); plugin_it.Advance()) {
+    // This method triggers the lazy initialization for all PluginInstallers.
+    FindPluginWithIdentifier(plugin_it.key());
+  }
+
+  // Use the group name matcher to find the plug-in installer we want.
+  for (std::map<std::string, PluginInstaller*>::const_iterator it =
+      installers_.begin(); it != installers_.end(); ++it) {
+    if (plugin.name.find(it->second->group_name_matcher()) == std::string::npos)
+      continue;
+
+    if (it->second->group_name().empty())
+      it->second->set_group_name(GetGroupName(plugin));
+
+    return it->second;
+  }
+
+  // The plug-in installer was not found, create a dummy one holding
+  // the name, identifier and group name only.
+  std::string identifier = GetIdentifier(plugin);
+  PluginInstaller* installer = new PluginInstaller(identifier, plugin.name,
+                                                   false, GURL(), GURL(),
+                                                   string16());
+  installer->set_group_name(GetGroupName(plugin));
+  installers_[identifier] = installer;
+  return installer;
+}
+
+// static
+std::string PluginFinder::GetIdentifier(const webkit::WebPluginInfo& plugin) {

[Bernhard Bauer, 2012/08/22 15:11:30]

Are you planning to call these methods from other classes? If not, you could put
them into an anonymous namespace.

[ibraaaa, 2012/08/22 18:29:25]

Done, I was not planning to use them in any other class, that is why they were
declared private. I am only curious about what is the difference between
declaring the helper methods as private static vs. in an anonymous namespace? Is
it only you want to decrease the implementation info exposed in the .h file by
hiding them in the .cc file?

On 2012/08/22 15:11:30, Bernhard Bauer wrote:

[Bernhard Bauer, 2012/08/22 19:45:57]

Yes, that's the only reason.

+#if defined(OS_POSIX)
+  return plugin.path.BaseName().value();
+#elif defined(OS_WIN)
+  return base::SysWideToUTF8(plugin.path.BaseName().value());
+#endif
+}
+
+// static
+string16 PluginFinder::GetGroupName(const webkit::WebPluginInfo& plugin) {
+  if (!plugin.name.empty())
+    return plugin.name;
+
+  FilePath::StringType path = plugin.path.BaseName().RemoveExtension().value();
+#if defined(OS_POSIX)
+  return UTF8ToUTF16(path);
+#elif defined(OS_WIN)
+  return WideToUTF16(path);
+#endif
+}