Return success value from PluginPrefs::EnablePlugin.

BUG=97179
TEST=chrome/test/functional/enterprise.py


Review URL: http://codereview.chromium.org/7976005

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

6 files changed, 75 insertions, 28 deletions

diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc
index d71ed55..bf88e8c 100644
--- a/chrome/browser/automation/testing_automation_provider.cc
+++ b/chrome/browser/automation/testing_automation_provider.cc
@@ -3575,11 +3575,13 @@ void TestingAutomationProvider::EnablePlugin(Browser* browser,
   AutomationJSONReply reply(this, reply_message);
   if (!args->GetString("path", &path)) {
     reply.SendError("path not specified.");
-    return;
+  } else if (!PluginPrefs::GetForProfile(browser->profile())->EnablePlugin(
+      true, FilePath(path))) {
+    reply.SendError(StringPrintf("Could not enable plugin for path %s.",
+                                 path.c_str()));
+  } else {
+    reply.SendSuccess(NULL);
   }
-  PluginPrefs::GetForProfile(browser->profile())->EnablePlugin(
-      true, FilePath(path));
-  reply.SendSuccess(NULL);
 }
 
 // Sample json input:
@@ -3592,11 +3594,13 @@ void TestingAutomationProvider::DisablePlugin(Browser* browser,
   AutomationJSONReply reply(this, reply_message);
   if (!args->GetString("path", &path)) {
     reply.SendError("path not specified.");
-    return;
+  } else if (!PluginPrefs::GetForProfile(browser->profile())->EnablePlugin(
+      false, FilePath(path))) {
+    reply.SendError(StringPrintf("Could not disable plugin for path %s.",
+                                 path.c_str()));
+  } else {
+    reply.SendSuccess(NULL);
   }
-  PluginPrefs::GetForProfile(browser->profile())->EnablePlugin(
-      false, FilePath(path));
-  reply.SendSuccess(NULL);
 }
 
 // Sample json input:
diff --git a/chrome/browser/plugin_prefs.cc b/chrome/browser/plugin_prefs.cc
index a60a28b..f6592ec5 100644
--- a/chrome/browser/plugin_prefs.cc
+++ b/chrome/browser/plugin_prefs.cc
@@ -111,6 +111,11 @@ PluginPrefs* PluginPrefs::GetForTestingProfile(Profile* profile) {
   return static_cast<PluginPrefsWrapper*>(wrapper)->plugin_prefs();
 }
 
+void PluginPrefs::SetPluginListForTesting(
+    webkit::npapi::PluginList* plugin_list) {
+  plugin_list_ = plugin_list;
+}
+
 void PluginPrefs::EnablePluginGroup(bool enabled, const string16& group_name) {
   if (!BrowserThread::CurrentlyOn(BrowserThread::FILE)) {
     BrowserThread::PostTask(
@@ -120,8 +125,7 @@ void PluginPrefs::EnablePluginGroup(bool enabled, const string16& group_name) {
     return;
   }
 
-  webkit::npapi::PluginList* plugin_list =
-      webkit::npapi::PluginList::Singleton();
+  webkit::npapi::PluginList* plugin_list = GetPluginList();
   std::vector<webkit::npapi::PluginGroup> groups;
   plugin_list->GetPluginGroups(true, &groups);
 
@@ -147,12 +151,29 @@ void PluginPrefs::EnablePluginGroup(bool enabled, const string16& group_name) {
       NewRunnableMethod(this, &PluginPrefs::NotifyPluginStatusChanged));
 }
 
-void PluginPrefs::EnablePlugin(bool enabled, const FilePath& path) {
+bool PluginPrefs::EnablePlugin(bool enabled, const FilePath& path) {
+  // Do policy checks first. These don't need to run on the FILE thread.
+  webkit::npapi::PluginList* plugin_list = GetPluginList();
+  webkit::WebPluginInfo plugin;
+  if (plugin_list->GetPluginInfoByPath(path, &plugin)) {
+    scoped_ptr<webkit::npapi::PluginGroup> group(
+        plugin_list->GetPluginGroup(plugin));
+    PolicyStatus plugin_status = PolicyStatusForPlugin(plugin.name);
+    PolicyStatus group_status = PolicyStatusForPlugin(group->GetGroupName());
+    if (enabled) {
+      if (plugin_status == POLICY_DISABLED || group_status == POLICY_DISABLED)
+        return false;
+    } else {
+      if (plugin_status == POLICY_ENABLED || group_status == POLICY_ENABLED)
+        return false;
+    }
+  }
+
   if (!BrowserThread::CurrentlyOn(BrowserThread::FILE)) {
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
         NewRunnableMethod(this, &PluginPrefs::EnablePlugin, enabled, path));
-    return;
+    return true;
   }
 
   {
@@ -161,8 +182,6 @@ void PluginPrefs::EnablePlugin(bool enabled, const FilePath& path) {
     plugin_state_[path] = enabled;
   }
 
-  webkit::npapi::PluginList* plugin_list =
-      webkit::npapi::PluginList::Singleton();
   std::vector<webkit::npapi::PluginGroup> groups;
   plugin_list->GetPluginGroups(true, &groups);
 
@@ -190,20 +209,25 @@ void PluginPrefs::EnablePlugin(bool enabled, const FilePath& path) {
       NewRunnableMethod(this, &PluginPrefs::OnUpdatePreferences, groups));
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
       NewRunnableMethod(this, &PluginPrefs::NotifyPluginStatusChanged));
+
+  return true;
 }
 
 // static
-void PluginPrefs::EnablePluginGlobally(bool enable, const FilePath& file_path) {
+bool PluginPrefs::EnablePluginGlobally(bool enable, const FilePath& file_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   g_default_plugin_state.Get()[file_path] = enable;
   std::vector<Profile*> profiles =
       g_browser_process->profile_manager()->GetLoadedProfiles();
+  bool result = true;
   for (std::vector<Profile*>::iterator it = profiles.begin();
        it != profiles.end(); ++it) {
     PluginPrefs* plugin_prefs = PluginPrefs::GetForProfile(*it);
     DCHECK(plugin_prefs);
-    plugin_prefs->EnablePlugin(enable, file_path);
+    if (!plugin_prefs->EnablePlugin(enable, file_path))
+      result = false;
   }
+  return result;
 }
 
 PluginPrefs::PolicyStatus PluginPrefs::PolicyStatusForPlugin(
@@ -222,7 +246,7 @@ PluginPrefs::PolicyStatus PluginPrefs::PolicyStatusForPlugin(
 
 bool PluginPrefs::IsPluginEnabled(const webkit::WebPluginInfo& plugin) {
   scoped_ptr<webkit::npapi::PluginGroup> group(
-      webkit::npapi::PluginList::Singleton()->GetPluginGroup(plugin));
+      GetPluginList()->GetPluginGroup(plugin));
   string16 group_name = group->GetGroupName();
 
   // Check if the plug-in or its group is enabled by policy.
@@ -505,7 +529,8 @@ ProfileKeyedService* PluginPrefs::Factory::BuildServiceInstanceFor(
 }
 
 PluginPrefs::PluginPrefs() : plugin_state_(g_default_plugin_state.Get()),
-                             prefs_(NULL) {
+                             prefs_(NULL),
+                             plugin_list_(NULL) {
 }
 
 PluginPrefs::~PluginPrefs() {
@@ -520,11 +545,16 @@ void PluginPrefs::SetPolicyEnforcedPluginPatterns(
   policy_enabled_plugin_patterns_ = enabled_patterns;
 }
 
+webkit::npapi::PluginList* PluginPrefs::GetPluginList() {
+  if (plugin_list_)
+    return plugin_list_;
+  return webkit::npapi::PluginList::Singleton();
+}
+
 void PluginPrefs::GetPreferencesDataOnFileThread() {
   std::vector<webkit::npapi::PluginGroup> groups;
 
-  webkit::npapi::PluginList* plugin_list =
-      webkit::npapi::PluginList::Singleton();
+  webkit::npapi::PluginList* plugin_list = GetPluginList();
   plugin_list->GetPluginGroups(false, &groups);
 
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
diff --git a/chrome/browser/plugin_prefs.h b/chrome/browser/plugin_prefs.h
index ecfd585..9201dbf 100644
--- a/chrome/browser/plugin_prefs.h
+++ b/chrome/browser/plugin_prefs.h
@@ -34,6 +34,7 @@ namespace webkit {
 struct WebPluginInfo;
 namespace npapi {
 class PluginGroup;
+class PluginList;
 }
 }
 
@@ -61,6 +62,9 @@ class PluginPrefs : public base::RefCountedThreadSafe<PluginPrefs>,
   // created PluginPrefs object.
   static PluginPrefs* GetForTestingProfile(Profile* profile);
 
+  // Sets the plug-in list for tests.
+  void SetPluginListForTesting(webkit::npapi::PluginList* plugin_list);
+
   // Creates a new instance. This method should only be used for testing.
   PluginPrefs();
 
@@ -77,12 +81,12 @@ class PluginPrefs : public base::RefCountedThreadSafe<PluginPrefs>,
   void EnablePluginGroup(bool enable, const string16& group_name);
 
   // Enable or disable a specific plugin file.
-  void EnablePlugin(bool enable, const FilePath& file_path);
+  bool EnablePlugin(bool enable, const FilePath& file_path);
 
   // Enable or disable a plug-in in all profiles. This sets a default for
   // profiles which are created later as well.
   // This method should only be called on the UI thread.
-  static void EnablePluginGlobally(bool enable, const FilePath& file_path);
+  static bool EnablePluginGlobally(bool enable, const FilePath& file_path);
 
   // Returns whether there is a policy enabling or disabling plug-ins of the
   // given name.
@@ -114,6 +118,9 @@ class PluginPrefs : public base::RefCountedThreadSafe<PluginPrefs>,
       const std::set<string16>& disabled_exception_patterns,
       const std::set<string16>& enabled_patterns);
 
+  // Returns the plugin list to use, either the singleton or the override.
+  webkit::npapi::PluginList* GetPluginList();
+
   // Called on the file thread to get the data necessary to update the saved
   // preferences.
   void GetPreferencesDataOnFileThread();
@@ -144,6 +151,10 @@ class PluginPrefs : public base::RefCountedThreadSafe<PluginPrefs>,
   // Weak pointer, owned by the profile (which owns us).
   PrefService* prefs_;
 
+  // PluginList to use for testing. If this is NULL, defaults to the global
+  // singleton.
+  webkit::npapi::PluginList* plugin_list_;
+
   PrefChangeRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(PluginPrefs);
diff --git a/chrome/browser/plugin_prefs_unittest.cc b/chrome/browser/plugin_prefs_unittest.cc
index 934d478..6ee2317 100644
--- a/chrome/browser/plugin_prefs_unittest.cc
+++ b/chrome/browser/plugin_prefs_unittest.cc
@@ -9,6 +9,7 @@
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/testing_profile_manager.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "webkit/plugins/npapi/mock_plugin_list.cc"
 #include "webkit/plugins/webplugininfo.h"
 
 class PluginPrefsTest : public ::testing::Test {
@@ -148,7 +149,10 @@ TEST_F(PluginPrefsTest, DisableGlobally) {
                                FilePath(FILE_PATH_LITERAL("/path/too/foo")),
                                ASCIIToUTF16("1.0.0"),
                                ASCIIToUTF16("Foo plug-in"));
-  PluginPrefs::EnablePluginGlobally(false, plugin.path);
+  webkit::npapi::MockPluginList plugin_list(NULL, 0);
+  plugin_list.AddPluginToLoad(plugin);
+  plugin_prefs->SetPluginListForTesting(&plugin_list);
+  EXPECT_TRUE(PluginPrefs::EnablePluginGlobally(false, plugin.path));
 
   EXPECT_FALSE(plugin_prefs->IsPluginEnabled(plugin));
 
@@ -156,5 +160,6 @@ TEST_F(PluginPrefsTest, DisableGlobally) {
       profile_manager.CreateTestingProfile("Profile 2");
   PluginPrefs* plugin_prefs_2 = PluginPrefs::GetForTestingProfile(profile_2);
   ASSERT_TRUE(plugin_prefs);
+  plugin_prefs_2->SetPluginListForTesting(&plugin_list);
   EXPECT_FALSE(plugin_prefs_2->IsPluginEnabled(plugin));
 }
diff --git a/chrome/browser/ui/webui/plugins_ui.cc b/chrome/browser/ui/webui/plugins_ui.cc
index d07b1bb..1538fff 100644
--- a/chrome/browser/ui/webui/plugins_ui.cc
+++ b/chrome/browser/ui/webui/plugins_ui.cc
@@ -205,7 +205,8 @@ void PluginsDOMHandler::HandleEnablePluginMessage(const ListValue* args) {
     FilePath::StringType file_path;
     if (!args->GetString(0, &file_path))
       return;
-    plugin_prefs->EnablePlugin(enable, FilePath(file_path));
+    bool result = plugin_prefs->EnablePlugin(enable, FilePath(file_path));
+    DCHECK(result);
   }
 }
 
diff --git a/chrome/test/functional/PYAUTO_TESTS b/chrome/test/functional/PYAUTO_TESTS
index b673d96..a4e7f35 100644
--- a/chrome/test/functional/PYAUTO_TESTS
+++ b/chrome/test/functional/PYAUTO_TESTS
@@ -261,10 +261,6 @@
       # ==================================================
       # Disabled tests that need to be investigated/fixed.
       # ==================================================
-      # crbug.com/97179
-      '-enterprise.EnterpriseTest.testAlwaysAuthorizePlugins',
-      '-enterprise.EnterpriseTest.testDisabledPlugins',
-      '-enterprise.EnterpriseTest.testEnabledPlugins',
       # crbug.com/91225
       '-find_in_page.FindMatchTests.testSearchInPDF',
       # crbug.com/91226