Enabling internal PDF reader should disable Reader, and vice versa.

BUG=50105
Review URL: http://codereview.chromium.org/3674002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@62154 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 114 insertions, 40 deletions

diff --git a/chrome/browser/dom_ui/plugins_ui.cc b/chrome/browser/dom_ui/plugins_ui.cc
index 828ee17..d3aea34 100644
--- a/chrome/browser/dom_ui/plugins_ui.cc
+++ b/chrome/browser/dom_ui/plugins_ui.cc
@@ -13,6 +13,7 @@
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/singleton.h"
+#include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/browser_window.h"
@@ -25,6 +26,7 @@
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/jstemplate_builder.h"
 #include "chrome/common/notification_service.h"
+#include "chrome/common/pepper_plugin_registry.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "grit/browser_resources.h"
@@ -210,27 +212,39 @@ void PluginsDOMHandler::HandleEnablePluginMessage(const ListValue* args) {
   std::string is_group_str;
   if (!args->GetString(1, &enable_str) || !args->GetString(2, &is_group_str))
     return;
+  bool enable = enable_str == "true";
 
+  PluginUpdater* plugin_updater = PluginUpdater::GetPluginUpdater();
   if (is_group_str == "true") {
     string16 group_name;
     if (!args->GetString(0, &group_name))
       return;
 
-    PluginUpdater::GetPluginUpdater()->EnablePluginGroup(
-        enable_str == "true", group_name);
+    plugin_updater->EnablePluginGroup(enable, group_name);
+    if (enable) {
+      // See http://crbug.com/50105 for background.
+      string16 reader8 = ASCIIToUTF16(PluginGroup::kAdobeReader8GroupName);
+      string16 reader9 = ASCIIToUTF16(PluginGroup::kAdobeReader9GroupName);
+      string16 internalpdf = ASCIIToUTF16(PepperPluginRegistry::kPDFPluginName);
+      if (group_name == reader8 || group_name == reader9) {
+        plugin_updater->EnablePluginGroup(false, internalpdf);
+      } else if (group_name == internalpdf) {
+        plugin_updater->EnablePluginGroup(false, reader8);
+        plugin_updater->EnablePluginGroup(false, reader9);
+      }
+    }
   } else {
     FilePath::StringType file_path;
     if (!args->GetString(0, &file_path))
       return;
 
-    PluginUpdater::GetPluginUpdater()->EnablePluginFile(
-        enable_str == "true", file_path);
+    plugin_updater->EnablePluginFile(enable, file_path);
   }
 
   // TODO(viettrungluu): We might also want to ensure that the plugins
   // list is always written to prefs even when the user hasn't disabled a
   // plugin. <http://crbug.com/39101>
-  PluginUpdater::GetPluginUpdater()->UpdatePreferences(dom_ui_->GetProfile());
+  plugin_updater->UpdatePreferences(dom_ui_->GetProfile());
 }
 
 void PluginsDOMHandler::HandleShowTermsOfServiceMessage(const ListValue* args) {
diff --git a/chrome/browser/plugin_updater.cc b/chrome/browser/plugin_updater.cc
index e1f1764..d2c5d4f 100644
--- a/chrome/browser/plugin_updater.cc
+++ b/chrome/browser/plugin_updater.cc
@@ -8,11 +8,13 @@
 #include <vector>
 
 #include "base/command_line.h"
+#include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/scoped_ptr.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "base/version.h"
+#include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profile.h"
 #include "chrome/common/chrome_paths.h"
@@ -20,11 +22,11 @@
 #include "chrome/common/notification_service.h"
 #include "chrome/common/pepper_plugin_registry.h"
 #include "chrome/common/pref_names.h"
-#include "webkit/glue/plugins/plugin_group.h"
-#include "webkit/glue/plugins/plugin_list.h"
 #include "webkit/glue/plugins/webplugininfo.h"
 
-PluginUpdater::PluginUpdater() : enable_internal_pdf_(true) {
+PluginUpdater::PluginUpdater()
+    : enable_internal_pdf_(true),
+      notify_pending_(false) {
 }
 
 DictionaryValue* PluginUpdater::CreatePluginFileSummary(
@@ -56,12 +58,8 @@ ListValue* PluginUpdater::GetPluginGroupsData() {
 void PluginUpdater::EnablePluginGroup(bool enable, const string16& group_name) {
   if (PluginGroup::IsPluginNameDisabledByPolicy(group_name))
     enable = false;
-  if (NPAPI::PluginList::Singleton()->EnableGroup(enable, group_name)) {
-    NotificationService::current()->Notify(
-        NotificationType::PLUGIN_ENABLE_STATUS_CHANGED,
-        Source<PluginUpdater>(this),
-        NotificationService::NoDetails());
-  }
+  NPAPI::PluginList::Singleton()->EnableGroup(enable, group_name);
+  NotifyPluginStatusChanged();
 }
 
 void PluginUpdater::EnablePluginFile(bool enable,
@@ -72,10 +70,7 @@ void PluginUpdater::EnablePluginFile(bool enable,
   else
     NPAPI::PluginList::Singleton()->DisablePlugin(file_path);
 
-  NotificationService::current()->Notify(
-      NotificationType::PLUGIN_ENABLE_STATUS_CHANGED,
-      Source<PluginUpdater>(this),
-      NotificationService::NoDetails());
+  NotifyPluginStatusChanged();
 }
 
 void PluginUpdater::Observe(NotificationType type,
@@ -111,15 +106,11 @@ void PluginUpdater::DisablePluginsFromPolicy(const ListValue* plugin_names) {
   }
   PluginGroup::SetPolicyDisabledPluginPatterns(policy_disabled_plugin_patterns);
 
-  NotificationService::current()->Notify(
-      NotificationType::PLUGIN_ENABLE_STATUS_CHANGED,
-      Source<PluginUpdater>(this),
-      NotificationService::NoDetails());
+  NotifyPluginStatusChanged();
 }
 
 void PluginUpdater::DisablePluginGroupsFromPrefs(Profile* profile) {
   bool update_internal_dir = false;
-  bool update_preferences = false;
   FilePath last_internal_dir =
   profile->GetPrefs()->GetFilePath(prefs::kPluginsLastInternalDirectory);
   FilePath cur_internal_dir;
@@ -189,7 +180,6 @@ void PluginUpdater::DisablePluginGroupsFromPrefs(Profile* profile) {
           if (!enabled && force_enable_internal_pdf) {
             enabled = true;
             plugin->SetBoolean("enabled", true);
-            update_preferences = true;  // Can't modify the list during looping.
           }
         }
         if (!enabled)
@@ -216,14 +206,44 @@ void PluginUpdater::DisablePluginGroupsFromPrefs(Profile* profile) {
     // The internal PDF plugin is disabled by default, and the user hasn't
     // overridden the default.
     NPAPI::PluginList::Singleton()->DisablePlugin(pdf_path);
-    NPAPI::PluginList::Singleton()->EnableGroup(false, pdf_group_name);
+    EnablePluginGroup(false, pdf_group_name);
   }
 
-  if (update_preferences)
+  if (force_enable_internal_pdf) {
+    // See http://crbug.com/50105 for background.
+    EnablePluginGroup(false, ASCIIToUTF16(PluginGroup::kAdobeReader8GroupName));
+    EnablePluginGroup(false, ASCIIToUTF16(PluginGroup::kAdobeReader9GroupName));
     UpdatePreferences(profile);
+  }
 }
 
 void PluginUpdater::UpdatePreferences(Profile* profile) {
+  ChromeThread::PostTask(
+    ChromeThread::FILE,
+    FROM_HERE,
+    NewRunnableFunction(
+        &PluginUpdater::GetPreferencesDataOnFileThread, profile));
+}
+
+void PluginUpdater::GetPreferencesDataOnFileThread(void* profile) {
+  std::vector<WebPluginInfo> plugins;
+  NPAPI::PluginList::Singleton()->GetPlugins(false, &plugins);
+
+  NPAPI::PluginList::PluginMap groups;
+  NPAPI::PluginList::Singleton()->GetPluginGroups(false, &groups);
+
+  ChromeThread::PostTask(
+    ChromeThread::UI,
+    FROM_HERE,
+    NewRunnableFunction(
+        &PluginUpdater::OnUpdatePreferences,
+        static_cast<Profile*>(profile), plugins, groups));
+}
+
+void PluginUpdater::OnUpdatePreferences(
+    Profile* profile,
+    const std::vector<WebPluginInfo>& plugins,
+    const NPAPI::PluginList::PluginMap& groups) {
   ListValue* plugins_list = profile->GetPrefs()->GetMutableList(
       prefs::kPluginsPluginsList);
   plugins_list->Clear();
@@ -234,8 +254,6 @@ void PluginUpdater::UpdatePreferences(Profile* profile) {
                                      internal_dir);
 
   // Add the plugin files.
-  std::vector<WebPluginInfo> plugins;
-  NPAPI::PluginList::Singleton()->GetPlugins(false, &plugins);
   for (std::vector<WebPluginInfo>::const_iterator it = plugins.begin();
        it != plugins.end();
        ++it) {
@@ -243,10 +261,8 @@ void PluginUpdater::UpdatePreferences(Profile* profile) {
   }
 
   // Add the groups as well.
-  NPAPI::PluginList::PluginMap plugin_groups;
-  NPAPI::PluginList::Singleton()->GetPluginGroups(false, &plugin_groups);
-  for (NPAPI::PluginList::PluginMap::iterator it = plugin_groups.begin();
-       it != plugin_groups.end(); ++it) {
+  for (NPAPI::PluginList::PluginMap::const_iterator it = groups.begin();
+       it != groups.end(); ++it) {
     // Don't save preferences for vulnerable pugins.
     if (!CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableOutdatedPlugins) ||
@@ -256,6 +272,23 @@ void PluginUpdater::UpdatePreferences(Profile* profile) {
   }
 }
 
+void PluginUpdater::NotifyPluginStatusChanged() {
+  if (notify_pending_)
+    return;
+  notify_pending_ = true;
+  MessageLoop::current()->PostTask(
+      FROM_HERE,
+      NewRunnableFunction(&PluginUpdater::OnNotifyPluginStatusChanged));
+}
+
+void PluginUpdater::OnNotifyPluginStatusChanged() {
+  GetPluginUpdater()->notify_pending_ = false;
+  NotificationService::current()->Notify(
+      NotificationType::PLUGIN_ENABLE_STATUS_CHANGED,
+      Source<PluginUpdater>(GetPluginUpdater()),
+      NotificationService::NoDetails());
+}
+
 /*static*/
 PluginUpdater* PluginUpdater::GetPluginUpdater() {
   return Singleton<PluginUpdater>::get();
diff --git a/chrome/browser/plugin_updater.h b/chrome/browser/plugin_updater.h
index 5ed620b..2a8df1a1 100644
--- a/chrome/browser/plugin_updater.h
+++ b/chrome/browser/plugin_updater.h
@@ -10,6 +10,7 @@
 #include "base/file_path.h"
 #include "base/singleton.h"
 #include "chrome/common/notification_observer.h"
+#include "webkit/glue/plugins/plugin_list.h"
 
 class DictionaryValue;
 class ListValue;
@@ -48,11 +49,23 @@ class PluginUpdater : public NotificationObserver {
   PluginUpdater();
   virtual ~PluginUpdater() {}
 
-  // Note: if you change this to false from true, you must update
-  // kPluginsEnabledInternalPDF to be a new name (i.e. add 2, 3, 4...) at end.
-  bool enable_internal_pdf_;
+  // Called on the file thread to get the data necessary to update the saved
+  // preferences.  The profile pointer is only to be passed to the UI thread.
+  static void GetPreferencesDataOnFileThread(void* profile);
+
+  // Called on the UI thread with the plugin data to save the preferences.
+  static void OnUpdatePreferences(Profile* profile,
+                                  const std::vector<WebPluginInfo>& plugins,
+                                  const NPAPI::PluginList::PluginMap& groups);
+
+  // Queues sending the notification that plugin data has changed.  This is done
+  // so that if a bunch of changes happen, we only send one notification.
+  void NotifyPluginStatusChanged();
+
+  // Used for the post task to notify that plugin enabled status changed.
+  static void OnNotifyPluginStatusChanged();
 
-  DictionaryValue* CreatePluginFileSummary(const WebPluginInfo& plugin);
+  static DictionaryValue* CreatePluginFileSummary(const WebPluginInfo& plugin);
 
   // Force plugins to be disabled due to policy. |plugins| contains
   // the list of StringValues of the names of the policy-disabled plugins.
@@ -61,6 +74,12 @@ class PluginUpdater : public NotificationObserver {
   // Needed to allow singleton instantiation using private constructor.
   friend struct DefaultSingletonTraits<PluginUpdater>;
 
+  // Note: if you change this to false from true, you must update
+  // kPluginsEnabledInternalPDF to be a new name (i.e. add 2, 3, 4...) at end.
+  bool enable_internal_pdf_;
+
+  bool notify_pending_;
+
   DISALLOW_COPY_AND_ASSIGN(PluginUpdater);
 };
 
diff --git a/webkit/glue/plugins/plugin_group.cc b/webkit/glue/plugins/plugin_group.cc
index f774957..6e876ad7 100644
--- a/webkit/glue/plugins/plugin_group.cc
+++ b/webkit/glue/plugins/plugin_group.cc
@@ -13,6 +13,9 @@
 #include "webkit/glue/plugins/plugin_list.h"
 #include "webkit/glue/plugins/webplugininfo.h"
 
+const char* PluginGroup::kAdobeReader8GroupName = "Adobe Reader 8";
+const char* PluginGroup::kAdobeReader9GroupName = "Adobe Reader 9";
+
 #if defined(OS_MACOSX)
 // Plugin Groups for Mac.
 // Plugins are listed here as soon as vulnerabilities and solutions
@@ -43,10 +46,10 @@ static const PluginGroupDefinition kGroupDefinitions[] = {
     "http://www.apple.com/quicktime/download/" },
   { "java-runtime-environment", "Java 6", "Java", "", "6", "6.0.200",
     "http://www.java.com/" },
-  { "adobe-reader", "Adobe Reader 9", "Adobe Acrobat", "9", "10", "9.4.0",
-    "http://get.adobe.com/reader/" },
-  { "adobe-reader-8", "Adobe Reader 8", "Adobe Acrobat", "0", "9", "8.2.5",
-    "http://get.adobe.com/reader/" },
+  { "adobe-reader", PluginGroup::kAdobeReader9GroupName, "Adobe Acrobat", "9",
+    "10", "9.4.0", "http://get.adobe.com/reader/" },
+  { "adobe-reader-8", PluginGroup::kAdobeReader8GroupName, "Adobe Acrobat", "0",
+    "9", "8.2.5", "http://get.adobe.com/reader/" },
   { "adobe-flash-player", "Flash", "Shockwave Flash", "", "", "10.1.85",
     "http://get.adobe.com/flashplayer/" },
   { "silverlight-3", "Silverlight 3", "Silverlight", "0", "4", "3.0.50106.0",
diff --git a/webkit/glue/plugins/plugin_group.h b/webkit/glue/plugins/plugin_group.h
index fb193d5..2281437 100644
--- a/webkit/glue/plugins/plugin_group.h
+++ b/webkit/glue/plugins/plugin_group.h
@@ -46,6 +46,11 @@ struct PluginGroupDefinition {
 
 class PluginGroup {
  public:
+  // Used by about:plugins to disable Reader plugin when internal PDF viewer is
+  // enabled.
+  static const char* kAdobeReader8GroupName;
+  static const char* kAdobeReader9GroupName;
+
   typedef std::map<std::string, linked_ptr<PluginGroup> > PluginMap;
 
   // Creates a PluginGroup from a PluginGroupDefinition.