[6] Moves CreateVersionFromString to plugin_utils and updates the callers.

chrome/browser/plugin_installer.cc

 // 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.
 
 #include "chrome/browser/plugin_installer.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/file_util.h"
 #include "base/process.h"
+#include "base/utf_string_conversions.h"
 #include "chrome/browser/download/download_service.h"
 #include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/platform_util.h"
+#include "chrome/browser/plugin_finder.h"
 #include "chrome/browser/plugin_installer_observer.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/download_id.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_manager.h"
 #include "content/public/browser/download_save_info.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
@@ skipping 40 matching lines @@
         base::Bind(callback, content::DownloadId::Invalid(), error));
   }
 }
 
 }  // namespace
 
 PluginInstaller::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)
     : identifier_(identifier),
       name_(name),
+      group_name_matcher_(group_name_matcher),
       url_for_display_(url_for_display),
       plugin_url_(plugin_url),
       help_url_(help_url),
       state_(INSTALLER_STATE_IDLE) {
 }
 
 PluginInstaller::~PluginInstaller() {
 }
 
 void PluginInstaller::AddVersion(const Version& version,
                                  SecurityStatus status) {
   DCHECK(versions_.find(version) == versions_.end());
   versions_[version] = status;
 }
 
 PluginInstaller::SecurityStatus PluginInstaller::GetSecurityStatus(
     const webkit::WebPluginInfo& plugin) const {
   // If there are no versions defined, the plug-in should require authorization.
   if (versions_.empty())
     return SECURITY_STATUS_REQUIRES_AUTHORIZATION;
 
   Version version;
-  webkit::npapi::PluginGroup::CreateVersionFromString(plugin.version, &version);
+  webkit::WebPluginInfo::CreateVersionFromString(plugin.version, &version);
   if (!version.IsValid())
     version = Version("0");
 
   // |lower_bound| returns the latest version that is not newer than |version|.
   std::map<Version, SecurityStatus, VersionComparator>::const_iterator it =
       versions_.lower_bound(version);
   // If there is at least one version defined, everything older than the oldest
   // defined version is considered out-of-date.
   if (it == versions_.end())
     return SECURITY_STATUS_OUT_OF_DATE;
@@ skipping 132 matching lines @@
   DCHECK_EQ(INSTALLER_STATE_DOWNLOADING, state_);
   state_ = INSTALLER_STATE_IDLE;
   FOR_EACH_OBSERVER(PluginInstallerObserver, observers_, DownloadError(msg));
 }
 
 void PluginInstaller::DownloadCancelled() {
   DCHECK_EQ(INSTALLER_STATE_DOWNLOADING, state_);
   state_ = INSTALLER_STATE_IDLE;
   FOR_EACH_OBSERVER(PluginInstallerObserver, observers_, DownloadCancelled());
 }
+
+/*static*/

[Bernhard Bauer, 2012/08/21 12:30:18]

Nit: The rest of the file uses // static.

[ibraaaa, 2012/08/21 14:26:04]

Done.

+string16 PluginInstaller::GetGroupName(const webkit::WebPluginInfo& plugin) {
+  PluginInstaller* plugin_installer = MatchWithConfiguredPlugins(plugin);
+  if (plugin_installer != NULL) {

[Bernhard Bauer, 2012/08/21 12:30:18]

Nit: Braces aren't required for single-line statements.

[ibraaaa, 2012/08/21 14:26:04]

Done.

+    return plugin_installer->name();
+  }
+
+  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
+}
+
+/*static*/
+std::string PluginInstaller::GetIdentifier(
+    const webkit::WebPluginInfo& plugin) {
+  PluginInstaller* plugin_installer = MatchWithConfiguredPlugins(plugin);
+  if (plugin_installer != NULL) {
+    return plugin_installer->identifier();
+  }
+
+  FilePath path = plugin.path;
+#if defined(OS_POSIX)
+  return path.BaseName().value();
+#elif defined(OS_WIN)
+  return base::SysWideToUTF8(path.BaseName().value());
+#endif
+}
+
+/*static*/
+PluginInstaller* PluginInstaller::MatchWithConfiguredPlugins(
+    const webkit::WebPluginInfo& plugin) {
+  const std::map<std::string, PluginInstaller*>& plugins =
+      PluginFinder::GetInstance()->GetAllPluginInstallers();

[Bernhard Bauer, 2012/08/21 12:30:18]

Ok, this is where stuff gets complicated.

Right now, we bundle the plug-in metadata files as resources which are mapped
into memory, so they can be accessed from any thread. Eventually though, we want
to move them to physical files on disk though that can be updated independently.
That means that we can't load them from disk directly, because that would mean
having to wait on disk I/O, which we are only allowed to do on specific threads.
That is why I started out with an asynchronous interface to PluginFinder (there
is no public method on PluginFinder that synchronously returns the instance).
So, we should probably move this to PluginFinder and make callers use the
asynchronous interface.

[ibraaaa, 2012/08/21 14:26:04]

I want to confirm something, places where I need a list of all plugins are in
PluginInstaller::GetIdentifier and PluginInstaller::GetGroupName, they basically
need to search in a list of all plugins and do the name matching thing before
they return. I may be missing something here, but the asynchronous interface
won't guarantee that all plugins are loaded before GetIdentifier/GetGroupName
return so the result may not be correct. In other words,
GetIdentifier/GetGroupName should not return a result until the asynchronous
call is 100% finished.

I hope we are on the same page, if not, please correct me.

On 2012/08/21 12:30:18, Bernhard Bauer wrote:

[Bernhard Bauer, 2012/08/21 19:14:52]

If by that you mean that we're going to need to make callers use an asynchronous
interface as well, then we're on the same page ;-) Blocking until the call to
GetIdentifier/GetGroupName has finished would defeat the purpose of making that
asynchronous in the first place.

The easiest way to achieve that would be to make these methods synchronous but
move them to PluginFinder (non-static!), so clients would have one asynchronous
call to get the current PluginFinder, and could afterward use a synchronous
interface.

***

Another thing we could do to keep the interface simple would be to have
PluginFinder always return a PluginInstaller. We add one method to PluginFinder
that finds the corresponding PluginInstaller for a given WebPluginInfo
(returning one from the metadata if available, or creating a dummy one
otherwise), then clients could ask that one for its name or identifier. WDYT?

+  for (std::map<std::string, PluginInstaller*>::const_iterator it =
+      plugins.begin(); it != plugins.end(); ++it) {
+    if (plugin.name.find(it->second->group_name_matcher()) == std::string::npos)
+      continue;
+    return it->second;
+  }
+
+  // Not found.
+  return NULL;
+}