diff options
5 files changed, 128 insertions, 12 deletions
diff --git a/chrome/browser/download/download_prefs.cc b/chrome/browser/download/download_prefs.cc index 281adf4..6d1cd67 100644 --- a/chrome/browser/download/download_prefs.cc +++ b/chrome/browser/download/download_prefs.cc @@ -5,6 +5,7 @@ #include "chrome/browser/download/download_prefs.h" #include "base/file_util.h" +#include "base/logging.h" #include "base/string_split.h" #include "base/string_util.h" #include "base/sys_string_conversions.h" @@ -79,7 +80,10 @@ void DownloadPrefs::RegisterUserPrefs(PrefService* prefs) { } bool DownloadPrefs::PromptForDownload() const { - return *prompt_for_download_ && !download_path_.IsManaged(); + // If the DownloadDirectory policy is set, then |prompt_for_download_| should + // always be false. + DCHECK(!download_path_.IsManaged() || !prompt_for_download_.GetValue()); + return *prompt_for_download_; } bool DownloadPrefs::IsDownloadPathManaged() const { diff --git a/chrome/browser/policy/configuration_policy_pref_store.cc b/chrome/browser/policy/configuration_policy_pref_store.cc index b5ab3d0..0d71a3b 100644 --- a/chrome/browser/policy/configuration_policy_pref_store.cc +++ b/chrome/browser/policy/configuration_policy_pref_store.cc @@ -92,6 +92,13 @@ class ConfigurationPolicyPrefKeeper // ownership of |value| in the case that the policy is recognized. bool ApplyDownloadDirPolicy(ConfigurationPolicyType policy, Value* value); + // Processes file-selection dialogs policy. Returns true if the specified + // policy is the file-selection dialogs policy. + // ApplyFileSelectionDialogsPolicy assumes the ownership of |value| in the + // case that the policy is recognized. + bool ApplyFileSelectionDialogsPolicy(ConfigurationPolicyType policy, + Value* value); + // Make sure that the |path| if present in |prefs_|. If not, set it to // a blank string. void EnsureStringPrefExists(const std::string& path); @@ -335,6 +342,9 @@ void ConfigurationPolicyPrefKeeper::Apply(ConfigurationPolicyType policy, if (ApplyDownloadDirPolicy(policy, value)) return; + if (ApplyFileSelectionDialogsPolicy(policy, value)) + return; + if (ApplyPolicyFromMap(policy, value, kDefaultSearchPolicyMap, arraysize(kDefaultSearchPolicyMap))) return; @@ -443,6 +453,27 @@ bool ConfigurationPolicyPrefKeeper::ApplyDownloadDirPolicy( return false; } +bool ConfigurationPolicyPrefKeeper::ApplyFileSelectionDialogsPolicy( + ConfigurationPolicyType policy, + Value* value) { + if (policy == kPolicyAllowFileSelectionDialogs) { + prefs_.SetValue(prefs::kAllowFileSelectionDialogs, value); + // If file-selection dialogs are not allowed we forbid the user to be + // prompted for the download location, since this would end up in an Infobar + // explaining that file-selection dialogs are forbidden anyways. + bool allow_file_selection_dialogs = true; + bool result = value->GetAsBoolean(&allow_file_selection_dialogs); + DCHECK(result); + if (!allow_file_selection_dialogs) { + prefs_.SetValue(prefs::kPromptForDownload, + Value::CreateBooleanValue(false)); + } + return true; + } + // We are not interested in this policy. + return false; +} + void ConfigurationPolicyPrefKeeper::EnsureStringPrefExists( const std::string& path) { std::string value; diff --git a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc index d4f515a..f780430 100644 --- a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc +++ b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc @@ -61,9 +61,10 @@ TEST_P(ConfigurationPolicyPrefStoreListTest, SetValue) { in_value->Append(Value::CreateStringValue("test2,")); provider_.AddPolicy(GetParam().type(), in_value); store_->OnUpdatePolicy(); - const Value* value; + const Value* value = NULL; EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(GetParam().pref_name(), &value)); + ASSERT_TRUE(value); EXPECT_TRUE(in_value->Equals(value)); } @@ -101,9 +102,10 @@ TEST_P(ConfigurationPolicyPrefStoreStringTest, SetValue) { provider_.AddPolicy(GetParam().type(), Value::CreateStringValue("http://chromium.org")); store_->OnUpdatePolicy(); - const Value* value; + const Value* value = NULL; EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(GetParam().pref_name(), &value)); + ASSERT_TRUE(value); EXPECT_TRUE(StringValue("http://chromium.org").Equals(value)); } @@ -142,18 +144,24 @@ TEST_P(ConfigurationPolicyPrefStoreBooleanTest, GetDefault) { TEST_P(ConfigurationPolicyPrefStoreBooleanTest, SetValue) { provider_.AddPolicy(GetParam().type(), Value::CreateBooleanValue(false)); store_->OnUpdatePolicy(); - const Value* value; - bool result = true; + const Value* value = NULL; EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(GetParam().pref_name(), &value)); - EXPECT_TRUE(FundamentalValue(false).Equals(value)); + ASSERT_TRUE(value); + bool boolean_value = true; + bool result = value->GetAsBoolean(&boolean_value); + ASSERT_TRUE(result); + EXPECT_FALSE(boolean_value); provider_.AddPolicy(GetParam().type(), Value::CreateBooleanValue(true)); store_->OnUpdatePolicy(); - result = false; + value = NULL; EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(GetParam().pref_name(), &value)); - EXPECT_TRUE(FundamentalValue(true).Equals(value)); + boolean_value = false; + result = value->GetAsBoolean(&boolean_value); + ASSERT_TRUE(result); + EXPECT_TRUE(boolean_value); } INSTANTIATE_TEST_CASE_P( @@ -673,8 +681,72 @@ TEST_F(ConfigurationPolicyPrefStoreSyncTest, Disabled) { // Sync should be flagged as managed. const Value* value = NULL; EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(prefs::kSyncManaged, &value)); - ASSERT_TRUE(value != NULL); - EXPECT_TRUE(FundamentalValue(true).Equals(value)); + ASSERT_TRUE(value); + bool sync_managed = false; + bool result = value->GetAsBoolean(&sync_managed); + ASSERT_TRUE(result); + EXPECT_TRUE(sync_managed); +} + +// Test cases for how the DownloadDirectory and AllowFileSelectionDialogs policy +// influence the PromptForDownload preference. +class ConfigurationPolicyPrefStorePromptDownloadTest + : public ConfigurationPolicyPrefStoreTestBase<testing::Test> { +}; + +TEST_F(ConfigurationPolicyPrefStorePromptDownloadTest, Default) { + EXPECT_EQ(PrefStore::READ_NO_VALUE, + store_->GetValue(prefs::kPromptForDownload, NULL)); +} + +TEST_F(ConfigurationPolicyPrefStorePromptDownloadTest, SetDownloadDirectory) { + EXPECT_EQ(PrefStore::READ_NO_VALUE, + store_->GetValue(prefs::kPromptForDownload, NULL)); + provider_.AddPolicy(kPolicyDownloadDirectory, Value::CreateStringValue("")); + store_->OnUpdatePolicy(); + + // Setting a DownloadDirectory should disable the PromptForDownload pref. + const Value* value = NULL; + EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(prefs::kPromptForDownload, + &value)); + ASSERT_TRUE(value); + bool prompt_for_download = true; + bool result = value->GetAsBoolean(&prompt_for_download); + ASSERT_TRUE(result); + EXPECT_FALSE(prompt_for_download); +} + +TEST_F(ConfigurationPolicyPrefStorePromptDownloadTest, + EnableFileSelectionDialogs) { + EXPECT_EQ(PrefStore::READ_NO_VALUE, + store_->GetValue(prefs::kPromptForDownload, NULL)); + provider_.AddPolicy(kPolicyAllowFileSelectionDialogs, + Value::CreateBooleanValue(true)); + store_->OnUpdatePolicy(); + + // Allowing file-selection dialogs should not influence the PromptForDownload + // pref. + EXPECT_EQ(PrefStore::READ_NO_VALUE, + store_->GetValue(prefs::kPromptForDownload, NULL)); +} + +TEST_F(ConfigurationPolicyPrefStorePromptDownloadTest, + DisableFileSelectionDialogs) { + EXPECT_EQ(PrefStore::READ_NO_VALUE, + store_->GetValue(prefs::kPromptForDownload, NULL)); + provider_.AddPolicy(kPolicyAllowFileSelectionDialogs, + Value::CreateBooleanValue(false)); + store_->OnUpdatePolicy(); + + // Disabling file-selection dialogs should disable the PromptForDownload pref. + const Value* value = NULL; + EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(prefs::kPromptForDownload, + &value)); + ASSERT_TRUE(value); + bool prompt_for_download = true; + bool result = value->GetAsBoolean(&prompt_for_download); + ASSERT_TRUE(result); + EXPECT_FALSE(prompt_for_download); } // Test cases for the Autofill policy setting. @@ -702,7 +774,11 @@ TEST_F(ConfigurationPolicyPrefStoreAutofillTest, Disabled) { const Value* value = NULL; EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(prefs::kAutofillEnabled, &value)); - EXPECT_TRUE(FundamentalValue(false).Equals(value)); + ASSERT_TRUE(value); + bool autofill_enabled = true; + bool result = value->GetAsBoolean(&autofill_enabled); + ASSERT_TRUE(result); + EXPECT_FALSE(autofill_enabled); } // Exercises the policy refresh mechanism. diff --git a/chrome/browser/policy/managed_prefs_banner_base.cc b/chrome/browser/policy/managed_prefs_banner_base.cc index 2c70167..ac105c5 100644 --- a/chrome/browser/policy/managed_prefs_banner_base.cc +++ b/chrome/browser/policy/managed_prefs_banner_base.cc @@ -94,6 +94,7 @@ void ManagedPrefsBannerBase::Init(PrefService* local_state, AddUserPref(prefs::kProxy); AddUserPref(prefs::kCloudPrintProxyEnabled); AddUserPref(prefs::kDownloadDefaultDirectory); + AddUserPref(prefs::kPromptForDownload); AddUserPref(prefs::kEnableTranslate); AddUserPref(prefs::kChromotingEnabled); AddUserPref(prefs::kChromotingHostEnabled); diff --git a/chrome/browser/ui/webui/options/advanced_options_handler.cc b/chrome/browser/ui/webui/options/advanced_options_handler.cc index b0ac033..071ff37 100644 --- a/chrome/browser/ui/webui/options/advanced_options_handler.cc +++ b/chrome/browser/ui/webui/options/advanced_options_handler.cc @@ -576,7 +576,11 @@ void AdvancedOptionsHandler::SetupDownloadLocationPath() { void AdvancedOptionsHandler::SetupPromptForDownload() { FundamentalValue checked(ask_for_save_location_.GetValue()); - FundamentalValue disabled(default_download_location_.IsManaged()); + // If either the DownloadDirectory is managed or if file-selection dialogs are + // disallowed then |ask_for_save_location_| must currently be false and cannot + // be changed. + FundamentalValue disabled(default_download_location_.IsManaged() || + !allow_file_selection_dialogs_.GetValue()); web_ui_->CallJavascriptFunction( "options.AdvancedOptions.SetPromptForDownload", checked, disabled); } |