diff options
31 files changed, 170 insertions, 90 deletions
diff --git a/chrome/browser/extensions/extension_pref_value_map.cc b/chrome/browser/extensions/extension_pref_value_map.cc index 7e68da1..02ebcdf 100644 --- a/chrome/browser/extensions/extension_pref_value_map.cc +++ b/chrome/browser/extensions/extension_pref_value_map.cc @@ -151,7 +151,7 @@ const Value* ExtensionPrefValueMap::GetEffectivePrefValue( if (winner == entries_.end()) return NULL; - Value* value = NULL; + const Value* value = NULL; const std::string& ext_id = winner->first; if (incognito) GetExtensionPrefValueMap(ext_id, true)->GetValue(key, &value); @@ -179,7 +179,7 @@ ExtensionPrefValueMap::GetEffectivePrefValueController( if (install_time < winners_install_time) continue; - Value* value = NULL; + const Value* value = NULL; const PrefValueMap* prefs = GetExtensionPrefValueMap(ext_id, false); if (prefs->GetValue(key, &value)) { winner = i; diff --git a/chrome/browser/policy/configuration_policy_pref_store.cc b/chrome/browser/policy/configuration_policy_pref_store.cc index 6933116..cae0144 100644 --- a/chrome/browser/policy/configuration_policy_pref_store.cc +++ b/chrome/browser/policy/configuration_policy_pref_store.cc @@ -42,7 +42,8 @@ class ConfigurationPolicyPrefKeeper virtual ~ConfigurationPolicyPrefKeeper(); // Get a preference value. - PrefStore::ReadResult GetValue(const std::string& key, Value** result) const; + PrefStore::ReadResult GetValue(const std::string& key, + const Value** result) const; // Compute the set of preference names that are different in |keeper|. This // includes preferences that are missing in either one. @@ -311,8 +312,8 @@ ConfigurationPolicyPrefKeeper::~ConfigurationPolicyPrefKeeper() { PrefStore::ReadResult ConfigurationPolicyPrefKeeper::GetValue(const std::string& key, - Value** result) const { - Value* stored_value = NULL; + const Value** result) const { + const Value* stored_value = NULL; if (!prefs_.GetValue(key, &stored_value)) return PrefStore::READ_NO_VALUE; @@ -777,7 +778,7 @@ bool ConfigurationPolicyPrefStore::IsInitializationComplete() const { PrefStore::ReadResult ConfigurationPolicyPrefStore::GetValue(const std::string& key, - Value** value) const { + const Value** value) const { if (policy_keeper_.get()) return policy_keeper_->GetValue(key, value); diff --git a/chrome/browser/policy/configuration_policy_pref_store.h b/chrome/browser/policy/configuration_policy_pref_store.h index bd1a990..a66b72c 100644 --- a/chrome/browser/policy/configuration_policy_pref_store.h +++ b/chrome/browser/policy/configuration_policy_pref_store.h @@ -38,7 +38,8 @@ class ConfigurationPolicyPrefStore virtual void AddObserver(PrefStore::Observer* observer); virtual void RemoveObserver(PrefStore::Observer* observer); virtual bool IsInitializationComplete() const; - virtual ReadResult GetValue(const std::string& key, Value** result) const; + virtual ReadResult GetValue(const std::string& key, + const Value** result) const; // ConfigurationPolicyProvider::Observer methods: virtual void OnUpdatePolicy(); diff --git a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc index edd2e43..a8c46a3 100644 --- a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc +++ b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc @@ -61,7 +61,7 @@ TEST_P(ConfigurationPolicyPrefStoreListTest, SetValue) { in_value->Append(Value::CreateStringValue("test2,")); provider_.AddPolicy(GetParam().type(), in_value); store_->OnUpdatePolicy(); - Value* value; + const Value* value; EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(GetParam().pref_name(), &value)); EXPECT_TRUE(in_value->Equals(value)); @@ -99,7 +99,7 @@ TEST_P(ConfigurationPolicyPrefStoreStringTest, SetValue) { provider_.AddPolicy(GetParam().type(), Value::CreateStringValue("http://chromium.org")); store_->OnUpdatePolicy(); - Value* value; + const Value* value; EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(GetParam().pref_name(), &value)); EXPECT_TRUE(StringValue("http://chromium.org").Equals(value)); @@ -140,7 +140,7 @@ TEST_P(ConfigurationPolicyPrefStoreBooleanTest, GetDefault) { TEST_P(ConfigurationPolicyPrefStoreBooleanTest, SetValue) { provider_.AddPolicy(GetParam().type(), Value::CreateBooleanValue(false)); store_->OnUpdatePolicy(); - Value* value; + const Value* value; bool result = true; EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(GetParam().pref_name(), &value)); @@ -230,7 +230,7 @@ TEST_P(ConfigurationPolicyPrefStoreIntegerTest, GetDefault) { TEST_P(ConfigurationPolicyPrefStoreIntegerTest, SetValue) { provider_.AddPolicy(GetParam().type(), Value::CreateIntegerValue(2)); store_->OnUpdatePolicy(); - Value* value = NULL; + const Value* value = NULL; EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(GetParam().pref_name(), &value)); EXPECT_TRUE(FundamentalValue(2).Equals(value)); @@ -255,11 +255,11 @@ class ConfigurationPolicyPrefStoreProxyTest : public testing::Test { const std::string& expected_proxy_pac_url, const std::string& expected_proxy_bypass_list, const ProxyPrefs::ProxyMode& expected_proxy_mode) { - Value* value = NULL; + const Value* value = NULL; ASSERT_EQ(PrefStore::READ_OK, store.GetValue(prefs::kProxy, &value)); ASSERT_EQ(Value::TYPE_DICTIONARY, value->GetType()); - ProxyConfigDictionary dict(static_cast<DictionaryValue*>(value)); + ProxyConfigDictionary dict(static_cast<const DictionaryValue*>(value)); std::string s; if (expected_proxy_server.empty()) { EXPECT_FALSE(dict.GetProxyServer(&s)); @@ -426,7 +426,7 @@ TEST_F(ConfigurationPolicyPrefStoreProxyTest, ProxyInvalid) { scoped_refptr<ConfigurationPolicyPrefStore> store( new ConfigurationPolicyPrefStore(&provider)); - Value* value = NULL; + const Value* value = NULL; EXPECT_EQ(PrefStore::READ_NO_VALUE, store->GetValue(prefs::kProxy, &value)); } @@ -448,7 +448,7 @@ TEST_F(ConfigurationPolicyPrefStoreDefaultSearchTest, MinimallyDefined) { scoped_refptr<ConfigurationPolicyPrefStore> store( new ConfigurationPolicyPrefStore(&provider)); - Value* value = NULL; + const Value* value = NULL; EXPECT_EQ(PrefStore::READ_OK, store->GetValue(prefs::kDefaultSearchProviderSearchURL, &value)); EXPECT_TRUE(StringValue(search_url).Equals(value)); @@ -506,7 +506,7 @@ TEST_F(ConfigurationPolicyPrefStoreDefaultSearchTest, FullyDefined) { scoped_refptr<ConfigurationPolicyPrefStore> store( new ConfigurationPolicyPrefStore(&provider)); - Value* value = NULL; + const Value* value = NULL; EXPECT_EQ(PrefStore::READ_OK, store->GetValue(prefs::kDefaultSearchProviderSearchURL, &value)); EXPECT_TRUE(StringValue(search_url).Equals(value)); @@ -635,7 +635,7 @@ TEST_F(ConfigurationPolicyPrefStoreSyncTest, Disabled) { provider_.AddPolicy(kPolicySyncDisabled, Value::CreateBooleanValue(true)); store_->OnUpdatePolicy(); // Sync should be flagged as managed. - Value* value = NULL; + const Value* value = NULL; EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(prefs::kSyncManaged, &value)); ASSERT_TRUE(value != NULL); EXPECT_TRUE(FundamentalValue(true).Equals(value)); @@ -663,7 +663,7 @@ TEST_F(ConfigurationPolicyPrefStoreAutofillTest, Disabled) { provider_.AddPolicy(kPolicyAutoFillEnabled, Value::CreateBooleanValue(false)); store_->OnUpdatePolicy(); // Disabling Autofill should switch the pref to managed. - Value* value = NULL; + const Value* value = NULL; EXPECT_EQ(PrefStore::READ_OK, store_->GetValue(prefs::kAutofillEnabled, &value)); EXPECT_TRUE(FundamentalValue(false).Equals(value)); @@ -685,7 +685,7 @@ class ConfigurationPolicyPrefStoreRefreshTest }; TEST_F(ConfigurationPolicyPrefStoreRefreshTest, Refresh) { - Value* value = NULL; + const Value* value = NULL; EXPECT_EQ(PrefStore::READ_NO_VALUE, store_->GetValue(prefs::kHomePage, NULL)); diff --git a/chrome/browser/prefs/command_line_pref_store_unittest.cc b/chrome/browser/prefs/command_line_pref_store_unittest.cc index 28c35d19..866b9c8 100644 --- a/chrome/browser/prefs/command_line_pref_store_unittest.cc +++ b/chrome/browser/prefs/command_line_pref_store_unittest.cc @@ -26,10 +26,10 @@ class TestCommandLinePrefStore : public CommandLinePrefStore { } void VerifyProxyMode(ProxyPrefs::ProxyMode expected_mode) { - Value* value = NULL; + const Value* value = NULL; ASSERT_EQ(PrefStore::READ_OK, GetValue(prefs::kProxy, &value)); ASSERT_EQ(Value::TYPE_DICTIONARY, value->GetType()); - ProxyConfigDictionary dict(static_cast<DictionaryValue*>(value)); + ProxyConfigDictionary dict(static_cast<const DictionaryValue*>(value)); ProxyPrefs::ProxyMode actual_mode; ASSERT_TRUE(dict.GetMode(&actual_mode)); EXPECT_EQ(expected_mode, actual_mode); @@ -47,7 +47,7 @@ TEST(CommandLinePrefStoreTest, SimpleStringPref) { cl.AppendSwitchASCII(switches::kLang, "hi-MOM"); scoped_refptr<CommandLinePrefStore> store = new CommandLinePrefStore(&cl); - Value* actual = NULL; + const Value* actual = NULL; EXPECT_EQ(PrefStore::READ_OK, store->GetValue(prefs::kApplicationLocale, &actual)); std::string result; @@ -72,7 +72,7 @@ TEST(CommandLinePrefStoreTest, NoPrefs) { cl.AppendSwitchASCII(unknown_bool, "a value"); scoped_refptr<CommandLinePrefStore> store = new CommandLinePrefStore(&cl); - Value* actual = NULL; + 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)); } @@ -87,16 +87,16 @@ TEST(CommandLinePrefStoreTest, MultipleSwitches) { scoped_refptr<TestCommandLinePrefStore> store = new TestCommandLinePrefStore(&cl); - Value* actual = NULL; + 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)); store->VerifyProxyMode(ProxyPrefs::MODE_FIXED_SERVERS); - Value* value = NULL; + const Value* value = NULL; ASSERT_EQ(PrefStore::READ_OK, store->GetValue(prefs::kProxy, &value)); ASSERT_EQ(Value::TYPE_DICTIONARY, value->GetType()); - ProxyConfigDictionary dict(static_cast<DictionaryValue*>(value)); + ProxyConfigDictionary dict(static_cast<const DictionaryValue*>(value)); std::string string_result = ""; diff --git a/chrome/browser/prefs/default_pref_store.cc b/chrome/browser/prefs/default_pref_store.cc index cacc833..8e43438 100644 --- a/chrome/browser/prefs/default_pref_store.cc +++ b/chrome/browser/prefs/default_pref_store.cc @@ -14,7 +14,7 @@ void DefaultPrefStore::SetDefaultValue(const std::string& key, Value* value) { } Value::ValueType DefaultPrefStore::GetType(const std::string& key) const { - Value* value; + const Value* value; return GetValue(key, &value) == READ_OK ? value->GetType() : Value::TYPE_NULL; } diff --git a/chrome/browser/prefs/overlay_persistent_pref_store.cc b/chrome/browser/prefs/overlay_persistent_pref_store.cc index 8f940c1..e75fbc6 100644 --- a/chrome/browser/prefs/overlay_persistent_pref_store.cc +++ b/chrome/browser/prefs/overlay_persistent_pref_store.cc @@ -4,6 +4,8 @@ #include "chrome/browser/prefs/overlay_persistent_pref_store.h" +#include "base/values.h" + OverlayPersistentPrefStore::OverlayPersistentPrefStore( PersistentPrefStore* underlay) : underlay_(underlay) { @@ -30,12 +32,32 @@ bool OverlayPersistentPrefStore::IsInitializationComplete() const { } PrefStore::ReadResult OverlayPersistentPrefStore::GetValue( - const std::string& key, Value** result) const { + const std::string& key, + const Value** result) const { if (overlay_.GetValue(key, result)) return READ_OK; return underlay_->GetValue(key, result); } +PrefStore::ReadResult OverlayPersistentPrefStore::GetMutableValue( + const std::string& key, + Value** result) { + if (overlay_.GetValue(key, result)) + return READ_OK; + + // Try to create copy of underlay if the overlay does not contain a value. + Value* underlay_value = NULL; + PrefStore::ReadResult read_result = + underlay_->GetMutableValue(key, &underlay_value); + if (read_result == READ_OK) { + *result = underlay_value->DeepCopy(); + overlay_.SetValue(key, *result); + return READ_OK; + } + // Return read failure if underlay stores no value for |key|. + return read_result; +} + void OverlayPersistentPrefStore::SetValue(const std::string& key, Value* value) { if (overlay_.SetValue(key, value)) diff --git a/chrome/browser/prefs/overlay_persistent_pref_store.h b/chrome/browser/prefs/overlay_persistent_pref_store.h index cb2b53f..92d207c 100644 --- a/chrome/browser/prefs/overlay_persistent_pref_store.h +++ b/chrome/browser/prefs/overlay_persistent_pref_store.h @@ -33,9 +33,11 @@ class OverlayPersistentPrefStore : public PersistentPrefStore, virtual void AddObserver(PrefStore::Observer* observer); virtual void RemoveObserver(PrefStore::Observer* observer); virtual bool IsInitializationComplete() const; - virtual ReadResult GetValue(const std::string& key, Value** result) const; + virtual ReadResult GetValue(const std::string& key, + const Value** result) const; // Methods of PersistentPrefStore. + virtual ReadResult GetMutableValue(const std::string& key, Value** result); virtual void SetValue(const std::string& key, Value* value); virtual void SetValueSilently(const std::string& key, Value* value); virtual void RemoveValue(const std::string& key); @@ -44,7 +46,6 @@ class OverlayPersistentPrefStore : public PersistentPrefStore, virtual bool WritePrefs(); virtual void ScheduleWritePrefs(); virtual void CommitPendingWrite(); - // TODO(battre) remove this function virtual void ReportValueChanged(const std::string& key); private: diff --git a/chrome/browser/prefs/overlay_persistent_pref_store_unittest.cc b/chrome/browser/prefs/overlay_persistent_pref_store_unittest.cc index 7e2e0a2..11dcc05 100644 --- a/chrome/browser/prefs/overlay_persistent_pref_store_unittest.cc +++ b/chrome/browser/prefs/overlay_persistent_pref_store_unittest.cc @@ -74,7 +74,7 @@ TEST_F(OverlayPersistentPrefStoreTest, Observer) { } TEST_F(OverlayPersistentPrefStoreTest, GetAndSet) { - Value* value = NULL; + const Value* value = NULL; int i = -1; EXPECT_EQ(PrefStore::READ_NO_VALUE, overlay_->GetValue(key, &value)); EXPECT_EQ(PrefStore::READ_NO_VALUE, underlay_->GetValue(key, &value)); @@ -117,3 +117,27 @@ TEST_F(OverlayPersistentPrefStoreTest, GetAndSet) { EXPECT_TRUE(value->GetAsInteger(&i)); EXPECT_EQ(42, i); } + +// Check that GetMutableValue does not return the dictionary of the underlay. +TEST_F(OverlayPersistentPrefStoreTest, ModifyDictionaries) { + underlay_->SetValue(key, new DictionaryValue); + + Value* modify = NULL; + EXPECT_EQ(PrefStore::READ_OK, overlay_->GetMutableValue(key, &modify)); + ASSERT_TRUE(modify); + ASSERT_TRUE(modify->GetType() == Value::TYPE_DICTIONARY); + static_cast<DictionaryValue*>(modify)->SetInteger(key, 42); + + Value* original_in_underlay = NULL; + EXPECT_EQ(PrefStore::READ_OK, + underlay_->GetMutableValue(key, &original_in_underlay)); + ASSERT_TRUE(original_in_underlay); + ASSERT_TRUE(original_in_underlay->GetType() == Value::TYPE_DICTIONARY); + EXPECT_TRUE(static_cast<DictionaryValue*>(original_in_underlay)->empty()); + + Value* modified = NULL; + EXPECT_EQ(PrefStore::READ_OK, overlay_->GetMutableValue(key, &modified)); + ASSERT_TRUE(modified); + ASSERT_TRUE(modified->GetType() == Value::TYPE_DICTIONARY); + EXPECT_TRUE(Value::Equals(modify, static_cast<DictionaryValue*>(modified))); +} diff --git a/chrome/browser/prefs/pref_service.cc b/chrome/browser/prefs/pref_service.cc index d9588b5..8e7563f 100644 --- a/chrome/browser/prefs/pref_service.cc +++ b/chrome/browser/prefs/pref_service.cc @@ -407,10 +407,6 @@ bool PrefService::ReadOnly() const { return user_pref_store_->ReadOnly(); } -PrefNotifier* PrefService::pref_notifier() const { - return pref_notifier_.get(); -} - bool PrefService::IsManagedPreference(const char* pref_name) const { const Preference* pref = FindPreference(pref_name); return pref && pref->IsManaged(); @@ -571,7 +567,7 @@ DictionaryValue* PrefService::GetMutableDictionary(const char* path) { Value* tmp_value = NULL; // 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. - if (user_pref_store_->GetValue(path, &tmp_value) + if (user_pref_store_->GetMutableValue(path, &tmp_value) != PersistentPrefStore::READ_OK || !tmp_value->IsType(Value::TYPE_DICTIONARY)) { dict = new DictionaryValue; @@ -601,7 +597,7 @@ ListValue* PrefService::GetMutableList(const char* path) { Value* tmp_value = NULL; // 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. - if (user_pref_store_->GetValue(path, &tmp_value) + if (user_pref_store_->GetMutableValue(path, &tmp_value) != PersistentPrefStore::READ_OK || !tmp_value->IsType(Value::TYPE_LIST)) { list = new ListValue; @@ -612,7 +608,7 @@ ListValue* PrefService::GetMutableList(const char* path) { return list; } -void PrefService::ReportValueChanged(const std::string& key) { +void PrefService::ReportUserPrefChanged(const std::string& key) { user_pref_store_->ReportValueChanged(key); } @@ -657,7 +653,7 @@ const Value* PrefService::Preference::GetValue() const { DCHECK(pref_service_->FindPreference(name_.c_str())) << "Must register pref before getting its value"; - Value* found_value = NULL; + const Value* found_value = NULL; if (pref_value_store()->GetValue(name_, type_, &found_value)) { DCHECK(found_value->IsType(type_)); return found_value; diff --git a/chrome/browser/prefs/pref_service.h b/chrome/browser/prefs/pref_service.h index 5ca24fd..64b8227 100644 --- a/chrome/browser/prefs/pref_service.h +++ b/chrome/browser/prefs/pref_service.h @@ -220,8 +220,6 @@ class PrefService : public base::NonThreadSafe { // a mutable from the user preferences store. DictionaryValue* GetMutableDictionary(const char* path); ListValue* GetMutableList(const char* path); - // TODO(battre) remove this function (hack). - void ReportValueChanged(const std::string& key); // Returns true if a value has been set for the specified path. // NOTE: this is NOT the same as FindPreference. In particular @@ -277,7 +275,7 @@ class PrefService : public base::NonThreadSafe { friend class PrefChangeRegistrar; friend class subtle::PrefMemberBase; - // Give access to pref_notifier(); + // Give access to ReportUserPrefChanged(); friend class ScopedUserPrefUpdate; // Construct an incognito version of the pref service. Use @@ -285,9 +283,10 @@ class PrefService : public base::NonThreadSafe { PrefService(const PrefService& original, PrefStore* incognito_extension_prefs); - // Returns a PrefNotifier. If you desire access to this, you will probably - // want to use a ScopedUserPrefUpdate. - PrefNotifier* pref_notifier() const; + // Sends notification of a changed preference. This needs to be called by + // a ScopedUserPrefUpdate if a DictionaryValue or ListValue retrieved by the + // GetMutable... methods is changed. + void ReportUserPrefChanged(const std::string& key); // If the pref at the given path changes, we call the observer's Observe // method with PREF_CHANGED. Note that observers should not call these methods diff --git a/chrome/browser/prefs/pref_value_map.cc b/chrome/browser/prefs/pref_value_map.cc index 0857f6d..5eace84 100644 --- a/chrome/browser/prefs/pref_value_map.cc +++ b/chrome/browser/prefs/pref_value_map.cc @@ -15,7 +15,18 @@ PrefValueMap::~PrefValueMap() { Clear(); } -bool PrefValueMap::GetValue(const std::string& key, Value** value) const { +bool PrefValueMap::GetValue(const std::string& key, const Value** value) const { + const Map::const_iterator entry = prefs_.find(key); + if (entry != prefs_.end()) { + if (value) + *value = entry->second; + return true; + } + + return false; +} + +bool PrefValueMap::GetValue(const std::string& key, Value** value) { const Map::const_iterator entry = prefs_.find(key); if (entry != prefs_.end()) { if (value) @@ -76,13 +87,13 @@ PrefValueMap::const_iterator PrefValueMap::end() const { bool PrefValueMap::GetBoolean(const std::string& key, bool* value) const { - Value* stored_value = NULL; + const Value* stored_value = NULL; return GetValue(key, &stored_value) && stored_value->GetAsBoolean(value); } bool PrefValueMap::GetString(const std::string& key, std::string* value) const { - Value* stored_value = NULL; + const Value* stored_value = NULL; return GetValue(key, &stored_value) && stored_value->GetAsString(value); } diff --git a/chrome/browser/prefs/pref_value_map.h b/chrome/browser/prefs/pref_value_map.h index 19aeb05..d4b9f4e 100644 --- a/chrome/browser/prefs/pref_value_map.h +++ b/chrome/browser/prefs/pref_value_map.h @@ -26,7 +26,8 @@ class PrefValueMap { // Gets the value for |key| and stores it in |value|. Ownership remains with // the map. Returns true if a value is present. If not, |value| is not // touched. - bool GetValue(const std::string& key, Value** value) const; + bool GetValue(const std::string& key, const Value** value) const; + bool GetValue(const std::string& key, Value** value); // Sets a new |value| for |key|. Takes ownership of |value|, which must be // non-NULL. Returns true if the value changed. diff --git a/chrome/browser/prefs/pref_value_map_unittest.cc b/chrome/browser/prefs/pref_value_map_unittest.cc index 5e248b0..cc47a85 100644 --- a/chrome/browser/prefs/pref_value_map_unittest.cc +++ b/chrome/browser/prefs/pref_value_map_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -11,7 +11,7 @@ class PrefValueMapTest : public testing::Test { TEST_F(PrefValueMapTest, SetValue) { PrefValueMap map; - Value* result = NULL; + const Value* result = NULL; EXPECT_FALSE(map.GetValue("key", &result)); EXPECT_FALSE(result); diff --git a/chrome/browser/prefs/pref_value_store.cc b/chrome/browser/prefs/pref_value_store.cc index b97a4fb..3318a1d 100644 --- a/chrome/browser/prefs/pref_value_store.cc +++ b/chrome/browser/prefs/pref_value_store.cc @@ -102,7 +102,7 @@ PrefValueStore* PrefValueStore::CloneAndSpecialize( bool PrefValueStore::GetValue(const std::string& name, Value::ValueType type, - Value** out_value) const { + const Value** out_value) const { *out_value = NULL; // Check the |PrefStore|s in order of their priority from highest to lowest // to find the value of the preference described by the given preference name. @@ -175,7 +175,7 @@ bool PrefValueStore::PrefValueInStore( PrefValueStore::PrefStoreType store) const { // Declare a temp Value* and call GetValueFromStore, // ignoring the output value. - Value* tmp_value = NULL; + const Value* tmp_value = NULL; return GetValueFromStore(name, store, &tmp_value); } @@ -207,7 +207,7 @@ PrefValueStore::PrefStoreType PrefValueStore::ControllingPrefStoreForPref( bool PrefValueStore::GetValueFromStore(const char* name, PrefValueStore::PrefStoreType store_type, - Value** out_value) const { + const Value** out_value) const { // 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)); diff --git a/chrome/browser/prefs/pref_value_store.h b/chrome/browser/prefs/pref_value_store.h index bddbdaa..e477d1d 100644 --- a/chrome/browser/prefs/pref_value_store.h +++ b/chrome/browser/prefs/pref_value_store.h @@ -78,7 +78,7 @@ class PrefValueStore { // Preference::GetValue() instead of calling this method directly. bool GetValue(const std::string& name, Value::ValueType type, - Value** out_value) const; + const Value** out_value) const; // These methods return true if a preference with the given name is in the // indicated pref store, even if that value is currently being overridden by @@ -195,7 +195,7 @@ class PrefValueStore { // Get a value from the specified store type. bool GetValueFromStore(const char* name, PrefStoreType store, - Value** out_value) const; + const Value** out_value) const; // Called upon changes in individual pref stores in order to determine whether // the user-visible pref value has changed. Triggers the change notification diff --git a/chrome/browser/prefs/pref_value_store_unittest.cc b/chrome/browser/prefs/pref_value_store_unittest.cc index 81c068c..f711247 100644 --- a/chrome/browser/prefs/pref_value_store_unittest.cc +++ b/chrome/browser/prefs/pref_value_store_unittest.cc @@ -289,7 +289,7 @@ class PrefValueStoreTest : public testing::Test { }; TEST_F(PrefValueStoreTest, GetValue) { - Value* value; + const Value* value; // The following tests read a value from the PrefService. The preferences are // set in a way such that all lower-priority stores have a value and we can diff --git a/chrome/browser/prefs/scoped_user_pref_update.cc b/chrome/browser/prefs/scoped_user_pref_update.cc index 2c3c995..3ef0029 100644 --- a/chrome/browser/prefs/scoped_user_pref_update.cc +++ b/chrome/browser/prefs/scoped_user_pref_update.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -13,8 +13,5 @@ ScopedUserPrefUpdate::ScopedUserPrefUpdate( path_(path) {} ScopedUserPrefUpdate::~ScopedUserPrefUpdate() { - // TODO(mnissler, danno): This sends a notification unconditionally, which is - // wrong. We should rather tell the PrefService that the user pref got - // updated. - service_->ReportValueChanged(path_); + service_->ReportUserPrefChanged(path_); } diff --git a/chrome/browser/prefs/testing_pref_store.cc b/chrome/browser/prefs/testing_pref_store.cc index a3d33d8..e85a836 100644 --- a/chrome/browser/prefs/testing_pref_store.cc +++ b/chrome/browser/prefs/testing_pref_store.cc @@ -14,7 +14,12 @@ TestingPrefStore::TestingPrefStore() TestingPrefStore::~TestingPrefStore() {} PrefStore::ReadResult TestingPrefStore::GetValue(const std::string& key, - Value** value) const { + const Value** value) const { + return prefs_.GetValue(key, value) ? READ_OK : READ_NO_VALUE; +} + +PrefStore::ReadResult TestingPrefStore::GetMutableValue(const std::string& key, + Value** value) { return prefs_.GetValue(key, value) ? READ_OK : READ_NO_VALUE; } @@ -90,7 +95,7 @@ void TestingPrefStore::SetBoolean(const std::string& key, bool value) { bool TestingPrefStore::GetString(const std::string& key, std::string* value) const { - Value* stored_value; + const Value* stored_value; if (!prefs_.GetValue(key, &stored_value) || !stored_value) return false; @@ -98,7 +103,7 @@ bool TestingPrefStore::GetString(const std::string& key, } bool TestingPrefStore::GetInteger(const std::string& key, int* value) const { - Value* stored_value; + const Value* stored_value; if (!prefs_.GetValue(key, &stored_value) || !stored_value) return false; @@ -106,7 +111,7 @@ bool TestingPrefStore::GetInteger(const std::string& key, int* value) const { } bool TestingPrefStore::GetBoolean(const std::string& key, bool* value) const { - Value* stored_value; + const Value* stored_value; if (!prefs_.GetValue(key, &stored_value) || !stored_value) return false; diff --git a/chrome/browser/prefs/testing_pref_store.h b/chrome/browser/prefs/testing_pref_store.h index ee2f185..bd68e25 100644 --- a/chrome/browser/prefs/testing_pref_store.h +++ b/chrome/browser/prefs/testing_pref_store.h @@ -23,12 +23,14 @@ class TestingPrefStore : public PersistentPrefStore { virtual ~TestingPrefStore(); // Overriden from PrefStore. - virtual ReadResult GetValue(const std::string& key, Value** result) const; + virtual ReadResult GetValue(const std::string& key, + const Value** result) const; virtual void AddObserver(PrefStore::Observer* observer); virtual void RemoveObserver(PrefStore::Observer* observer); virtual bool IsInitializationComplete() const; // PersistentPrefStore overrides: + virtual ReadResult GetMutableValue(const std::string& key, Value** result); virtual void SetValue(const std::string& key, Value* value); virtual void SetValueSilently(const std::string& key, Value* value); virtual void RemoveValue(const std::string& key); diff --git a/chrome/browser/prefs/value_map_pref_store.cc b/chrome/browser/prefs/value_map_pref_store.cc index 58c4016..4d2c8ca 100644 --- a/chrome/browser/prefs/value_map_pref_store.cc +++ b/chrome/browser/prefs/value_map_pref_store.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -14,7 +14,7 @@ ValueMapPrefStore::ValueMapPrefStore() {} ValueMapPrefStore::~ValueMapPrefStore() {} PrefStore::ReadResult ValueMapPrefStore::GetValue(const std::string& key, - Value** value) const { + const Value** value) const { return prefs_.GetValue(key, value) ? READ_OK : READ_NO_VALUE; } diff --git a/chrome/browser/prefs/value_map_pref_store.h b/chrome/browser/prefs/value_map_pref_store.h index a8b66c4..c713156 100644 --- a/chrome/browser/prefs/value_map_pref_store.h +++ b/chrome/browser/prefs/value_map_pref_store.h @@ -22,7 +22,8 @@ class ValueMapPrefStore : public PrefStore { virtual ~ValueMapPrefStore(); // PrefStore overrides: - virtual ReadResult GetValue(const std::string& key, Value** value) const; + virtual ReadResult GetValue(const std::string& key, + const Value** value) const; virtual void AddObserver(PrefStore::Observer* observer); virtual void RemoveObserver(PrefStore::Observer* observer); diff --git a/chrome/common/json_pref_store.cc b/chrome/common/json_pref_store.cc index d20d05b..b8d323b 100644 --- a/chrome/common/json_pref_store.cc +++ b/chrome/common/json_pref_store.cc @@ -30,8 +30,13 @@ JsonPrefStore::~JsonPrefStore() { } PrefStore::ReadResult JsonPrefStore::GetValue(const std::string& key, - Value** result) const { - return prefs_->Get(key, result) ? READ_OK : READ_NO_VALUE; + const Value** result) const { + Value* tmp = NULL; + if (prefs_->Get(key, &tmp)) { + *result = tmp; + return READ_OK; + } + return READ_NO_VALUE; } void JsonPrefStore::AddObserver(PrefStore::Observer* observer) { @@ -42,6 +47,11 @@ void JsonPrefStore::RemoveObserver(PrefStore::Observer* observer) { observers_.RemoveObserver(observer); } +PrefStore::ReadResult JsonPrefStore::GetMutableValue(const std::string& key, + Value** result) { + return prefs_->Get(key, result) ? READ_OK : READ_NO_VALUE; +} + void JsonPrefStore::SetValue(const std::string& key, Value* value) { DCHECK(value); scoped_ptr<Value> new_value(value); diff --git a/chrome/common/json_pref_store.h b/chrome/common/json_pref_store.h index 8f1cbd7..0b02363 100644 --- a/chrome/common/json_pref_store.h +++ b/chrome/common/json_pref_store.h @@ -34,11 +34,13 @@ class JsonPrefStore : public PersistentPrefStore, virtual ~JsonPrefStore(); // PrefStore overrides: - virtual ReadResult GetValue(const std::string& key, Value** result) const; + virtual ReadResult GetValue(const std::string& key, + const Value** result) const; virtual void AddObserver(PrefStore::Observer* observer); virtual void RemoveObserver(PrefStore::Observer* observer); // PersistentPrefStore overrides: + virtual ReadResult GetMutableValue(const std::string& key, Value** result); virtual void SetValue(const std::string& key, Value* value); virtual void SetValueSilently(const std::string& key, Value* value); virtual void RemoveValue(const std::string& key); @@ -47,7 +49,6 @@ class JsonPrefStore : public PersistentPrefStore, virtual bool WritePrefs(); virtual void ScheduleWritePrefs(); virtual void CommitPendingWrite(); - // TODO(battre) remove this function virtual void ReportValueChanged(const std::string& key); private: diff --git a/chrome/common/json_pref_store_unittest.cc b/chrome/common/json_pref_store_unittest.cc index 1b98aa9..adf7d8d 100644 --- a/chrome/common/json_pref_store_unittest.cc +++ b/chrome/common/json_pref_store_unittest.cc @@ -108,7 +108,7 @@ TEST_F(JsonPrefStoreTest, Basic) { std::string cnn("http://www.cnn.com"); - Value* actual; + const Value* actual; EXPECT_EQ(PrefStore::READ_OK, pref_store->GetValue(prefs::kHomePage, &actual)); std::string string_value; diff --git a/chrome/common/persistent_pref_store.h b/chrome/common/persistent_pref_store.h index 300e649..8bd677a 100644 --- a/chrome/common/persistent_pref_store.h +++ b/chrome/common/persistent_pref_store.h @@ -34,6 +34,16 @@ class PersistentPrefStore : public PrefStore { PREF_READ_ERROR_FILE_NOT_SPECIFIED }; + // Equivalent to PrefStore::GetValue but returns a mutable value. + virtual ReadResult GetMutableValue(const std::string& key, + 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 + // value. SetValue takes care of notifications itself. Note that + // ReportValueChanged will trigger notifications even if nothing has changed. + virtual void ReportValueChanged(const std::string& key) = 0; + // Sets a |value| for |key| in the store. Assumes ownership of |value|, which // must be non-NULL. virtual void SetValue(const std::string& key, Value* value) = 0; @@ -51,9 +61,6 @@ class PersistentPrefStore : public PrefStore { // Removes the value for |key|. virtual void RemoveValue(const std::string& key) = 0; - // TODO(battre) Remove this function. - virtual void ReportValueChanged(const std::string& key) = 0; - // Whether the store is in a pseudo-read-only mode where changes are not // actually persisted to disk. This happens in some cases when there are // read errors during startup. diff --git a/chrome/common/pref_store.h b/chrome/common/pref_store.h index 819a764..a9185fe 100644 --- a/chrome/common/pref_store.h +++ b/chrome/common/pref_store.h @@ -55,7 +55,8 @@ class PrefStore : public base::RefCounted<PrefStore> { // Get the value for a given preference |key| and stores it in |result|. // |result| is only modified if the return value is READ_OK. Ownership of the // |result| value remains with the PrefStore. - virtual ReadResult GetValue(const std::string& key, Value** result) const = 0; + virtual ReadResult GetValue(const std::string& key, + const Value** result) const = 0; protected: friend class base::RefCounted<PrefStore>; diff --git a/chrome/service/cloud_print/cloud_print_proxy.cc b/chrome/service/cloud_print/cloud_print_proxy.cc index 10f147d..a6d070b 100644 --- a/chrome/service/cloud_print/cloud_print_proxy.cc +++ b/chrome/service/cloud_print/cloud_print_proxy.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -70,7 +70,7 @@ void CloudPrintProxy::EnableForUser(const std::string& lsid) { } // Getting print system specific settings from the preferences. - DictionaryValue* print_system_settings = NULL; + const DictionaryValue* print_system_settings = NULL; service_prefs_->GetDictionary(prefs::kCloudPrintPrintSystemSettings, &print_system_settings); diff --git a/chrome/service/service_process_prefs.cc b/chrome/service/service_process_prefs.cc index 9164712..7aa94b05 100644 --- a/chrome/service/service_process_prefs.cc +++ b/chrome/service/service_process_prefs.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -24,7 +24,7 @@ void ServiceProcessPrefs::WritePrefs() { void ServiceProcessPrefs::GetString(const std::string& key, std::string* result) { - Value* value; + const Value* value; if (prefs_->GetValue(key, &value) == PersistentPrefStore::READ_OK) value->GetAsString(result); } @@ -35,7 +35,7 @@ void ServiceProcessPrefs::SetString(const std::string& key, } void ServiceProcessPrefs::GetBoolean(const std::string& key, bool* result) { - Value* value; + const Value* value; if (prefs_->GetValue(key, &value) == PersistentPrefStore::READ_OK) value->GetAsBoolean(result); } @@ -45,12 +45,12 @@ void ServiceProcessPrefs::SetBoolean(const std::string& key, bool value) { } void ServiceProcessPrefs::GetDictionary(const std::string& key, - DictionaryValue** result) { - Value* value; + const DictionaryValue** result) { + const Value* value; if (prefs_->GetValue(key, &value) != PersistentPrefStore::READ_OK || !value->IsType(Value::TYPE_DICTIONARY)) { return; } - *result = static_cast<DictionaryValue*>(value); + *result = static_cast<const DictionaryValue*>(value); } diff --git a/chrome/service/service_process_prefs.h b/chrome/service/service_process_prefs.h index e936515..a16f620 100644 --- a/chrome/service/service_process_prefs.h +++ b/chrome/service/service_process_prefs.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -39,7 +39,7 @@ class ServiceProcessPrefs { void SetBoolean(const std::string& key, bool value); // Get a dictionary preference for |key| and store it in |result|. - void GetDictionary(const std::string& key, DictionaryValue** result); + void GetDictionary(const std::string& key, const DictionaryValue** result); private: scoped_refptr<JsonPrefStore> prefs_; diff --git a/chrome/test/testing_pref_service.cc b/chrome/test/testing_pref_service.cc index 043dae4..815ae1e 100644 --- a/chrome/test/testing_pref_service.cc +++ b/chrome/test/testing_pref_service.cc @@ -55,7 +55,7 @@ void TestingPrefServiceBase::RemoveUserPref(const char* path) { const Value* TestingPrefServiceBase::GetPref(TestingPrefStore* pref_store, const char* path) const { - Value* res; + const Value* res; return pref_store->GetValue(path, &res) == PrefStore::READ_OK ? res : NULL; } |