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

chrome/browser/plugin_installer.h

 // Copyright (c) 2012 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.
 
 #ifndef CHROME_BROWSER_PLUGIN_INSTALLER_H_
 #define CHROME_BROWSER_PLUGIN_INSTALLER_H_
 
 #include "base/observer_list.h"
 #include "base/string16.h"
 #include "base/version.h"
@@ skipping 26 matching lines @@
   enum SecurityStatus {
     SECURITY_STATUS_UP_TO_DATE,
     SECURITY_STATUS_OUT_OF_DATE,
     SECURITY_STATUS_REQUIRES_AUTHORIZATION,
   };
 
   PluginInstaller(const std::string& identifier,
                   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;
   virtual void OnDownloadDestroyed(content::DownloadItem* download) OVERRIDE;
 
   void AddObserver(PluginInstallerObserver* observer);
   void RemoveObserver(PluginInstallerObserver* observer);
 
   void AddWeakObserver(WeakPluginInstallerObserver* observer);
   void RemoveWeakObserver(WeakPluginInstallerObserver* observer);
 
   // Unique identifier for the plug-in.
   const std::string& identifier() const { return identifier_; }
 
   // 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
   // directly downloaded.
   bool url_for_display() const { return url_for_display_; }
   const GURL& plugin_url() const { return plugin_url_; }
 
   // URL to open when the user clicks on the "Problems installing?" link.
   const GURL& help_url() const { return help_url_; }
 
@@ skipping 26 matching lines @@
   };
 
   void DownloadStarted(scoped_refptr<content::DownloadManager> dlm,
                        content::DownloadId download_id,
                        net::Error error);
   void DownloadError(const std::string& msg);
   void DownloadCancelled();
 
   std::string identifier_;
   string16 name_;
+  string16 group_name_;
+  string16 group_name_matcher_;
   bool url_for_display_;
   GURL plugin_url_;
   GURL help_url_;
   std::map<Version, SecurityStatus, VersionComparator> versions_;
 
   InstallerState state_;
   ObserverList<PluginInstallerObserver> observers_;
   ObserverList<WeakPluginInstallerObserver> weak_observers_;
 
   DISALLOW_COPY_AND_ASSIGN(PluginInstaller);
 };
 
 #endif  // CHROME_BROWSER_PLUGIN_INSTALLER_H_