Changing PluginPrefs to use PluginFinder's async interface.

chrome/browser/ui/webui/plugins_ui.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/ui/webui/plugins_ui.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/memory/singleton.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/api/prefs/pref_member.h"
 #include "chrome/browser/content_settings/host_content_settings_map.h"
+#include "chrome/browser/plugin_finder.h"
+#include "chrome/browser/plugin_installer.h"
 #include "chrome/browser/plugin_prefs.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/webui/chrome_url_data_manager.h"
 #include "chrome/browser/ui/webui/chrome_web_ui_data_source.h"
 #include "chrome/common/chrome_content_client.h"
 #include "chrome/common/chrome_notification_types.h"
@@ skipping 10 matching lines @@
 #include "grit/theme_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/layout.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "webkit/plugins/npapi/plugin_group.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/ui/webui/chromeos/ui_account_tweaks.h"
 #endif
 
-#if defined(ENABLE_PLUGIN_INSTALLATION)
-#include "chrome/browser/plugin_finder.h"
-#include "chrome/browser/plugin_installer.h"
-#else
-// Forward-declare PluginFinder. It's never actually used, but we pass a NULL
-// pointer instead.
-class PluginFinder;
-#endif
-
 using content::PluginService;
 using content::WebContents;
 using content::WebUIMessageHandler;
 using webkit::npapi::PluginGroup;
 using webkit::WebPluginInfo;
 
 namespace {
 
+// Helper callback method to process result of CanEnablePlugin method.

[James Hawkins, 2012/09/04 14:13:17]

nit: No need to specify that this is a helper callback.

For future reference this is a function not a method, the latter being a
function on an object.

[ibraaaa, 2012/09/04 14:52:09]

Done.

+void CheckCanEnablePluginCallback(bool can_enable) {

[James Hawkins, 2012/09/04 14:13:17]

What is the purpose of this function?

[ibraaaa, 2012/09/04 14:52:09]

We merged EnablePlugin and CanEnablePlugin into one method and accordingly the
result of the state-change check done by CanEnablePlugin is now supplied as
input to the Callback function (in this case CheckCanEnablePluginCallback).

Since the old code used to DCHECK the result of CanEnablePlugin we maintained
the same behavior by DCHECK-ing the result input to the callback.

On 2012/09/04 14:13:17, James Hawkins wrote:

[James Hawkins, 2012/09/04 14:55:50]

OK.  The function name seems a bit misleading as it's not checking anything (it
is asserting, however).  It's also misleading since a 'check' callback function
(to me) implies there is a boolean return value that the caller will use.

[ibraaaa, 2012/09/04 15:09:08]

Ahh, ok. Actually, the name is a left over from multiple review iterations. Just
missed it out, also the documentation had CanEnablePlugin which does not exist
now.
I modified this and the parameter name to be consistent with other usages of
EnablePlugin and also to be consistent with the fact that the check has already
happened and this is the result of either the plugin was enabled or not. Thanks!

On 2012/09/04 14:55:50, James Hawkins wrote:

+  DCHECK(can_enable);
+}
+
 ChromeWebUIDataSource* CreatePluginsUIHTMLSource() {
   ChromeWebUIDataSource* source =
       new ChromeWebUIDataSource(chrome::kChromeUIPluginsHost);
   source->set_use_json_js_format_v2();
 
   source->AddLocalizedString("pluginsTitle", IDS_PLUGINS_TITLE);
   source->AddLocalizedString("pluginsDetailsModeLink",
                              IDS_PLUGINS_DETAILS_MODE_LINK);
   source->AddLocalizedString("pluginsNoneInstalled",
                              IDS_PLUGINS_NONE_INSTALLED);
@@ skipping 88 matching lines @@
 
  private:
   // Call this to start getting the plugins on the UI thread.
   void GetPluginFinder();
 
   // Called when we have a PluginFinder and need to load the list of plug-ins.
   void LoadPlugins(PluginFinder* plugin_finder);
 
   // Called on the UI thread when the plugin information is ready.
   void PluginsLoaded(PluginFinder* plugin_finder,
-                     const std::vector<PluginGroup>& groups);
+                     const std::vector<webkit::WebPluginInfo>& plugins);
 
   content::NotificationRegistrar registrar_;
 
   base::WeakPtrFactory<PluginsDOMHandler> weak_ptr_factory_;
 
+  typedef base::hash_map<std::string, std::vector<const WebPluginInfo*> >

[James Hawkins, 2012/09/04 14:13:17]

nit: Document the map.

[ibraaaa, 2012/09/04 14:52:09]

Done.

+      PluginGroups;
+
   // This pref guards the value whether about:plugins is in the details mode or
   // not.
   BooleanPrefMember show_details_;
 
   DISALLOW_COPY_AND_ASSIGN(PluginsDOMHandler);
 };
 
 PluginsDOMHandler::PluginsDOMHandler()
     : ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
 }
@@ skipping 66 matching lines @@
         plugin_prefs->EnablePluginGroup(false, internalpdf);
       else if (group_name == internalpdf)
         plugin_prefs->EnablePluginGroup(false, adobereader);
     }
   } else {
     FilePath::StringType file_path;
     if (!args->GetString(0, &file_path)) {
       NOTREACHED();
       return;
     }
-    DCHECK(plugin_prefs->CanEnablePlugin(enable, FilePath(file_path)));
+
     plugin_prefs->EnablePlugin(enable, FilePath(file_path),
-                               base::Bind(&base::DoNothing));
+                               base::Bind(&CheckCanEnablePluginCallback));
   }
 }
 
 void PluginsDOMHandler::HandleSaveShowDetailsToPrefs(const ListValue* args) {
   std::string details_mode;
   if (!args->GetString(0, &details_mode)) {
     NOTREACHED();
     return;
   }
   show_details_.SetValue(details_mode == "true");
@@ skipping 45 matching lines @@
 
 #if defined(ENABLE_PLUGIN_INSTALLATION)
   PluginFinder::Get(base::Bind(&PluginsDOMHandler::LoadPlugins,
                                weak_ptr_factory_.GetWeakPtr()));
 #else
   LoadPlugins(NULL);
 #endif
 }
 
 void PluginsDOMHandler::LoadPlugins(PluginFinder* plugin_finder) {
-  PluginService::GetInstance()->GetPluginGroups(
+  PluginService::GetInstance()->GetPlugins(
       base::Bind(&PluginsDOMHandler::PluginsLoaded,
           weak_ptr_factory_.GetWeakPtr(), plugin_finder));
 }
 
-void PluginsDOMHandler::PluginsLoaded(PluginFinder* plugin_finder,
-                                      const std::vector<PluginGroup>& groups) {
+void PluginsDOMHandler::PluginsLoaded(
+    PluginFinder* plugin_finder,
+    const std::vector<webkit::WebPluginInfo>& plugins) {
   Profile* profile = Profile::FromWebUI(web_ui());
   PluginPrefs* plugin_prefs = PluginPrefs::GetForProfile(profile);
 
   ContentSettingsPattern wildcard = ContentSettingsPattern::Wildcard();
 
-  // Construct DictionaryValues to return to the UI
+  // Group plug-ins by identifier. This is done to be able to display
+  // the plug-ins in UI in a grouped fashion.
+  PluginGroups groups;
+  for (size_t i = 0; i < plugins.size(); ++i) {
+    PluginInstaller* installer = plugin_finder->GetPluginInstaller(plugins[i]);
+    groups[installer->identifier()].push_back(&plugins[i]);
+  }
+
+  // Construct DictionaryValues to return to UI.
   ListValue* plugin_groups_data = new ListValue();
-  for (size_t i = 0; i < groups.size(); ++i) {
-    const PluginGroup& group = groups[i];
-    if (group.IsEmpty())
-      continue;
+  for (PluginGroups::const_iterator it = groups.begin();
+      it != groups.end(); ++it) {
+    const std::vector<const WebPluginInfo*>& group_plugins = it->second;
     ListValue* plugin_files = new ListValue();
-    string16 group_name = group.GetGroupName();
+    PluginInstaller* plugin_installer =
+        plugin_finder->GetPluginInstaller(*group_plugins[0]);
+    string16 group_name = plugin_installer->name();
+    std::string group_identifier = plugin_installer->identifier();
     bool group_enabled = false;
     bool all_plugins_enabled_by_policy = true;
     bool all_plugins_disabled_by_policy = true;
     const WebPluginInfo* active_plugin = NULL;
-    for (size_t j = 0; j < group.web_plugin_infos().size(); ++j) {
-      const WebPluginInfo& group_plugin = group.web_plugin_infos()[j];
+    for (size_t j = 0; j < group_plugins.size(); ++j) {
+      const WebPluginInfo& group_plugin = *group_plugins[j];
 
       DictionaryValue* plugin_file = new DictionaryValue();
       plugin_file->SetString("name", group_plugin.name);
       plugin_file->SetString("description", group_plugin.desc);
       plugin_file->SetString("path", group_plugin.path.value());
       plugin_file->SetString("version", group_plugin.version);
       plugin_file->SetString("type", PluginTypeToString(group_plugin.type));
 
       ListValue* mime_types = new ListValue();
       const std::vector<webkit::WebPluginMimeType>& plugin_mime_types =
@@ skipping 24 matching lines @@
       PluginPrefs::PolicyStatus plugin_status =
           plugin_prefs->PolicyStatusForPlugin(group_plugin.name);
       PluginPrefs::PolicyStatus group_status =
           plugin_prefs->PolicyStatusForPlugin(group_name);
       if (plugin_status == PluginPrefs::POLICY_ENABLED ||
           group_status == PluginPrefs::POLICY_ENABLED) {
         enabled_mode = "enabledByPolicy";
       } else {
         all_plugins_enabled_by_policy = false;
         if (plugin_status == PluginPrefs::POLICY_DISABLED ||
-          group_status == PluginPrefs::POLICY_DISABLED) {
+            group_status == PluginPrefs::POLICY_DISABLED) {
           enabled_mode = "disabledByPolicy";
         } else {
           all_plugins_disabled_by_policy = false;
           if (plugin_enabled) {
             enabled_mode = "enabledByUser";
           } else {
             enabled_mode = "disabledByUser";
           }
         }
       }
       plugin_file->SetString("enabledMode", enabled_mode);
 
       plugin_files->Append(plugin_file);
     }
     DictionaryValue* group_data = new DictionaryValue();
 
     group_data->Set("plugin_files", plugin_files);
     group_data->SetString("name", group_name);
-    group_data->SetString("id", group.identifier());
+    group_data->SetString("id", group_identifier);
     group_data->SetString("description", active_plugin->desc);
     group_data->SetString("version", active_plugin->version);
 
 #if defined(ENABLE_PLUGIN_INSTALLATION)
     PluginInstaller* installer =
-        plugin_finder->FindPluginWithIdentifier(group.identifier());
+        plugin_finder->FindPluginWithIdentifier(group_identifier);
     if (installer) {
       bool out_of_date = installer->GetSecurityStatus(*active_plugin) ==
                          PluginInstaller::SECURITY_STATUS_OUT_OF_DATE;
       group_data->SetBoolean("critical", out_of_date);
       group_data->SetString("update_url", installer->plugin_url().spec());
     }
 #endif
 
     std::string enabled_mode;
     if (all_plugins_enabled_by_policy) {
       enabled_mode = "enabledByPolicy";
     } else if (all_plugins_disabled_by_policy) {
       enabled_mode = "disabledByPolicy";
     } else if (group_enabled) {
       enabled_mode = "enabledByUser";
     } else {
       enabled_mode = "disabledByUser";
     }
     group_data->SetString("enabledMode", enabled_mode);
 
     bool always_allowed = false;
     if (group_enabled) {
       const DictionaryValue* whitelist = profile->GetPrefs()->GetDictionary(
           prefs::kContentSettingsPluginWhitelist);
-      whitelist->GetBoolean(group.identifier(), &always_allowed);
+      whitelist->GetBoolean(group_identifier, &always_allowed);
     }
     group_data->SetBoolean("alwaysAllowed", always_allowed);
 
     plugin_groups_data->Append(group_data);
   }
   DictionaryValue results;
   results.Set("plugins", plugin_groups_data);
   web_ui()->CallJavascriptFunction("returnPluginsData", results);
 }
 
@@ skipping 23 matching lines @@
 void PluginsUI::RegisterUserPrefs(PrefService* prefs) {
   prefs->RegisterBooleanPref(prefs::kPluginsShowDetails,
                              false,
                              PrefService::UNSYNCABLE_PREF);
   prefs->RegisterBooleanPref(prefs::kPluginsShowSetReaderDefaultInfobar,
                              true,
                              PrefService::UNSYNCABLE_PREF);
   prefs->RegisterDictionaryPref(prefs::kContentSettingsPluginWhitelist,
                                 PrefService::SYNCABLE_PREF);
 }