Fixing Leak in PluginFinder#GetPluginMetadata

Causes of the leak:
1- We cache plugins in a map keyed by the plugin identifier however, plugins are matched by their names. When the name is empty, no match is found. We then create a new instance and insert it in the map (which may have an entry for this plugin identifier) without deleting old entries with the same identifier (if any) causing a leak.

2- Plugin identifier is the plugin's file name(without the extension). If two plugins have the same plugin name and file name but in different directories, then a match by the plugin name is not found. A leak happens when a new instance is created (will have the same identifier) and inserted in the map without deleting old entries with the same identifier (if any).

Fix:
1- For any unknown plugin (we didn't find a match for it) we use its MIME types in matching.

2- For any unknown plugin, we check if the identifier doesn't exist before we insert a new instance in the map. If it exists, the full path of the plugin file is used as the identifier.  



BUG=153791, 147755

Review URL: https://chromiumcodereview.appspot.com/11175006

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

2 files changed, 18 insertions, 7 deletions

diff --git a/chrome/browser/chromeos/gview_request_interceptor_unittest.cc b/chrome/browser/chromeos/gview_request_interceptor_unittest.cc
index b7895b6..49e3d7b 100644
--- a/chrome/browser/chromeos/gview_request_interceptor_unittest.cc
+++ b/chrome/browser/chromeos/gview_request_interceptor_unittest.cc
@@ -134,6 +134,8 @@ class GViewRequestInterceptorTest : public testing::Test {
   void RegisterPDFPlugin() {
     webkit::WebPluginInfo info;
     info.path = pdf_path_;
+    info.mime_types.push_back(webkit::WebPluginMimeType(
+        "application/pdf", ".test", "Test Plugin"));
     plugin_list_.AddPluginToLoad(info);
     plugin_list_.RefreshPlugins();
   }
diff --git a/chrome/browser/plugins/plugin_finder.cc b/chrome/browser/plugins/plugin_finder.cc
index d5ee101..4e64c50 100644
--- a/chrome/browser/plugins/plugin_finder.cc
+++ b/chrome/browser/plugins/plugin_finder.cc
@@ -33,13 +33,14 @@ namespace {
 
 typedef std::map<std::string, PluginMetadata*> PluginMap;
 
+// Gets the full path of the plug-in file as the identifier.
+std::string GetLongIdentifier(const webkit::WebPluginInfo& plugin) {
+  return plugin.path.AsUTF8Unsafe();
+}
+
 // Gets the base name of the file path as the identifier.
-static std::string GetIdentifier(const webkit::WebPluginInfo& plugin) {
-#if defined(OS_POSIX)
-  return plugin.path.BaseName().value();
-#elif defined(OS_WIN)
-  return base::SysWideToUTF8(plugin.path.BaseName().value());
-#endif
+std::string GetIdentifier(const webkit::WebPluginInfo& plugin) {
+  return plugin.path.BaseName().AsUTF8Unsafe();
 }
 
 // Gets the plug-in group name as the plug-in name if it is not empty or
@@ -273,8 +274,16 @@ scoped_ptr<PluginMetadata> PluginFinder::GetPluginMetadata(
   PluginMetadata* metadata = new PluginMetadata(identifier,
                                                 GetGroupName(plugin),
                                                 false, GURL(), GURL(),
-                                                GetGroupName(plugin),
+                                                plugin.name,
                                                 "");
+  for (size_t i = 0; i < plugin.mime_types.size(); ++i)
+    metadata->AddMatchingMimeType(plugin.mime_types[i].mime_type);
+
+  DCHECK(metadata->MatchesPlugin(plugin));
+  if (identifier_plugin_.find(identifier) != identifier_plugin_.end())
+    identifier = GetLongIdentifier(plugin);
+
+  DCHECK(identifier_plugin_.find(identifier) == identifier_plugin_.end());
   identifier_plugin_[identifier] = metadata;
   return metadata->Clone();
 }