[4] Changing content_settings_api to use PluginFinder's async interface

chrome/browser/extensions/api/content_settings/content_settings_api.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/extensions/api/content_settings/content_settings_api.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/hash_tables.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/api/content_settings/content_settings_api_constants.h"
 #include "chrome/browser/extensions/api/content_settings/content_settings_helpers.h"
 #include "chrome/browser/extensions/api/content_settings/content_settings_store.h"
 #include "chrome/browser/content_settings/cookie_settings.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
 #include "chrome/browser/extensions/extension_preference_api_constants.h"
 #include "chrome/browser/extensions/extension_preference_helpers.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/plugin_installer.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/api/content_settings.h"
 #include "chrome/common/extensions/extension_error_utils.h"
 #include "content/public/browser/plugin_service.h"
 #include "webkit/plugins/npapi/plugin_group.h"
 
 using content::BrowserThread;
 using content::PluginService;
 
 namespace Clear = extensions::api::content_settings::ContentSetting::Clear;
 namespace Get = extensions::api::content_settings::ContentSetting::Get;
 namespace Set = extensions::api::content_settings::ContentSetting::Set;
 namespace pref_helpers = extension_preference_helpers;
 namespace pref_keys = extension_preference_api_constants;
 
 namespace {
 
-const std::vector<webkit::npapi::PluginGroup>* g_testing_plugin_groups_;
+const std::vector<webkit::WebPluginInfo>* g_testing_plugins_;
 
 bool RemoveContentType(ListValue* args, ContentSettingsType* content_type) {
   std::string content_type_str;
   if (!args->GetString(0, &content_type_str))
     return false;
   // We remove the ContentSettingsType parameter since this is added by the
   // renderer, and is not part of the JSON schema.
   args->Remove(0, NULL);
   *content_type =
       extensions::content_settings_helpers::StringToContentSettingsType(
@@ skipping 194 matching lines @@
                                     secondary_pattern, content_type,
                                     resource_identifier, setting, scope);
   return true;
 }
 
 bool GetResourceIdentifiersFunction::RunImpl() {
   ContentSettingsType content_type;
   EXTENSION_FUNCTION_VALIDATE(RemoveContentType(args_.get(), &content_type));
 
   if (content_type == CONTENT_SETTINGS_TYPE_PLUGINS) {
-    if (g_testing_plugin_groups_) {
-      OnGotPluginGroups(*g_testing_plugin_groups_);
-    } else {
-      PluginService::GetInstance()->GetPluginGroups(
-          base::Bind(&GetResourceIdentifiersFunction::OnGotPluginGroups, this));
-    }
+    PluginFinder::Get(base::Bind(
+        &GetResourceIdentifiersFunction::GetPluginFinderForRunImpl, this));
   } else {
     SendResponse(true);
   }
 
   return true;
 }
 
-void GetResourceIdentifiersFunction::OnGotPluginGroups(
-    const std::vector<webkit::npapi::PluginGroup>& groups) {
+void GetResourceIdentifiersFunction::GetPluginFinderForRunImpl(

[Mattias Nissler (ping if slow), 2012/08/27 15:42:09]

The name of this function is unfortunate. It actually doesn't "get" anything.

+    PluginFinder* finder) {
+  if (g_testing_plugins_) {
+    OnGotPlugins(finder, *g_testing_plugins_);
+    return;
+  }
+  PluginService::GetInstance()->GetPlugins(
+      base::Bind(&GetResourceIdentifiersFunction::OnGotPlugins, this,
+                 finder));

[Mattias Nissler (ping if slow), 2012/08/27 15:42:09]

Aha, so we make a call to PluginFinder::Get that invokes a callback in which we
run PluginService::GetPlugins, which again results in an asynchronous callback?
Any chance to get the thing you need in a single async operation, e.g. making
PluginService::GetPlugins() get the finder behind the scenes?

[ibraaaa, 2012/08/27 17:34:05]

Unfortunately, we can't make PluginService::GetPlugins get the finder behind the
scenes because not all callers of PluginService::GetPlugins will need to operate
on the list of plugins using PluginFinder instance. So it will be useful for
this call (and some others) but not for the rest.

On 2012/08/27 15:42:09, Mattias Nissler wrote:

[Bernhard Bauer, 2012/08/29 12:56:54]

Also, PluginService lives in content/, so it can't know about PluginFinder.

Another thing we could try is to issue both asynchronous calls in parallel and
only continue when both have returned their value. That would require more
bookkeeping, but we might be able to extract it to a generic method.

[ibraaaa, 2012/08/29 16:39:37]

mmm, let me clarify how can we end up with PluginFinder and
vector<WebPluginInfo> (where each of them is returned in a different callback
method) in the same method so we can operate on the plugins using the finder?

On 2012/08/29 12:56:54, Bernhard Bauer wrote:

[Bernhard Bauer, 2012/08/29 16:59:15]

It's similar to the CallbackBarrier we use in PluginPrefs: We construct an
object that takes a Callback<void(PluginFinder*, vector<WebPluginInfo>)> and
hands out a Callback<void(PluginFinder*)> and a
Callback<void(vector<WebPluginInfo>)>. In these callbacks the objects stores the
passed in values and calls the final callback as soon as both callbacks have
been called.

+}
+
+void GetResourceIdentifiersFunction::OnGotPlugins(
+    PluginFinder* finder,
+    const std::vector<webkit::WebPluginInfo>& plugins) {
+  base::hash_set<std::string> group_identifiers;
   ListValue* list = new ListValue();
-  for (std::vector<webkit::npapi::PluginGroup>::const_iterator it =
-          groups.begin();
-       it != groups.end(); ++it) {
+  for (std::vector<webkit::WebPluginInfo>::const_iterator it = plugins.begin();
+       it != plugins.end(); ++it) {
+    PluginInstaller* installer = finder->GetPluginInstaller(*it);
+    const std::string& group_identifier = installer->identifier();
+    if (group_identifiers.find(group_identifier) != group_identifiers.end())
+      continue;
+
+    group_identifiers.insert(group_identifier);
     DictionaryValue* dict = new DictionaryValue();
-    dict->SetString(keys::kIdKey, it->identifier());
-    dict->SetString(keys::kDescriptionKey, it->GetGroupName());
+    dict->SetString(keys::kIdKey, group_identifier);
+    dict->SetString(keys::kDescriptionKey, installer->name());
     list->Append(dict);
   }
   SetResult(list);
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE, base::Bind(
           &GetResourceIdentifiersFunction::SendResponse, this, true));
 }
 
 // static
-void GetResourceIdentifiersFunction::SetPluginGroupsForTesting(
-    const std::vector<webkit::npapi::PluginGroup>* plugin_groups) {
-  g_testing_plugin_groups_ = plugin_groups;
+void GetResourceIdentifiersFunction::SetPluginsForTesting(
+    const std::vector<webkit::WebPluginInfo>* plugins) {
+  g_testing_plugins_ = plugins;
 }
 
 }  // namespace extensions