Skip to content

Commit

Permalink
Change PrefStore::ReadResult to a boolean.
Browse files Browse the repository at this point in the history 
The third value in the enum (READ_USE_DEFAULT) isn't used anymore.

TBR=phajdan.jr@chromium.org,abodenha@chromium.org,tim@chromium.org
BUG=none

Review URL: https://chromiumcodereview.appspot.com/11365112

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@166706 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
bauerb@chromium.org committed Nov 8, 2012
1 parent 47ddf33 commit 892f1d6
Show file tree
Hide file tree
Showing 25 changed files with 214 additions and 332 deletions.
12 changes: 6 additions & 6 deletions base/prefs/default_pref_store.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions base/prefs/default_pref_store.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
22 changes: 11 additions & 11 deletions base/prefs/json_pref_store.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
8 changes: 4 additions & 4 deletions base/prefs/json_pref_store.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
19 changes: 8 additions & 11 deletions base/prefs/json_pref_store_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,53 +96,50 @@ 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);
FilePath some_path(FILE_PATH_LITERAL("/usr/sbin/"));

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);
Expand Down
26 changes: 11 additions & 15 deletions base/prefs/overlay_user_pref_store.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions base/prefs/overlay_user_pref_store.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
56 changes: 23 additions & 33 deletions base/prefs/overlay_user_pref_store_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand All @@ -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)));
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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);
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions base/prefs/persistent_pref_store.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 892f1d6

Please sign in to comment.