Separate plugin_metadata from plugin_installer, thread-safe plugin_finder

chrome/browser/plugin_finder.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_finder.h"
 
 #include "base/bind.h"
 #include "base/json/json_reader.h"
 #include "base/message_loop.h"
 #include "base/stl_util.h"
 #include "base/sys_string_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/plugin_installer.h"
+#include "chrome/browser/plugin_metadata.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/plugin_service.h"
 #include "googleurl/src/gurl.h"
 #include "grit/browser_resources.h"
 #include "ui/base/layout.h"
 #include "ui/base/resource/resource_bundle.h"
 
+#if defined(ENABLE_PLUGIN_INSTALLATION)
+#include "chrome/browser/plugin_installer.h"
+#endif
+
 using base::DictionaryValue;
 using content::PluginService;
 
 namespace {
 
 // 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
 }
 
 // Gets the plug-in group name as the plug-in name if it is not empty or
 // the filename without extension if the name is empty.
 static string16 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
 }
 
-// A callback barrier used to enforce the execution of a callback function
-// only when two different asynchronous callbacks are done execution.
-// The first asynchronous callback gets a PluginFinder instance and the
-// second asynchronous callback gets a list of plugins.
-class PluginFinderCallbackBarrier
-    : public base::RefCountedThreadSafe<PluginFinderCallbackBarrier> {
- public:
-  typedef base::Callback<void(const PluginFinder::PluginVector&)>
-      PluginsCallback;
-
-  explicit PluginFinderCallbackBarrier(
-      const PluginFinder::CombinedCallback& callback)
-      : callback_(callback),
-        finder_(NULL) {
-    DCHECK(!callback_.is_null());
-  }
-
-  base::Callback<void(PluginFinder*)> CreatePluginFinderCallback() {
-    return base::Bind(&PluginFinderCallbackBarrier::GotPluginFinder, this);
-  }
-
-  PluginsCallback CreatePluginsCallback() {
-    return base::Bind(&PluginFinderCallbackBarrier::GotPlugins, this);
-  }
-
- private:
-  friend class base::RefCountedThreadSafe<PluginFinderCallbackBarrier>;
-
-  ~PluginFinderCallbackBarrier() {
-    DCHECK(callback_.is_null());
-  }
-
-  void GotPlugins(const PluginFinder::PluginVector& plugins) {
-    plugins_.reset(new PluginFinder::PluginVector(plugins));
-    MaybeRunCallback();
-  }
-
-  void GotPluginFinder(PluginFinder* finder) {
-    finder_ = finder;
-    MaybeRunCallback();
-  }
-
-  // Executes the callback only when both asynchronous methods have finished
-  // their executions. This is identified by having non-null values in both
-  // |finder_| and |plugins_|.
-  void MaybeRunCallback() {
-    if (!finder_ || !plugins_.get())
-      return;
-
-    callback_.Run(*plugins_, finder_);
-    callback_.Reset();
-  }
-
-  PluginFinder::CombinedCallback callback_;
-  PluginFinder* finder_;
-  scoped_ptr<PluginFinder::PluginVector> plugins_;
-};
-
 }  // namespace
 
-// static
-void PluginFinder::GetPluginsAndPluginFinder(
-    const PluginFinder::CombinedCallback& cb) {
-  scoped_refptr<PluginFinderCallbackBarrier> barrier =
-      new PluginFinderCallbackBarrier(cb);
-
-  PluginFinder::Get(barrier->CreatePluginFinderCallback());
-  PluginService::GetInstance()->GetPlugins(barrier->CreatePluginsCallback());
-}
-
+// TODO(ibraaaa): DELETE. http://crbug.com/124396
 // static
 void PluginFinder::Get(const base::Callback<void(PluginFinder*)>& cb) {
   // At a later point we might want to do intialization here that needs to be
   // done asynchronously, like loading the plug-in list from disk or from a URL.
   MessageLoop::current()->PostTask(FROM_HERE, base::Bind(cb, GetInstance()));
 }
 
 // static
 PluginFinder* PluginFinder::GetInstance() {
   // PluginFinder::GetInstance() is the only method that's allowed to call
   // Singleton<PluginFinder>::get().
   return Singleton<PluginFinder>::get();
 }
 
-PluginFinder::PluginFinder() : plugin_list_(LoadPluginList()) {
+PluginFinder::PluginFinder() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+}
+
+// TODO(ibraaaa): initialize |plugin_list_| from Local State.
+// http://crbug.com/124396
+void PluginFinder::Init() {

[Bernhard Bauer, 2012/09/17 15:34:34]

Why don't we do this in the constructor?

[ibraaaa, 2012/09/17 16:12:56]

well, I remember that the style guide does not recommend doing work in Ctors,
here is a link:
http://www.corp.google.com/eng/doc/cppguide.xml#Doing_Work_in_Constructors

I personally like doing work in the Ctor :), do I move the logic there?

On 2012/09/17 15:34:34, Bernhard Bauer wrote:

[Bernhard Bauer, 2012/09/20 17:13:02]

I think doing work in the constructor is a lesser evil than having to initialize
a singleton in a completely different place. If we want to avoid both (at the
expense of more code), we could add special SingletonTraits that call the Init()
method. 

[ibraaaa, 2012/09/21 14:11:46]

Done.

+  plugin_list_.reset(LoadPluginList());
+
   if (!plugin_list_.get())
     plugin_list_.reset(new DictionaryValue());
+
+  for (DictionaryValue::Iterator plugin_it(*plugin_list_);
+      plugin_it.HasNext(); plugin_it.Advance()) {
+    DictionaryValue* plugin = NULL;
+    const std::string& identifier = plugin_it.key();
+    if (plugin_list_->GetDictionaryWithoutPathExpansion(identifier, &plugin)) {
+      PluginMetadata* metadata = CreatePluginMetadata(identifier, plugin);
+      identifier_plugin_[identifier] = metadata;
+
+#if defined(ENABLE_PLUGIN_INSTALLATION)
+      installers_[identifier] = new PluginInstaller(metadata);
+#endif
+    }
+  }
 }
 
 // static
 DictionaryValue* PluginFinder::LoadPluginList() {
 #if defined(OS_WIN) || defined(OS_MACOSX) || defined(OS_LINUX)
   base::StringPiece json_resource(
       ResourceBundle::GetSharedInstance().GetRawDataResource(
           IDR_PLUGIN_DB_JSON, ui::SCALE_FACTOR_NONE));
   std::string error_str;
   scoped_ptr<base::Value> value(base::JSONReader::ReadAndReturnError(
       json_resource,
       base::JSON_PARSE_RFC,
       NULL,
       &error_str));
   if (!value.get()) {
     DLOG(ERROR) << error_str;
     return NULL;
   }
   if (value->GetType() != base::Value::TYPE_DICTIONARY)
     return NULL;
   return static_cast<base::DictionaryValue*>(value.release());
 #else
   return new DictionaryValue();
 #endif
 }
 
 PluginFinder::~PluginFinder() {
+#if defined(ENABLE_PLUGIN_INSTALLATION)
   STLDeleteValues(&installers_);
+#endif
+  STLDeleteValues(&identifier_plugin_);
 }
 
+#if defined(ENABLE_PLUGIN_INSTALLATION)
 PluginInstaller* PluginFinder::FindPlugin(const std::string& mime_type,
                                           const std::string& language) {
   if (g_browser_process->local_state()->GetBoolean(prefs::kDisablePluginFinder))
     return NULL;
   for (DictionaryValue::Iterator plugin_it(*plugin_list_);
        plugin_it.HasNext(); plugin_it.Advance()) {
     const DictionaryValue* plugin = NULL;
     if (!plugin_it.value().GetAsDictionary(&plugin)) {
       NOTREACHED();
       continue;
     }
     std::string language_str;
     bool success = plugin->GetString("lang", &language_str);
     if (language_str != language)
       continue;
     const ListValue* mime_types = NULL;
     plugin->GetList("mime_types", &mime_types);
     DCHECK(success);
     for (ListValue::const_iterator mime_type_it = mime_types->begin();
          mime_type_it != mime_types->end(); ++mime_type_it) {
       std::string mime_type_str;
       success = (*mime_type_it)->GetAsString(&mime_type_str);
       DCHECK(success);
       if (mime_type_str == mime_type) {
         std::string identifier = plugin_it.key();
-        std::map<std::string, PluginInstaller*>::const_iterator installer =
-            installers_.find(identifier);
-        if (installer != installers_.end())
+        {
+          base::AutoLock lock(mutex_);
+          std::map<std::string, PluginInstaller*>::const_iterator installer =
+              installers_.find(identifier);
+          DCHECK(installer != installers_.end());
           return installer->second;
-        return CreateInstaller(identifier, plugin);
+        }
       }
     }
   }
   return NULL;
 }
 
 PluginInstaller* PluginFinder::FindPluginWithIdentifier(
     const std::string& identifier) {
+  base::AutoLock lock(mutex_);
   std::map<std::string, PluginInstaller*>::const_iterator it =
       installers_.find(identifier);
   if (it != installers_.end())
     return it->second;
-  DictionaryValue* plugin = NULL;
-  if (plugin_list_->GetDictionaryWithoutPathExpansion(identifier, &plugin))
-    return CreateInstaller(identifier, plugin);
+
+  return NULL;
+}
+#endif
+
+PluginMetadata* PluginFinder::FindPluginMetadataWithIdentifier(

[Bernhard Bauer, 2012/09/17 15:34:34]

Nit: I would prefer "FindPlugin..." here.

[ibraaaa, 2012/09/17 16:12:56]

Well, there is already "FindPluginWithIdentifier" above. Probably, it is better
to name this one FindPluginWithIdentifier and the one above
FindPluginInstallerWithIdentifier?

However, this naming switch will make this CL much bigger. I suggest we make
this switch in a separate CL?

On 2012/09/17 15:34:34, Bernhard Bauer wrote:

[Bernhard Bauer, 2012/09/20 17:05:11]

Sure.

+    const std::string& identifier) {
+  base::AutoLock lock(mutex_);
+  std::map<std::string, PluginMetadata*>::const_iterator it =
+      identifier_plugin_.find(identifier);
+  if (it != identifier_plugin_.end())
+    return it->second;
+
   return NULL;
 }
 
-PluginInstaller* PluginFinder::CreateInstaller(
+PluginMetadata* PluginFinder::CreatePluginMetadata(
     const std::string& identifier,
     const DictionaryValue* plugin_dict) {
-  DCHECK(!installers_[identifier]);
+  DCHECK(!identifier_plugin_[identifier]);
   std::string url;
   bool success = plugin_dict->GetString("url", &url);
   std::string help_url;
   plugin_dict->GetString("help_url", &help_url);
   string16 name;
   success = plugin_dict->GetString("name", &name);
   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),
-                                                   group_name_matcher);
+  PluginMetadata* plugin = new PluginMetadata(identifier,
+                                              name,
+                                              display_url,
+                                              GURL(url),
+                                              GURL(help_url),
+                                              group_name_matcher);
   const ListValue* versions = NULL;
   if (plugin_dict->GetList("versions", &versions)) {
     for (ListValue::const_iterator it = versions->begin();
          it != versions->end(); ++it) {
       DictionaryValue* version_dict = NULL;
       if (!(*it)->GetAsDictionary(&version_dict)) {
         NOTREACHED();
         continue;
       }
       std::string version;
       success = version_dict->GetString("version", &version);
       DCHECK(success);
       std::string status_str;
       success = version_dict->GetString("status", &status_str);
       DCHECK(success);
-      PluginInstaller::SecurityStatus status =
-          PluginInstaller::SECURITY_STATUS_UP_TO_DATE;
-      success = PluginInstaller::ParseSecurityStatus(status_str, &status);
+      PluginMetadata::SecurityStatus status =
+          PluginMetadata::SECURITY_STATUS_UP_TO_DATE;
+      success = PluginMetadata::ParseSecurityStatus(status_str, &status);
       DCHECK(success);
-      installer->AddVersion(Version(version), status);
+      plugin->AddVersion(Version(version), status);
     }
   }
 
-  installers_[identifier] = installer;
-  return installer;
+  return plugin;
 }
 
-PluginInstaller* PluginFinder::GetPluginInstaller(
+PluginMetadata* PluginFinder::GetPluginMetadata(
     const webkit::WebPluginInfo& plugin) {
-  if (name_installers_.find(plugin.name) != name_installers_.end())
-    return name_installers_[plugin.name];
+  base::AutoLock lock(mutex_);
+  if (name_plugin_.find(plugin.name) != name_plugin_.end())
+    return name_plugin_[plugin.name];
 
-  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) {
+  // Use the group name matcher to find the plug-in metadata we want.
+  for (std::map<std::string, PluginMetadata*>::const_iterator it =
+      identifier_plugin_.begin(); it != identifier_plugin_.end(); ++it) {
     if (!it->second->MatchesPlugin(plugin))
       continue;
 
-    name_installers_[plugin.name] = it->second;
+    name_plugin_[plugin.name] = it->second;
     return it->second;
   }
 
-  // The plug-in installer was not found, create a dummy one holding
+  // The plug-in metadata 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,
-                                                   GetGroupName(plugin),
-                                                   false, GURL(), GURL(),
-                                                   GetGroupName(plugin));
-  installers_[identifier] = installer;
-  name_installers_[plugin.name] = installer;
-  return installer;
+  PluginMetadata* metadata = new PluginMetadata(identifier,
+                                                GetGroupName(plugin),
+                                                false, GURL(), GURL(),
+                                                GetGroupName(plugin));
+
+  name_plugin_[plugin.name] = metadata;
+  identifier_plugin_[identifier] = metadata;
+  return metadata;
 }