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

chrome/browser/plugin_installer.h

Index: chrome/browser/plugin_installer.h
diff --git a/chrome/browser/plugin_installer.h b/chrome/browser/plugin_installer.h
index bd2606d1d33e0cd5ceabf1ac4df61bdf97624ef7..0fb159201086ed7e985cc3960d250fbcaf31132d 100644
--- a/chrome/browser/plugin_installer.h
+++ b/chrome/browser/plugin_installer.h
@@ -44,7 +44,8 @@ class PluginInstaller : public content::DownloadItem::Observer {
                   const string16& name,
                   bool url_for_display,
                   const GURL& plugin_url,
-                  const GURL& help_url);
+                  const GURL& help_url,
+                  const string16& group_name_matcher);
   virtual ~PluginInstaller();
 
   virtual void OnDownloadUpdated(content::DownloadItem* download) OVERRIDE;
@@ -62,6 +63,15 @@ class PluginInstaller : public content::DownloadItem::Observer {
   // Human-readable name of the plug-in.
   const string16& name() const { return name_; }
 
+  // Plug-in group name.
+  const string16& group_name() const { return group_name_; }
+
+  // Sets |group_name_|
+  void set_group_name(const string16& group_name) { group_name_ = group_name; }

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

We don't really need a separate "group name" anymore. We can just use one name
which is the predefined one where we have it, and the one from the plug-in where
we don't.

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

That is what I am doing. If you checked how the group_name field is populated in
PluginFinder#GetPluginInstaller you will notice that the logic you mentioned
above is the one I used to populate this field. The reason for having the setter
function is because at the PluginInstaller construction time in
PluginFinder#CreateInstaller the plug-in(WebPluginInfo) is not available so I
would need to modify it later when the user asks for the group name (previously
calling GetGroupName and now calling GetPluginInstaller). The only reason for
having the field is to make PluginInstaller class complete, on the other hand we
can remove it and the caller can always deduce the group name by the plug-in
name + file path same way we do to set group_name before returning
PluginInstaller.

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

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

What I meant was that there is no reason to have both "group name" and "plug-in
name". When we call PluginFinder::CreateInstaller, we either have the plug-in
name from the metadata, or the deduced plug-in name from the WebPluginInfo.
Yes, except that PluginFinder can do that.

[ibraaaa, 2012/08/23 08:35:55]

Done.

+
+  // Plug-in group name substring used in matching plug-ins of the same group.
+  const string16& group_name_matcher() const { return group_name_matcher_; }

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

It might be nice to just have a public method that tells us whether a plug-in
name matches the group matcher. Then the substring matching that we do would be
just an implementation detail.

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

LIKE, I mean +1 :)

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

+
   // If |url_for_display| is false, |plugin_url| is the URL of the download page
   // for the plug-in, which should be opened in a new tab. If it is true,
   // |plugin_url| is the URL of the plug-in installer binary, which can be
@@ -108,6 +118,8 @@ class PluginInstaller : public content::DownloadItem::Observer {
 
   std::string identifier_;
   string16 name_;
+  string16 group_name_;
+  string16 group_name_matcher_;
   bool url_for_display_;
   GURL plugin_url_;
   GURL help_url_;