diff options
25 files changed, 214 insertions, 332 deletions
diff --git a/base/prefs/default_pref_store.cc b/base/prefs/default_pref_store.cc index eb14a50..8e2644c 100644 --- a/base/prefs/default_pref_store.cc +++ b/base/prefs/default_pref_store.cc @@ -9,25 +9,25 @@ using base::Value; DefaultPrefStore::DefaultPrefStore() {} -PrefStore::ReadResult DefaultPrefStore::GetValue( +bool DefaultPrefStore::GetValue( const std::string& key, const base::Value** result) const { - return prefs_.GetValue(key, result) ? READ_OK : READ_NO_VALUE; + return prefs_.GetValue(key, result); } void DefaultPrefStore::SetDefaultValue(const std::string& key, Value* value) { - CHECK(GetValue(key, NULL) == READ_NO_VALUE); + DCHECK(!GetValue(key, NULL)); prefs_.SetValue(key, value); } void DefaultPrefStore::RemoveDefaultValue(const std::string& key) { - CHECK(GetValue(key, NULL) == READ_OK); + DCHECK(GetValue(key, NULL)); prefs_.RemoveValue(key); } base::Value::Type DefaultPrefStore::GetType(const std::string& key) const { - const Value* value; - return GetValue(key, &value) == READ_OK ? value->GetType() : Value::TYPE_NULL; + const Value* value = NULL; + return GetValue(key, &value) ? value->GetType() : Value::TYPE_NULL; } DefaultPrefStore::const_iterator DefaultPrefStore::begin() const { diff --git a/base/prefs/default_pref_store.h b/base/prefs/default_pref_store.h index e6fdbfa..996bf8b 100644 --- a/base/prefs/default_pref_store.h +++ b/base/prefs/default_pref_store.h @@ -20,8 +20,8 @@ class BASE_PREFS_EXPORT DefaultPrefStore : public PrefStore { DefaultPrefStore(); - virtual ReadResult GetValue(const std::string& key, - const base::Value** result) const OVERRIDE; + virtual bool GetValue(const std::string& key, + const base::Value** result) const OVERRIDE; // Stores a new |value| for |key|. Assumes ownership of |value|. void SetDefaultValue(const std::string& key, Value* value); diff --git a/base/prefs/json_pref_store.cc b/base/prefs/json_pref_store.cc index 9e1e777..0447975 100644 --- a/base/prefs/json_pref_store.cc +++ b/base/prefs/json_pref_store.cc @@ -162,15 +162,15 @@ JsonPrefStore::JsonPrefStore(const FilePath& filename, read_error_(PREF_READ_ERROR_OTHER) { } -PrefStore::ReadResult JsonPrefStore::GetValue(const std::string& key, - const Value** result) const { +bool JsonPrefStore::GetValue(const std::string& key, + const Value** result) const { Value* tmp = NULL; - if (prefs_->Get(key, &tmp)) { - if (result) - *result = tmp; - return READ_OK; - } - return READ_NO_VALUE; + if (!prefs_->Get(key, &tmp)) + return false; + + if (result) + *result = tmp; + return true; } void JsonPrefStore::AddObserver(PrefStore::Observer* observer) { @@ -189,9 +189,9 @@ bool JsonPrefStore::IsInitializationComplete() const { return initialized_; } -PrefStore::ReadResult JsonPrefStore::GetMutableValue(const std::string& key, - Value** result) { - return prefs_->Get(key, result) ? READ_OK : READ_NO_VALUE; +bool JsonPrefStore::GetMutableValue(const std::string& key, + Value** result) { + return prefs_->Get(key, result); } void JsonPrefStore::SetValue(const std::string& key, Value* value) { diff --git a/base/prefs/json_pref_store.h b/base/prefs/json_pref_store.h index 7a75d4c..49dd71a 100644 --- a/base/prefs/json_pref_store.h +++ b/base/prefs/json_pref_store.h @@ -44,16 +44,16 @@ class BASE_PREFS_EXPORT JsonPrefStore base::SequencedTaskRunner* sequenced_task_runner); // PrefStore overrides: - virtual ReadResult GetValue(const std::string& key, - const base::Value** result) const OVERRIDE; + virtual bool GetValue(const std::string& key, + const base::Value** result) const OVERRIDE; virtual void AddObserver(PrefStore::Observer* observer) OVERRIDE; virtual void RemoveObserver(PrefStore::Observer* observer) OVERRIDE; virtual size_t NumberOfObservers() const OVERRIDE; virtual bool IsInitializationComplete() const OVERRIDE; // PersistentPrefStore overrides: - virtual ReadResult GetMutableValue(const std::string& key, - base::Value** result) OVERRIDE; + virtual bool GetMutableValue(const std::string& key, + base::Value** result) OVERRIDE; virtual void SetValue(const std::string& key, base::Value* value) OVERRIDE; virtual void SetValueSilently(const std::string& key, base::Value* value) OVERRIDE; diff --git a/base/prefs/json_pref_store_unittest.cc b/base/prefs/json_pref_store_unittest.cc index e7239bc..5258660 100644 --- a/base/prefs/json_pref_store_unittest.cc +++ b/base/prefs/json_pref_store_unittest.cc @@ -96,15 +96,14 @@ void RunBasicJsonPrefStoreTest(JsonPrefStore* pref_store, std::string cnn("http://www.cnn.com"); const Value* actual; - EXPECT_EQ(PrefStore::READ_OK, - pref_store->GetValue(prefs::kHomePage, &actual)); + EXPECT_TRUE(pref_store->GetValue(prefs::kHomePage, &actual)); std::string string_value; EXPECT_TRUE(actual->GetAsString(&string_value)); EXPECT_EQ(cnn, string_value); const char kSomeDirectory[] = "some_directory"; - EXPECT_EQ(PrefStore::READ_OK, pref_store->GetValue(kSomeDirectory, &actual)); + EXPECT_TRUE(pref_store->GetValue(kSomeDirectory, &actual)); FilePath::StringType path; EXPECT_TRUE(actual->GetAsString(&path)); EXPECT_EQ(FilePath::StringType(FILE_PATH_LITERAL("/usr/local/")), path); @@ -112,37 +111,35 @@ void RunBasicJsonPrefStoreTest(JsonPrefStore* pref_store, pref_store->SetValue(kSomeDirectory, Value::CreateStringValue(some_path.value())); - EXPECT_EQ(PrefStore::READ_OK, pref_store->GetValue(kSomeDirectory, &actual)); + EXPECT_TRUE(pref_store->GetValue(kSomeDirectory, &actual)); EXPECT_TRUE(actual->GetAsString(&path)); EXPECT_EQ(some_path.value(), path); // Test reading some other data types from sub-dictionaries. - EXPECT_EQ(PrefStore::READ_OK, - pref_store->GetValue(kNewWindowsInTabs, &actual)); + EXPECT_TRUE(pref_store->GetValue(kNewWindowsInTabs, &actual)); bool boolean = false; EXPECT_TRUE(actual->GetAsBoolean(&boolean)); EXPECT_TRUE(boolean); pref_store->SetValue(kNewWindowsInTabs, Value::CreateBooleanValue(false)); - EXPECT_EQ(PrefStore::READ_OK, - pref_store->GetValue(kNewWindowsInTabs, &actual)); + EXPECT_TRUE(pref_store->GetValue(kNewWindowsInTabs, &actual)); EXPECT_TRUE(actual->GetAsBoolean(&boolean)); EXPECT_FALSE(boolean); - EXPECT_EQ(PrefStore::READ_OK, pref_store->GetValue(kMaxTabs, &actual)); + EXPECT_TRUE(pref_store->GetValue(kMaxTabs, &actual)); int integer = 0; EXPECT_TRUE(actual->GetAsInteger(&integer)); EXPECT_EQ(20, integer); pref_store->SetValue(kMaxTabs, Value::CreateIntegerValue(10)); - EXPECT_EQ(PrefStore::READ_OK, pref_store->GetValue(kMaxTabs, &actual)); + EXPECT_TRUE(pref_store->GetValue(kMaxTabs, &actual)); EXPECT_TRUE(actual->GetAsInteger(&integer)); EXPECT_EQ(10, integer); pref_store->SetValue(kLongIntPref, Value::CreateStringValue( base::Int64ToString(214748364842LL))); - EXPECT_EQ(PrefStore::READ_OK, pref_store->GetValue(kLongIntPref, &actual)); + EXPECT_TRUE(pref_store->GetValue(kLongIntPref, &actual)); EXPECT_TRUE(actual->GetAsString(&string_value)); int64 value; base::StringToInt64(string_value, &value); diff --git a/base/prefs/overlay_user_pref_store.cc b/base/prefs/overlay_user_pref_store.cc index 26ee45e..3a74a5c 100644 --- a/base/prefs/overlay_user_pref_store.cc +++ b/base/prefs/overlay_user_pref_store.cc @@ -33,41 +33,37 @@ bool OverlayUserPrefStore::IsInitializationComplete() const { return underlay_->IsInitializationComplete(); } -PrefStore::ReadResult OverlayUserPrefStore::GetValue( - const std::string& key, - const Value** result) const { +bool OverlayUserPrefStore::GetValue(const std::string& key, + const Value** result) const { // If the |key| shall NOT be stored in the overlay store, there must not // be an entry. DCHECK(ShallBeStoredInOverlay(key) || !overlay_.GetValue(key, NULL)); if (overlay_.GetValue(key, result)) - return READ_OK; + return true; return underlay_->GetValue(GetUnderlayKey(key), result); } -PrefStore::ReadResult OverlayUserPrefStore::GetMutableValue( - const std::string& key, - Value** result) { +bool OverlayUserPrefStore::GetMutableValue(const std::string& key, + Value** result) { if (!ShallBeStoredInOverlay(key)) return underlay_->GetMutableValue(GetUnderlayKey(key), result); if (overlay_.GetValue(key, result)) - return READ_OK; + return true; // Try to create copy of underlay if the overlay does not contain a value. Value* underlay_value = NULL; - PrefStore::ReadResult read_result = - underlay_->GetMutableValue(GetUnderlayKey(key), &underlay_value); - if (read_result != READ_OK) - return read_result; + if (!underlay_->GetMutableValue(GetUnderlayKey(key), &underlay_value)) + return false; *result = underlay_value->DeepCopy(); overlay_.SetValue(key, *result); - return READ_OK; + return true; } void OverlayUserPrefStore::SetValue(const std::string& key, - Value* value) { + Value* value) { if (!ShallBeStoredInOverlay(key)) { underlay_->SetValue(GetUnderlayKey(key), value); return; @@ -78,7 +74,7 @@ void OverlayUserPrefStore::SetValue(const std::string& key, } void OverlayUserPrefStore::SetValueSilently(const std::string& key, - Value* value) { + Value* value) { if (!ShallBeStoredInOverlay(key)) { underlay_->SetValueSilently(GetUnderlayKey(key), value); return; diff --git a/base/prefs/overlay_user_pref_store.h b/base/prefs/overlay_user_pref_store.h index 0b105aa..120d405 100644 --- a/base/prefs/overlay_user_pref_store.h +++ b/base/prefs/overlay_user_pref_store.h @@ -34,12 +34,12 @@ class BASE_PREFS_EXPORT OverlayUserPrefStore : public PersistentPrefStore, virtual void RemoveObserver(PrefStore::Observer* observer) OVERRIDE; virtual size_t NumberOfObservers() const OVERRIDE; virtual bool IsInitializationComplete() const OVERRIDE; - virtual ReadResult GetValue(const std::string& key, - const base::Value** result) const OVERRIDE; + virtual bool GetValue(const std::string& key, + const base::Value** result) const OVERRIDE; // Methods of PersistentPrefStore. - virtual ReadResult GetMutableValue(const std::string& key, - base::Value** result) OVERRIDE; + virtual bool GetMutableValue(const std::string& key, + base::Value** result) OVERRIDE; virtual void SetValue(const std::string& key, base::Value* value) OVERRIDE; virtual void SetValueSilently(const std::string& key, base::Value* value) OVERRIDE; diff --git a/base/prefs/overlay_user_pref_store_unittest.cc b/base/prefs/overlay_user_pref_store_unittest.cc index 85b937f..d6615af 100644 --- a/base/prefs/overlay_user_pref_store_unittest.cc +++ b/base/prefs/overlay_user_pref_store_unittest.cc @@ -89,35 +89,33 @@ TEST_F(OverlayUserPrefStoreTest, Observer) { TEST_F(OverlayUserPrefStoreTest, GetAndSet) { const Value* value = NULL; - EXPECT_EQ(PrefStore::READ_NO_VALUE, - overlay_->GetValue(overlay_key, &value)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - underlay_->GetValue(overlay_key, &value)); + EXPECT_FALSE(overlay_->GetValue(overlay_key, &value)); + EXPECT_FALSE(underlay_->GetValue(overlay_key, &value)); underlay_->SetValue(overlay_key, Value::CreateIntegerValue(42)); // Value shines through: - EXPECT_EQ(PrefStore::READ_OK, overlay_->GetValue(overlay_key, &value)); + EXPECT_TRUE(overlay_->GetValue(overlay_key, &value)); EXPECT_TRUE(base::FundamentalValue(42).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, underlay_->GetValue(overlay_key, &value)); + EXPECT_TRUE(underlay_->GetValue(overlay_key, &value)); EXPECT_TRUE(base::FundamentalValue(42).Equals(value)); overlay_->SetValue(overlay_key, Value::CreateIntegerValue(43)); - EXPECT_EQ(PrefStore::READ_OK, overlay_->GetValue(overlay_key, &value)); + EXPECT_TRUE(overlay_->GetValue(overlay_key, &value)); EXPECT_TRUE(base::FundamentalValue(43).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, underlay_->GetValue(overlay_key, &value)); + EXPECT_TRUE(underlay_->GetValue(overlay_key, &value)); EXPECT_TRUE(base::FundamentalValue(42).Equals(value)); overlay_->RemoveValue(overlay_key); // Value shines through: - EXPECT_EQ(PrefStore::READ_OK, overlay_->GetValue(overlay_key, &value)); + EXPECT_TRUE(overlay_->GetValue(overlay_key, &value)); EXPECT_TRUE(base::FundamentalValue(42).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, underlay_->GetValue(overlay_key, &value)); + EXPECT_TRUE(underlay_->GetValue(overlay_key, &value)); EXPECT_TRUE(base::FundamentalValue(42).Equals(value)); } @@ -126,22 +124,19 @@ TEST_F(OverlayUserPrefStoreTest, ModifyDictionaries) { underlay_->SetValue(overlay_key, new DictionaryValue); Value* modify = NULL; - EXPECT_EQ(PrefStore::READ_OK, - overlay_->GetMutableValue(overlay_key, &modify)); + EXPECT_TRUE(overlay_->GetMutableValue(overlay_key, &modify)); ASSERT_TRUE(modify); ASSERT_TRUE(modify->IsType(Value::TYPE_DICTIONARY)); static_cast<DictionaryValue*>(modify)->SetInteger(overlay_key, 42); Value* original_in_underlay = NULL; - EXPECT_EQ(PrefStore::READ_OK, - underlay_->GetMutableValue(overlay_key, &original_in_underlay)); + EXPECT_TRUE(underlay_->GetMutableValue(overlay_key, &original_in_underlay)); ASSERT_TRUE(original_in_underlay); ASSERT_TRUE(original_in_underlay->IsType(Value::TYPE_DICTIONARY)); EXPECT_TRUE(static_cast<DictionaryValue*>(original_in_underlay)->empty()); Value* modified = NULL; - EXPECT_EQ(PrefStore::READ_OK, - overlay_->GetMutableValue(overlay_key, &modified)); + EXPECT_TRUE(overlay_->GetMutableValue(overlay_key, &modified)); ASSERT_TRUE(modified); ASSERT_TRUE(modified->IsType(Value::TYPE_DICTIONARY)); EXPECT_TRUE(Value::Equals(modify, static_cast<DictionaryValue*>(modified))); @@ -165,7 +160,7 @@ TEST_F(OverlayUserPrefStoreTest, GlobalPref) { Mock::VerifyAndClearExpectations(&obs); // Check that we get this value from the overlay - EXPECT_EQ(PrefStore::READ_OK, overlay_->GetValue(regular_key, &value)); + EXPECT_TRUE(overlay_->GetValue(regular_key, &value)); EXPECT_TRUE(base::FundamentalValue(43).Equals(value)); // Check that overwriting change in overlay is reported. @@ -174,9 +169,9 @@ TEST_F(OverlayUserPrefStoreTest, GlobalPref) { Mock::VerifyAndClearExpectations(&obs); // Check that we get this value from the overlay and the underlay. - EXPECT_EQ(PrefStore::READ_OK, overlay_->GetValue(regular_key, &value)); + EXPECT_TRUE(overlay_->GetValue(regular_key, &value)); EXPECT_TRUE(base::FundamentalValue(44).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, underlay_->GetValue(regular_key, &value)); + EXPECT_TRUE(underlay_->GetValue(regular_key, &value)); EXPECT_TRUE(base::FundamentalValue(44).Equals(value)); // Check that overlay remove is reported. @@ -185,8 +180,8 @@ TEST_F(OverlayUserPrefStoreTest, GlobalPref) { Mock::VerifyAndClearExpectations(&obs); // Check that value was removed from overlay and underlay - EXPECT_EQ(PrefStore::READ_NO_VALUE, overlay_->GetValue(regular_key, &value)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, underlay_->GetValue(regular_key, &value)); + EXPECT_FALSE(overlay_->GetValue(regular_key, &value)); + EXPECT_FALSE(underlay_->GetValue(regular_key, &value)); // Check respecting of silence. EXPECT_CALL(obs, OnPrefValueChanged(StrEq(regular_key))).Times(0); @@ -221,11 +216,10 @@ TEST_F(OverlayUserPrefStoreTest, NamesMapping) { Mock::VerifyAndClearExpectations(&obs); // Check that we get this value from the overlay with both keys - EXPECT_EQ(PrefStore::READ_OK, overlay_->GetValue(mapped_overlay_key, &value)); + EXPECT_TRUE(overlay_->GetValue(mapped_overlay_key, &value)); EXPECT_TRUE(base::FundamentalValue(43).Equals(value)); // In this case, overlay reads directly from the underlay. - EXPECT_EQ(PrefStore::READ_OK, - overlay_->GetValue(mapped_underlay_key, &value)); + EXPECT_TRUE(overlay_->GetValue(mapped_underlay_key, &value)); EXPECT_TRUE(base::FundamentalValue(43).Equals(value)); // Check that overwriting change in overlay is reported. @@ -235,13 +229,11 @@ TEST_F(OverlayUserPrefStoreTest, NamesMapping) { // Check that we get an overriden value from overlay, while reading the // value from underlay still holds an old value. - EXPECT_EQ(PrefStore::READ_OK, overlay_->GetValue(mapped_overlay_key, &value)); + EXPECT_TRUE(overlay_->GetValue(mapped_overlay_key, &value)); EXPECT_TRUE(base::FundamentalValue(44).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, - overlay_->GetValue(mapped_underlay_key, &value)); + EXPECT_TRUE(overlay_->GetValue(mapped_underlay_key, &value)); EXPECT_TRUE(base::FundamentalValue(43).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, - underlay_->GetValue(mapped_underlay_key, &value)); + EXPECT_TRUE(underlay_->GetValue(mapped_underlay_key, &value)); EXPECT_TRUE(base::FundamentalValue(43).Equals(value)); // Check that hidden underlay change is not reported. @@ -260,10 +252,8 @@ TEST_F(OverlayUserPrefStoreTest, NamesMapping) { Mock::VerifyAndClearExpectations(&obs); // Check that value was removed. - EXPECT_EQ(PrefStore::READ_NO_VALUE, - overlay_->GetValue(mapped_overlay_key, &value)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - overlay_->GetValue(mapped_underlay_key, &value)); + EXPECT_FALSE(overlay_->GetValue(mapped_overlay_key, &value)); + EXPECT_FALSE(overlay_->GetValue(mapped_underlay_key, &value)); // Check respecting of silence. EXPECT_CALL(obs, OnPrefValueChanged(StrEq(mapped_overlay_key))).Times(0); diff --git a/base/prefs/persistent_pref_store.h b/base/prefs/persistent_pref_store.h index 7f5b97c..0baf02a 100644 --- a/base/prefs/persistent_pref_store.h +++ b/base/prefs/persistent_pref_store.h @@ -41,8 +41,8 @@ class BASE_PREFS_EXPORT PersistentPrefStore : public PrefStore { }; // Equivalent to PrefStore::GetValue but returns a mutable value. - virtual ReadResult GetMutableValue(const std::string& key, - base::Value** result) = 0; + virtual bool GetMutableValue(const std::string& key, + base::Value** result) = 0; // Triggers a value changed notification. This function needs to be called // if one retrieves a list or dictionary with GetMutableValue and change its diff --git a/base/prefs/pref_store.h b/base/prefs/pref_store.h index 264c1d4..2239528 100644 --- a/base/prefs/pref_store.h +++ b/base/prefs/pref_store.h @@ -36,16 +36,6 @@ class BASE_PREFS_EXPORT PrefStore : public base::RefCounted<PrefStore> { virtual ~Observer() {} }; - // Return values for GetValue(). - enum ReadResult { - // Value found and returned. - READ_OK, - // No value present, but skip other pref stores and use default. - READ_USE_DEFAULT, - // No value present. - READ_NO_VALUE, - }; - PrefStore() {} // Add and remove observers. @@ -57,10 +47,10 @@ class BASE_PREFS_EXPORT PrefStore : public base::RefCounted<PrefStore> { virtual bool IsInitializationComplete() const; // Get the value for a given preference |key| and stores it in |*result|. - // |*result| is only modified if the return value is READ_OK and if |result| + // |*result| is only modified if the return value is true and if |result| // is not NULL. Ownership of the |*result| value remains with the PrefStore. - virtual ReadResult GetValue(const std::string& key, - const base::Value** result) const = 0; + virtual bool GetValue(const std::string& key, + const base::Value** result) const = 0; protected: friend class base::RefCounted<PrefStore>; diff --git a/base/prefs/testing_pref_store.cc b/base/prefs/testing_pref_store.cc index f7c7830..d40fb8a 100644 --- a/base/prefs/testing_pref_store.cc +++ b/base/prefs/testing_pref_store.cc @@ -12,14 +12,14 @@ TestingPrefStore::TestingPrefStore() init_complete_(false) { } -PrefStore::ReadResult TestingPrefStore::GetValue(const std::string& key, - const Value** value) const { - return prefs_.GetValue(key, value) ? READ_OK : READ_NO_VALUE; +bool TestingPrefStore::GetValue(const std::string& key, + const Value** value) const { + return prefs_.GetValue(key, value); } -PrefStore::ReadResult TestingPrefStore::GetMutableValue(const std::string& key, - Value** value) { - return prefs_.GetValue(key, value) ? READ_OK : READ_NO_VALUE; +bool TestingPrefStore::GetMutableValue(const std::string& key, + Value** value) { + return prefs_.GetValue(key, value); } void TestingPrefStore::AddObserver(PrefStore::Observer* observer) { diff --git a/base/prefs/testing_pref_store.h b/base/prefs/testing_pref_store.h index 07e1401..a9f1e92 100644 --- a/base/prefs/testing_pref_store.h +++ b/base/prefs/testing_pref_store.h @@ -21,16 +21,16 @@ class TestingPrefStore : public PersistentPrefStore { TestingPrefStore(); // Overriden from PrefStore. - virtual ReadResult GetValue(const std::string& key, - const base::Value** result) const OVERRIDE; + virtual bool GetValue(const std::string& key, + const base::Value** result) const OVERRIDE; virtual void AddObserver(PrefStore::Observer* observer) OVERRIDE; virtual void RemoveObserver(PrefStore::Observer* observer) OVERRIDE; virtual size_t NumberOfObservers() const OVERRIDE; virtual bool IsInitializationComplete() const OVERRIDE; // PersistentPrefStore overrides: - virtual ReadResult GetMutableValue(const std::string& key, - base::Value** result) OVERRIDE; + virtual bool GetMutableValue(const std::string& key, + base::Value** result) OVERRIDE; virtual void ReportValueChanged(const std::string& key) OVERRIDE; virtual void SetValue(const std::string& key, base::Value* value) OVERRIDE; virtual void SetValueSilently(const std::string& key, diff --git a/base/prefs/value_map_pref_store.cc b/base/prefs/value_map_pref_store.cc index f1eb440..1d58aa3 100644 --- a/base/prefs/value_map_pref_store.cc +++ b/base/prefs/value_map_pref_store.cc @@ -11,9 +11,9 @@ ValueMapPrefStore::ValueMapPrefStore() {} -PrefStore::ReadResult ValueMapPrefStore::GetValue(const std::string& key, - const Value** value) const { - return prefs_.GetValue(key, value) ? READ_OK : READ_NO_VALUE; +bool ValueMapPrefStore::GetValue(const std::string& key, + const Value** value) const { + return prefs_.GetValue(key, value); } void ValueMapPrefStore::AddObserver(PrefStore::Observer* observer) { diff --git a/base/prefs/value_map_pref_store.h b/base/prefs/value_map_pref_store.h index dc79939..c9c9b1c 100644 --- a/base/prefs/value_map_pref_store.h +++ b/base/prefs/value_map_pref_store.h @@ -21,8 +21,8 @@ class BASE_PREFS_EXPORT ValueMapPrefStore : public PrefStore { ValueMapPrefStore(); // PrefStore overrides: - virtual ReadResult GetValue(const std::string& key, - const base::Value** value) const OVERRIDE; + virtual bool GetValue(const std::string& key, + const base::Value** value) const OVERRIDE; virtual void AddObserver(PrefStore::Observer* observer) OVERRIDE; virtual void RemoveObserver(PrefStore::Observer* observer) OVERRIDE; virtual size_t NumberOfObservers() const OVERRIDE; diff --git a/chrome/browser/policy/configuration_policy_pref_store.cc b/chrome/browser/policy/configuration_policy_pref_store.cc index 968b95f..1449a6d 100644 --- a/chrome/browser/policy/configuration_policy_pref_store.cc +++ b/chrome/browser/policy/configuration_policy_pref_store.cc @@ -65,16 +65,15 @@ bool ConfigurationPolicyPrefStore::IsInitializationComplete() const { return policy_service_->IsInitializationComplete(); } -PrefStore::ReadResult -ConfigurationPolicyPrefStore::GetValue(const std::string& key, - const Value** value) const { +bool ConfigurationPolicyPrefStore::GetValue(const std::string& key, + const Value** value) const { const Value* stored_value = NULL; if (!prefs_.get() || !prefs_->GetValue(key, &stored_value)) - return PrefStore::READ_NO_VALUE; + return false; if (value) *value = stored_value; - return PrefStore::READ_OK; + return true; } void ConfigurationPolicyPrefStore::OnPolicyUpdated( diff --git a/chrome/browser/policy/configuration_policy_pref_store.h b/chrome/browser/policy/configuration_policy_pref_store.h index a3ee36b..1611b28 100644 --- a/chrome/browser/policy/configuration_policy_pref_store.h +++ b/chrome/browser/policy/configuration_policy_pref_store.h @@ -35,8 +35,8 @@ class ConfigurationPolicyPrefStore virtual void RemoveObserver(PrefStore::Observer* observer) OVERRIDE; virtual size_t NumberOfObservers() const OVERRIDE; virtual bool IsInitializationComplete() const OVERRIDE; - virtual ReadResult GetValue(const std::string& key, - const Value** result) const OVERRIDE; + virtual bool GetValue(const std::string& key, + const Value** result) const OVERRIDE; // PolicyService::Observer methods: virtual void OnPolicyUpdated(PolicyDomain domain, diff --git a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc index cf5f0c5..2b5ea8e 100644 --- a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc +++ b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc @@ -69,8 +69,7 @@ class ConfigurationPolicyPrefStoreListTest public testing::WithParamInterface<PolicyAndPref> {}; TEST_P(ConfigurationPolicyPrefStoreListTest, GetDefault) { - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(GetParam().pref_name(), NULL)); + EXPECT_FALSE(store_->GetValue(GetParam().pref_name(), NULL)); } TEST_P(ConfigurationPolicyPrefStoreListTest, SetValue) { @@ -82,8 +81,7 @@ TEST_P(ConfigurationPolicyPrefStoreListTest, SetValue) { POLICY_SCOPE_USER, in_value); provider_.UpdateChromePolicy(policy); const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(GetParam().pref_name(), &value)); + EXPECT_TRUE(store_->GetValue(GetParam().pref_name(), &value)); ASSERT_TRUE(value); EXPECT_TRUE(in_value->Equals(value)); } @@ -115,8 +113,7 @@ class ConfigurationPolicyPrefStoreStringTest public testing::WithParamInterface<PolicyAndPref> {}; TEST_P(ConfigurationPolicyPrefStoreStringTest, GetDefault) { - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(GetParam().pref_name(), NULL)); + EXPECT_FALSE(store_->GetValue(GetParam().pref_name(), NULL)); } TEST_P(ConfigurationPolicyPrefStoreStringTest, SetValue) { @@ -126,8 +123,7 @@ TEST_P(ConfigurationPolicyPrefStoreStringTest, SetValue) { base::Value::CreateStringValue("http://chromium.org")); provider_.UpdateChromePolicy(policy); const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(GetParam().pref_name(), &value)); + EXPECT_TRUE(store_->GetValue(GetParam().pref_name(), &value)); ASSERT_TRUE(value); EXPECT_TRUE(base::StringValue("http://chromium.org").Equals(value)); } @@ -167,8 +163,7 @@ class ConfigurationPolicyPrefStoreBooleanTest public testing::WithParamInterface<PolicyAndPref> {}; TEST_P(ConfigurationPolicyPrefStoreBooleanTest, GetDefault) { - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(GetParam().pref_name(), NULL)); + EXPECT_FALSE(store_->GetValue(GetParam().pref_name(), NULL)); } TEST_P(ConfigurationPolicyPrefStoreBooleanTest, SetValue) { @@ -177,8 +172,7 @@ TEST_P(ConfigurationPolicyPrefStoreBooleanTest, SetValue) { POLICY_SCOPE_USER, base::Value::CreateBooleanValue(false)); provider_.UpdateChromePolicy(policy); const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(GetParam().pref_name(), &value)); + EXPECT_TRUE(store_->GetValue(GetParam().pref_name(), &value)); ASSERT_TRUE(value); bool boolean_value = true; bool result = value->GetAsBoolean(&boolean_value); @@ -189,8 +183,7 @@ TEST_P(ConfigurationPolicyPrefStoreBooleanTest, SetValue) { POLICY_SCOPE_USER, base::Value::CreateBooleanValue(true)); provider_.UpdateChromePolicy(policy); value = NULL; - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(GetParam().pref_name(), &value)); + EXPECT_TRUE(store_->GetValue(GetParam().pref_name(), &value)); boolean_value = false; result = value->GetAsBoolean(&boolean_value); ASSERT_TRUE(result); @@ -307,8 +300,7 @@ class ConfigurationPolicyPrefStoreIntegerTest public testing::WithParamInterface<PolicyAndPref> {}; TEST_P(ConfigurationPolicyPrefStoreIntegerTest, GetDefault) { - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(GetParam().pref_name(), NULL)); + EXPECT_FALSE(store_->GetValue(GetParam().pref_name(), NULL)); } TEST_P(ConfigurationPolicyPrefStoreIntegerTest, SetValue) { @@ -317,8 +309,7 @@ TEST_P(ConfigurationPolicyPrefStoreIntegerTest, SetValue) { POLICY_SCOPE_USER, base::Value::CreateIntegerValue(2)); provider_.UpdateChromePolicy(policy); const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(GetParam().pref_name(), &value)); + EXPECT_TRUE(store_->GetValue(GetParam().pref_name(), &value)); EXPECT_TRUE(base::FundamentalValue(2).Equals(value)); } @@ -360,8 +351,7 @@ class ConfigurationPolicyPrefStoreProxyTest const std::string& expected_proxy_bypass_list, const ProxyPrefs::ProxyMode& expected_proxy_mode) { const base::Value* value = NULL; - ASSERT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kProxy, &value)); + ASSERT_TRUE(store_->GetValue(prefs::kProxy, &value)); ASSERT_EQ(base::Value::TYPE_DICTIONARY, value->GetType()); ProxyConfigDictionary dict( static_cast<const base::DictionaryValue*>(value)); @@ -433,7 +423,7 @@ TEST_F(ConfigurationPolicyPrefStoreProxyTest, ManualOptionsInvalid) { provider_.UpdateChromePolicy(policy); const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_NO_VALUE, store_->GetValue(prefs::kProxy, &value)); + EXPECT_FALSE(store_->GetValue(prefs::kProxy, &value)); } @@ -492,7 +482,7 @@ TEST_F(ConfigurationPolicyPrefStoreProxyTest, PacScriptProxyModeInvalid) { ProxyPrefs::kPacScriptProxyModeName)); provider_.UpdateChromePolicy(policy); const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_NO_VALUE, store_->GetValue(prefs::kProxy, &value)); + EXPECT_FALSE(store_->GetValue(prefs::kProxy, &value)); } // Regression test for http://crbug.com/78016, CPanel returns empty strings @@ -555,8 +545,7 @@ TEST_F(ConfigurationPolicyPrefStoreProxyTest, ProxyInvalid) { POLICY_SCOPE_USER, base::Value::CreateIntegerValue(i)); provider_.UpdateChromePolicy(policy); const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kProxy, &value)); + EXPECT_FALSE(store_->GetValue(prefs::kProxy, &value)); } } @@ -630,36 +619,30 @@ TEST_F(ConfigurationPolicyPrefStoreDefaultSearchTest, MinimallyDefined) { provider_.UpdateChromePolicy(policy); const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderSearchURL, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderSearchURL, &value)); EXPECT_TRUE(base::StringValue(kSearchURL).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderName, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderName, &value)); EXPECT_TRUE(base::StringValue("test.com").Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderKeyword, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderKeyword, &value)); EXPECT_TRUE(base::StringValue("test.com").Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderSuggestURL, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderSuggestURL, + &value)); EXPECT_TRUE(base::StringValue(std::string()).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderIconURL, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderIconURL, &value)); EXPECT_TRUE(base::StringValue(std::string()).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderEncodings, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderEncodings, &value)); EXPECT_TRUE(base::StringValue(std::string()).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderInstantURL, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderInstantURL, + &value)); EXPECT_TRUE(base::StringValue(std::string()).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderAlternateURLs, + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderAlternateURLs, &value)); EXPECT_TRUE(base::ListValue().Equals(value)); } @@ -672,31 +655,26 @@ TEST_F(ConfigurationPolicyPrefStoreDefaultSearchTest, FullyDefined) { provider_.UpdateChromePolicy(policy); const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderSearchURL, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderSearchURL, &value)); EXPECT_TRUE(base::StringValue(kSearchURL).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderName, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderName, &value)); EXPECT_TRUE(base::StringValue(kName).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderKeyword, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderKeyword, &value)); EXPECT_TRUE(base::StringValue(kKeyword).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderSuggestURL, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderSuggestURL, + &value)); EXPECT_TRUE(base::StringValue(kSuggestURL).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderIconURL, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderIconURL, &value)); EXPECT_TRUE(base::StringValue(kIconURL).Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderEncodings, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderEncodings, &value)); EXPECT_TRUE(base::StringValue("UTF-16;UTF-8").Equals(value)); - EXPECT_EQ(PrefStore::READ_OK, store_->GetValue( + EXPECT_TRUE(store_->GetValue( prefs::kDefaultSearchProviderAlternateURLs, &value)); EXPECT_TRUE(default_alternate_urls_.Equals(value)); } @@ -709,20 +687,14 @@ TEST_F(ConfigurationPolicyPrefStoreDefaultSearchTest, MissingUrl) { policy.Erase(key::kDefaultSearchProviderSearchURL); provider_.UpdateChromePolicy(policy); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kDefaultSearchProviderSearchURL, NULL)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kDefaultSearchProviderName, NULL)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kDefaultSearchProviderKeyword, NULL)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kDefaultSearchProviderSuggestURL, NULL)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kDefaultSearchProviderIconURL, NULL)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kDefaultSearchProviderEncodings, NULL)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kDefaultSearchProviderAlternateURLs, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kDefaultSearchProviderSearchURL, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kDefaultSearchProviderName, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kDefaultSearchProviderKeyword, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kDefaultSearchProviderSuggestURL, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kDefaultSearchProviderIconURL, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kDefaultSearchProviderEncodings, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kDefaultSearchProviderAlternateURLs, + NULL)); } // Checks that if the default search policy is invalid, that no elements of the @@ -736,20 +708,14 @@ TEST_F(ConfigurationPolicyPrefStoreDefaultSearchTest, Invalid) { base::Value::CreateStringValue(bad_search_url)); provider_.UpdateChromePolicy(policy); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kDefaultSearchProviderSearchURL, NULL)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kDefaultSearchProviderName, NULL)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kDefaultSearchProviderKeyword, NULL)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kDefaultSearchProviderSuggestURL, NULL)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kDefaultSearchProviderIconURL, NULL)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kDefaultSearchProviderEncodings, NULL)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kDefaultSearchProviderAlternateURLs, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kDefaultSearchProviderSearchURL, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kDefaultSearchProviderName, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kDefaultSearchProviderKeyword, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kDefaultSearchProviderSuggestURL, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kDefaultSearchProviderIconURL, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kDefaultSearchProviderEncodings, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kDefaultSearchProviderAlternateURLs, + NULL)); } // Checks that if the default search policy is invalid, that no elements of the @@ -761,12 +727,10 @@ TEST_F(ConfigurationPolicyPrefStoreDefaultSearchTest, Disabled) { provider_.UpdateChromePolicy(policy); const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderEnabled, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderEnabled, &value)); base::FundamentalValue expected_enabled(false); EXPECT_TRUE(base::Value::Equals(&expected_enabled, value)); - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kDefaultSearchProviderSearchURL, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kDefaultSearchProviderSearchURL, &value)); base::StringValue expected_search_url(""); EXPECT_TRUE(base::Value::Equals(&expected_search_url, value)); } @@ -802,8 +766,7 @@ class ConfigurationPolicyPrefStoreIncognitoModeTest void VerifyValues(IncognitoModePrefs::Availability availability) { const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kIncognitoModeAvailability, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kIncognitoModeAvailability, &value)); EXPECT_TRUE(base::FundamentalValue(availability).Equals(value)); } }; @@ -833,8 +796,7 @@ TEST_F(ConfigurationPolicyPrefStoreIncognitoModeTest, NoObsoletePolicyAndNoIncognitoAvailability) { SetPolicies(INCOGNITO_ENABLED_UNKNOWN, kIncognitoModeAvailabilityNotSet); const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kIncognitoModeAvailability, &value)); + EXPECT_FALSE(store_->GetValue(prefs::kIncognitoModeAvailability, &value)); } // Checks that if the obsolete IncognitoEnabled policy is set, if sets @@ -869,8 +831,7 @@ class ConfigurationPolicyPrefStoreSyncTest : public ConfigurationPolicyPrefStoreTest {}; TEST_F(ConfigurationPolicyPrefStoreSyncTest, Default) { - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kSyncManaged, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kSyncManaged, NULL)); } TEST_F(ConfigurationPolicyPrefStoreSyncTest, Enabled) { @@ -879,8 +840,7 @@ TEST_F(ConfigurationPolicyPrefStoreSyncTest, Enabled) { base::Value::CreateBooleanValue(false)); provider_.UpdateChromePolicy(policy); // Enabling Sync should not set the pref. - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kSyncManaged, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kSyncManaged, NULL)); } TEST_F(ConfigurationPolicyPrefStoreSyncTest, Disabled) { @@ -890,7 +850,7 @@ TEST_F(ConfigurationPolicyPrefStoreSyncTest, Disabled) { provider_.UpdateChromePolicy(policy); // Sync should be flagged as managed. const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(prefs::kSyncManaged, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kSyncManaged, &value)); ASSERT_TRUE(value); bool sync_managed = false; bool result = value->GetAsBoolean(&sync_managed); @@ -904,22 +864,20 @@ class ConfigurationPolicyPrefStorePromptDownloadTest : public ConfigurationPolicyPrefStoreTest {}; TEST_F(ConfigurationPolicyPrefStorePromptDownloadTest, Default) { - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kPromptForDownload, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kPromptForDownload, NULL)); } #if !defined(OS_CHROMEOS) TEST_F(ConfigurationPolicyPrefStorePromptDownloadTest, SetDownloadDirectory) { PolicyMap policy; - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kPromptForDownload, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kPromptForDownload, NULL)); policy.Set(key::kDownloadDirectory, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, base::Value::CreateStringValue("")); provider_.UpdateChromePolicy(policy); // Setting a DownloadDirectory should disable the PromptForDownload pref. const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(prefs::kPromptForDownload, + EXPECT_TRUE(store_->GetValue(prefs::kPromptForDownload, &value)); ASSERT_TRUE(value); bool prompt_for_download = true; @@ -932,30 +890,27 @@ TEST_F(ConfigurationPolicyPrefStorePromptDownloadTest, SetDownloadDirectory) { TEST_F(ConfigurationPolicyPrefStorePromptDownloadTest, EnableFileSelectionDialogs) { PolicyMap policy; - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kPromptForDownload, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kPromptForDownload, NULL)); policy.Set(key::kAllowFileSelectionDialogs, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, base::Value::CreateBooleanValue(true)); provider_.UpdateChromePolicy(policy); // Allowing file-selection dialogs should not influence the PromptForDownload // pref. - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kPromptForDownload, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kPromptForDownload, NULL)); } TEST_F(ConfigurationPolicyPrefStorePromptDownloadTest, DisableFileSelectionDialogs) { PolicyMap policy; - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kPromptForDownload, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kPromptForDownload, NULL)); policy.Set(key::kAllowFileSelectionDialogs, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, base::Value::CreateBooleanValue(false)); provider_.UpdateChromePolicy(policy); // Disabling file-selection dialogs should disable the PromptForDownload pref. const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(prefs::kPromptForDownload, + EXPECT_TRUE(store_->GetValue(prefs::kPromptForDownload, &value)); ASSERT_TRUE(value); bool prompt_for_download = true; @@ -969,8 +924,7 @@ class ConfigurationPolicyPrefStoreAutofillTest : public ConfigurationPolicyPrefStoreTest {}; TEST_F(ConfigurationPolicyPrefStoreAutofillTest, Default) { - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kAutofillEnabled, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kAutofillEnabled, NULL)); } TEST_F(ConfigurationPolicyPrefStoreAutofillTest, Enabled) { @@ -979,8 +933,7 @@ TEST_F(ConfigurationPolicyPrefStoreAutofillTest, Enabled) { base::Value::CreateBooleanValue(true)); provider_.UpdateChromePolicy(policy); // Enabling Autofill should not set the pref. - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kAutofillEnabled, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kAutofillEnabled, NULL)); } TEST_F(ConfigurationPolicyPrefStoreAutofillTest, Disabled) { @@ -990,8 +943,7 @@ TEST_F(ConfigurationPolicyPrefStoreAutofillTest, Disabled) { provider_.UpdateChromePolicy(policy); // Disabling Autofill should switch the pref to managed. const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kAutofillEnabled, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kAutofillEnabled, &value)); ASSERT_TRUE(value); bool autofill_enabled = true; bool result = value->GetAsBoolean(&autofill_enabled); @@ -1018,8 +970,7 @@ class ConfigurationPolicyPrefStoreRefreshTest TEST_F(ConfigurationPolicyPrefStoreRefreshTest, Refresh) { const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kHomePage, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kHomePage, NULL)); EXPECT_CALL(observer_, OnPrefValueChanged(prefs::kHomePage)).Times(1); PolicyMap policy; @@ -1027,8 +978,7 @@ TEST_F(ConfigurationPolicyPrefStoreRefreshTest, Refresh) { base::Value::CreateStringValue("http://www.chromium.org")); provider_.UpdateChromePolicy(policy); Mock::VerifyAndClearExpectations(&observer_); - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kHomePage, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kHomePage, &value)); EXPECT_TRUE(base::StringValue("http://www.chromium.org").Equals(value)); EXPECT_CALL(observer_, OnPrefValueChanged(_)).Times(0); @@ -1039,8 +989,7 @@ TEST_F(ConfigurationPolicyPrefStoreRefreshTest, Refresh) { policy.Erase(key::kHomepageLocation); provider_.UpdateChromePolicy(policy); Mock::VerifyAndClearExpectations(&observer_); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kHomePage, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kHomePage, NULL)); } TEST_F(ConfigurationPolicyPrefStoreRefreshTest, Initialization) { @@ -1060,41 +1009,38 @@ class ConfigurationPolicyPrefStoreOthersTest TEST_F(ConfigurationPolicyPrefStoreOthersTest, JavascriptEnabled) { // This is a boolean policy, but affects an integer preference. - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kManagedDefaultJavaScriptSetting, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kManagedDefaultJavaScriptSetting, NULL)); PolicyMap policy; policy.Set(key::kJavascriptEnabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, base::Value::CreateBooleanValue(true)); provider_.UpdateChromePolicy(policy); - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kManagedDefaultJavaScriptSetting, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kManagedDefaultJavaScriptSetting, NULL)); policy.Set(key::kJavascriptEnabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, base::Value::CreateBooleanValue(false)); provider_.UpdateChromePolicy(policy); const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kManagedDefaultJavaScriptSetting, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kManagedDefaultJavaScriptSetting, + &value)); EXPECT_TRUE(base::FundamentalValue(CONTENT_SETTING_BLOCK).Equals(value)); } TEST_F(ConfigurationPolicyPrefStoreOthersTest, JavascriptEnabledOverridden) { - EXPECT_EQ(PrefStore::READ_NO_VALUE, - store_->GetValue(prefs::kManagedDefaultJavaScriptSetting, NULL)); + EXPECT_FALSE(store_->GetValue(prefs::kManagedDefaultJavaScriptSetting, NULL)); PolicyMap policy; policy.Set(key::kJavascriptEnabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, base::Value::CreateBooleanValue(false)); provider_.UpdateChromePolicy(policy); const base::Value* value = NULL; - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kManagedDefaultJavaScriptSetting, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kManagedDefaultJavaScriptSetting, + &value)); EXPECT_TRUE(base::FundamentalValue(CONTENT_SETTING_BLOCK).Equals(value)); // DefaultJavaScriptSetting overrides JavascriptEnabled. policy.Set(key::kDefaultJavaScriptSetting, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, base::Value::CreateIntegerValue(CONTENT_SETTING_ALLOW)); provider_.UpdateChromePolicy(policy); - EXPECT_EQ(PrefStore::READ_OK, - store_->GetValue(prefs::kManagedDefaultJavaScriptSetting, &value)); + EXPECT_TRUE(store_->GetValue(prefs::kManagedDefaultJavaScriptSetting, + &value)); EXPECT_TRUE(base::FundamentalValue(CONTENT_SETTING_ALLOW).Equals(value)); } diff --git a/chrome/browser/policy/managed_mode_policy_provider.cc b/chrome/browser/policy/managed_mode_policy_provider.cc index cbd64c4..d8bc07c 100644 --- a/chrome/browser/policy/managed_mode_policy_provider.cc +++ b/chrome/browser/policy/managed_mode_policy_provider.cc @@ -77,20 +77,12 @@ void ManagedModePolicyProvider::OnInitializationCompleted(bool success) { base::DictionaryValue* ManagedModePolicyProvider::GetCachedPolicy() const { base::Value* value = NULL; base::DictionaryValue* dict = NULL; - PrefStore::ReadResult result = store_->GetMutableValue(kPolicies, &value); - switch (result) { - case PrefStore::READ_NO_VALUE: { - dict = new base::DictionaryValue; - store_->SetValue(kPolicies, dict); - break; - } - case PrefStore::READ_OK: { - bool success = value->GetAsDictionary(&dict); - DCHECK(success); - break; - } - default: - NOTREACHED(); + if (store_->GetMutableValue(kPolicies, &value)) { + bool success = value->GetAsDictionary(&dict); + DCHECK(success); + } else { + dict = new base::DictionaryValue; + store_->SetValue(kPolicies, dict); } return dict; diff --git a/chrome/browser/policy/managed_mode_policy_provider_unittest.cc b/chrome/browser/policy/managed_mode_policy_provider_unittest.cc index 7aa89cd..7e1bd8e 100644 --- a/chrome/browser/policy/managed_mode_policy_provider_unittest.cc +++ b/chrome/browser/policy/managed_mode_policy_provider_unittest.cc @@ -99,22 +99,15 @@ void TestHarness::InstallPolicy(const std::string& policy_name, base::Value* policy_value) { base::DictionaryValue* cached_policy = NULL; base::Value* value = NULL; - PrefStore::ReadResult result = - pref_store_->GetMutableValue(ManagedModePolicyProvider::kPolicies, - &value); - switch (result) { - case PrefStore::READ_NO_VALUE: - cached_policy = new base::DictionaryValue; - pref_store_->SetValue(ManagedModePolicyProvider::kPolicies, - cached_policy); - break; - case PrefStore::READ_OK: - ASSERT_TRUE(value->GetAsDictionary(&cached_policy)); - break; - default: - FAIL() << "Invalid result reading policy: " << result; - return; + if (pref_store_->GetMutableValue(ManagedModePolicyProvider::kPolicies, + &value)) { + ASSERT_TRUE(value->GetAsDictionary(&cached_policy)); + } else { + cached_policy = new base::DictionaryValue; + pref_store_->SetValue(ManagedModePolicyProvider::kPolicies, + cached_policy); } + cached_policy->SetWithoutPathExpansion(policy_name, policy_value); } diff --git a/chrome/browser/prefs/command_line_pref_store_unittest.cc b/chrome/browser/prefs/command_line_pref_store_unittest.cc index bc1edb9..5d2c939 100644 --- a/chrome/browser/prefs/command_line_pref_store_unittest.cc +++ b/chrome/browser/prefs/command_line_pref_store_unittest.cc @@ -32,7 +32,7 @@ class TestCommandLinePrefStore : public CommandLinePrefStore { void VerifyProxyMode(ProxyPrefs::ProxyMode expected_mode) { const Value* value = NULL; - ASSERT_EQ(PrefStore::READ_OK, GetValue(prefs::kProxy, &value)); + ASSERT_TRUE(GetValue(prefs::kProxy, &value)); ASSERT_EQ(Value::TYPE_DICTIONARY, value->GetType()); ProxyConfigDictionary dict(static_cast<const DictionaryValue*>(value)); ProxyPrefs::ProxyMode actual_mode; @@ -43,8 +43,7 @@ class TestCommandLinePrefStore : public CommandLinePrefStore { void VerifySSLCipherSuites(const char* const* ciphers, size_t cipher_count) { const Value* value = NULL; - ASSERT_EQ(PrefStore::READ_OK, - GetValue(prefs::kCipherSuiteBlacklist, &value)); + ASSERT_TRUE(GetValue(prefs::kCipherSuiteBlacklist, &value)); ASSERT_EQ(Value::TYPE_LIST, value->GetType()); const ListValue* list_value = static_cast<const ListValue*>(value); ASSERT_EQ(cipher_count, list_value->GetSize()); @@ -68,8 +67,7 @@ TEST(CommandLinePrefStoreTest, SimpleStringPref) { scoped_refptr<CommandLinePrefStore> store = new CommandLinePrefStore(&cl); const Value* actual = NULL; - EXPECT_EQ(PrefStore::READ_OK, - store->GetValue(prefs::kApplicationLocale, &actual)); + EXPECT_TRUE(store->GetValue(prefs::kApplicationLocale, &actual)); std::string result; EXPECT_TRUE(actual->GetAsString(&result)); EXPECT_EQ("hi-MOM", result); @@ -93,8 +91,8 @@ TEST(CommandLinePrefStoreTest, NoPrefs) { scoped_refptr<CommandLinePrefStore> store = new CommandLinePrefStore(&cl); const Value* actual = NULL; - EXPECT_EQ(PrefStore::READ_NO_VALUE, store->GetValue(unknown_bool, &actual)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, store->GetValue(unknown_string, &actual)); + EXPECT_FALSE(store->GetValue(unknown_bool, &actual)); + EXPECT_FALSE(store->GetValue(unknown_string, &actual)); } // Tests a complex command line with multiple known and unknown switches. @@ -108,13 +106,13 @@ TEST(CommandLinePrefStoreTest, MultipleSwitches) { new TestCommandLinePrefStore(&cl); const Value* actual = NULL; - EXPECT_EQ(PrefStore::READ_NO_VALUE, store->GetValue(unknown_bool, &actual)); - EXPECT_EQ(PrefStore::READ_NO_VALUE, store->GetValue(unknown_string, &actual)); + EXPECT_FALSE(store->GetValue(unknown_bool, &actual)); + EXPECT_FALSE(store->GetValue(unknown_string, &actual)); store->VerifyProxyMode(ProxyPrefs::MODE_FIXED_SERVERS); const Value* value = NULL; - ASSERT_EQ(PrefStore::READ_OK, store->GetValue(prefs::kProxy, &value)); + ASSERT_TRUE(store->GetValue(prefs::kProxy, &value)); ASSERT_EQ(Value::TYPE_DICTIONARY, value->GetType()); ProxyConfigDictionary dict(static_cast<const DictionaryValue*>(value)); diff --git a/chrome/browser/prefs/pref_service.cc b/chrome/browser/prefs/pref_service.cc index 1eb80b4..06d5b79 100644 --- a/chrome/browser/prefs/pref_service.cc +++ b/chrome/browser/prefs/pref_service.cc @@ -731,7 +731,7 @@ const base::Value* PrefService::GetUserPrefValue(const char* path) const { // Look for an existing preference in the user store. If it doesn't // exist, return NULL. base::Value* value = NULL; - if (user_pref_store_->GetMutableValue(path, &value) != PrefStore::READ_OK) + if (!user_pref_store_->GetMutableValue(path, &value)) return NULL; if (!value->IsType(pref->GetType())) { @@ -746,7 +746,7 @@ const base::Value* PrefService::GetDefaultPrefValue(const char* path) const { DCHECK(CalledOnValidThread()); // Lookup the preference in the default store. const base::Value* value = NULL; - if (default_store_->GetValue(path, &value) != PrefStore::READ_OK) { + if (!default_store_->GetValue(path, &value)) { NOTREACHED() << "Default value missing for pref: " << path; return NULL; } @@ -789,7 +789,7 @@ void PrefService::RegisterPreference(const char* path, // The main code path takes ownership, but most don't. We'll be safe. scoped_ptr<Value> scoped_value(default_value); - DCHECK(!FindPreference(path)) << "Tried to register duplicate pref " << path; + CHECK(!FindPreference(path)) << "Tried to register duplicate pref " << path; base::Value::Type orig_type = default_value->GetType(); DCHECK(orig_type != Value::TYPE_NULL && orig_type != Value::TYPE_BINARY) << @@ -824,10 +824,8 @@ void PrefService::UnregisterPreference(const char* path) { DCHECK(CalledOnValidThread()); PreferenceMap::iterator it = prefs_map_.find(path); - if (it == prefs_map_.end()) { - NOTREACHED() << "Trying to unregister an unregistered pref: " << path; - return; - } + CHECK(it != prefs_map_.end()) << "Trying to unregister an unregistered pref: " + << path; prefs_map_.erase(it); default_store_->RemoveDefaultValue(path); @@ -932,7 +930,7 @@ Value* PrefService::GetMutableUserPref(const char* path, // Look for an existing preference in the user store. If it doesn't // exist or isn't the correct type, create a new user preference. Value* value = NULL; - if (user_pref_store_->GetMutableValue(path, &value) != PrefStore::READ_OK || + if (!user_pref_store_->GetMutableValue(path, &value) || !value->IsType(type)) { if (type == Value::TYPE_DICTIONARY) { value = new DictionaryValue; diff --git a/chrome/browser/prefs/pref_value_store.cc b/chrome/browser/prefs/pref_value_store.cc index b023d8b..7797077 100644 --- a/chrome/browser/prefs/pref_value_store.cc +++ b/chrome/browser/prefs/pref_value_store.cc @@ -214,23 +214,11 @@ bool PrefValueStore::GetValueFromStore(const char* name, // Only return true if we find a value and it is the correct type, so stale // values with the incorrect type will be ignored. const PrefStore* store = GetPrefStore(static_cast<PrefStoreType>(store_type)); - if (store) { - switch (store->GetValue(name, out_value)) { - case PrefStore::READ_USE_DEFAULT: - store = GetPrefStore(DEFAULT_STORE); - if (!store || store->GetValue(name, out_value) != PrefStore::READ_OK) { - *out_value = NULL; - return false; - } - // Fall through... - case PrefStore::READ_OK: - return true; - case PrefStore::READ_NO_VALUE: - break; - } - } + if (store && store->GetValue(name, out_value)) + return true; - // No valid value found for the given preference name: set the return false. + // No valid value found for the given preference name: set the return value + // to false. *out_value = NULL; return false; } diff --git a/chrome/browser/sync/credential_cache_service_win.cc b/chrome/browser/sync/credential_cache_service_win.cc index 8d05e0d..7b1c621 100644 --- a/chrome/browser/sync/credential_cache_service_win.cc +++ b/chrome/browser/sync/credential_cache_service_win.cc @@ -361,7 +361,7 @@ void CredentialCacheService::ScheduleNextReadFromAlternateCredentialCache( bool CredentialCacheService::HasPref(scoped_refptr<JsonPrefStore> store, const std::string& pref_name) { - return (store->GetValue(pref_name, NULL) == PrefStore::READ_OK); + return store->GetValue(pref_name, NULL); } // static diff --git a/chrome/service/service_process_prefs.cc b/chrome/service/service_process_prefs.cc index e417988..71b74fe 100644 --- a/chrome/service/service_process_prefs.cc +++ b/chrome/service/service_process_prefs.cc @@ -28,10 +28,9 @@ std::string ServiceProcessPrefs::GetString( const std::string& default_value) const { const Value* value; std::string result; - if (prefs_->GetValue(key, &value) != PersistentPrefStore::READ_OK || - !value->GetAsString(&result)) { + if (!prefs_->GetValue(key, &value) || !value->GetAsString(&result)) return default_value; - } + return result; } @@ -44,10 +43,9 @@ bool ServiceProcessPrefs::GetBoolean(const std::string& key, bool default_value) const { const Value* value; bool result = false; - if (prefs_->GetValue(key, &value) != PersistentPrefStore::READ_OK || - !value->GetAsBoolean(&result)) { + if (!prefs_->GetValue(key, &value) || !value->GetAsBoolean(&result)) return default_value; - } + return result; } @@ -59,10 +57,9 @@ int ServiceProcessPrefs::GetInt(const std::string& key, int default_value) const { const Value* value; int result = default_value; - if (prefs_->GetValue(key, &value) != PersistentPrefStore::READ_OK || - !value->GetAsInteger(&result)) { + if (!prefs_->GetValue(key, &value) || !value->GetAsInteger(&result)) return default_value; - } + return result; } @@ -73,7 +70,7 @@ void ServiceProcessPrefs::SetInt(const std::string& key, int value) { const DictionaryValue* ServiceProcessPrefs::GetDictionary( const std::string& key) const { const Value* value; - if (prefs_->GetValue(key, &value) != PersistentPrefStore::READ_OK || + if (!prefs_->GetValue(key, &value) || !value->IsType(Value::TYPE_DICTIONARY)) { return NULL; } @@ -84,10 +81,8 @@ const DictionaryValue* ServiceProcessPrefs::GetDictionary( const base::ListValue* ServiceProcessPrefs::GetList( const std::string& key) const { const Value* value; - if (prefs_->GetValue(key, &value) != PersistentPrefStore::READ_OK || - !value->IsType(Value::TYPE_LIST)) { - return NULL; - } + if (!prefs_->GetValue(key, &value) || !value->IsType(Value::TYPE_LIST)) + return NULL; return static_cast<const ListValue*>(value); } diff --git a/chrome/test/base/testing_pref_service.cc b/chrome/test/base/testing_pref_service.cc index b066439..4ef4ec8 100644 --- a/chrome/test/base/testing_pref_service.cc +++ b/chrome/test/base/testing_pref_service.cc @@ -85,7 +85,7 @@ void TestingPrefServiceBase::RemoveRecommendedPref(const char* path) { const Value* TestingPrefServiceBase::GetPref(TestingPrefStore* pref_store, const char* path) const { const Value* res; - return pref_store->GetValue(path, &res) == PrefStore::READ_OK ? res : NULL; + return pref_store->GetValue(path, &res) ? res : NULL; } void TestingPrefServiceBase::SetPref(TestingPrefStore* pref_store, |