diff options
author | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-11-25 20:47:52 +0000 |
---|---|---|
committer | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-11-25 20:47:52 +0000 |
commit | 4dad9ad838f6671fbd67e1c5292525e739e31983 (patch) | |
tree | 4d79fc17f12752cc221e0e40d16951677da71f92 | |
parent | 2b3f0f59a6761a41e22007c2c3096e8e18517e08 (diff) | |
download | chromium_src-4dad9ad838f6671fbd67e1c5292525e739e31983.zip chromium_src-4dad9ad838f6671fbd67e1c5292525e739e31983.tar.gz chromium_src-4dad9ad838f6671fbd67e1c5292525e739e31983.tar.bz2 |
Many changes to DictionaryValues:
* Add support for keys with "." in them via new XXXWithoutPathExpansion() APIs.
* Use these APIs with all key iterator usage.
* SetXXX() calls cannot fail, so change them from bool to void.
* Change GetSize() to size() since it's cheap, and add empty().
Other:
* Use standard for loop format in more places (e.g. instead of while loops when they're really doing a for loop).
* Shorten a few bits of code.
BUG=567
TEST=none
Review URL: http://codereview.chromium.org/441008
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@33109 0039d316-1c4b-4281-b951-d872f2087c98
34 files changed, 423 insertions, 320 deletions
diff --git a/base/json/json_reader.cc b/base/json/json_reader.cc index 06d790c..bdc682b 100644 --- a/base/json/json_reader.cc +++ b/base/json/json_reader.cc @@ -281,7 +281,8 @@ Value* JSONReader::BuildValue(bool is_root) { Value* dict_value = BuildValue(false); if (!dict_value) return NULL; - static_cast<DictionaryValue*>(node.get())->Set(dict_key, dict_value); + static_cast<DictionaryValue*>(node.get())->SetWithoutPathExpansion( + dict_key, dict_value); // After a key/value pair, we expect a comma or the end of the // object. diff --git a/base/json/json_reader_unittest.cc b/base/json/json_reader_unittest.cc index 8ae51c5..17dea56 100644 --- a/base/json/json_reader_unittest.cc +++ b/base/json/json_reader_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2009 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. @@ -297,8 +297,7 @@ TEST(JSONReaderTest, Reading) { ASSERT_TRUE(root->IsType(Value::TYPE_DICTIONARY)); root.reset(JSONReader::Read( - "{\"number\":9.87654321, \"null\":null , \"\\x53\" : \"str\" }", - false)); + "{\"number\":9.87654321, \"null\":null , \"\\x53\" : \"str\" }", false)); ASSERT_TRUE(root.get()); ASSERT_TRUE(root->IsType(Value::TYPE_DICTIONARY)); DictionaryValue* dict_val = static_cast<DictionaryValue*>(root.get()); @@ -313,32 +312,32 @@ TEST(JSONReaderTest, Reading) { ASSERT_EQ(L"str", str_val); root2.reset(JSONReader::Read( - "{\"number\":9.87654321, \"null\":null , \"\\x53\" : \"str\", }", true)); + "{\"number\":9.87654321, \"null\":null , \"\\x53\" : \"str\", }", true)); ASSERT_TRUE(root2.get()); EXPECT_TRUE(root->Equals(root2.get())); // Test newline equivalence. root2.reset(JSONReader::Read( - "{\n" - " \"number\":9.87654321,\n" - " \"null\":null,\n" - " \"\\x53\":\"str\",\n" - "}\n", true)); + "{\n" + " \"number\":9.87654321,\n" + " \"null\":null,\n" + " \"\\x53\":\"str\",\n" + "}\n", true)); ASSERT_TRUE(root2.get()); EXPECT_TRUE(root->Equals(root2.get())); root2.reset(JSONReader::Read( - "{\r\n" - " \"number\":9.87654321,\r\n" - " \"null\":null,\r\n" - " \"\\x53\":\"str\",\r\n" - "}\r\n", true)); + "{\r\n" + " \"number\":9.87654321,\r\n" + " \"null\":null,\r\n" + " \"\\x53\":\"str\",\r\n" + "}\r\n", true)); ASSERT_TRUE(root2.get()); EXPECT_TRUE(root->Equals(root2.get())); // Test nesting root.reset(JSONReader::Read( - "{\"inner\":{\"array\":[true]},\"false\":false,\"d\":{}}", false)); + "{\"inner\":{\"array\":[true]},\"false\":false,\"d\":{}}", false)); ASSERT_TRUE(root.get()); ASSERT_TRUE(root->IsType(Value::TYPE_DICTIONARY)); dict_val = static_cast<DictionaryValue*>(root.get()); @@ -354,9 +353,37 @@ TEST(JSONReaderTest, Reading) { ASSERT_TRUE(dict_val->GetDictionary(L"d", &inner_dict)); root2.reset(JSONReader::Read( - "{\"inner\": {\"array\":[true] , },\"false\":false,\"d\":{},}", true)); + "{\"inner\": {\"array\":[true] , },\"false\":false,\"d\":{},}", true)); EXPECT_TRUE(root->Equals(root2.get())); + // Test keys with periods + root.reset(JSONReader::Read( + "{\"a.b\":3,\"c\":2,\"d.e.f\":{\"g.h.i.j\":1}}", false)); + ASSERT_TRUE(root.get()); + ASSERT_TRUE(root->IsType(Value::TYPE_DICTIONARY)); + dict_val = static_cast<DictionaryValue*>(root.get()); + int integer_value = 0; + EXPECT_TRUE(dict_val->GetIntegerWithoutPathExpansion(L"a.b", &integer_value)); + EXPECT_EQ(3, integer_value); + EXPECT_TRUE(dict_val->GetIntegerWithoutPathExpansion(L"c", &integer_value)); + EXPECT_EQ(2, integer_value); + inner_dict = NULL; + ASSERT_TRUE(dict_val->GetDictionaryWithoutPathExpansion(L"d.e.f", + &inner_dict)); + ASSERT_EQ(1U, inner_dict->size()); + EXPECT_TRUE(inner_dict->GetIntegerWithoutPathExpansion(L"g.h.i.j", + &integer_value)); + EXPECT_EQ(1, integer_value); + + root.reset(JSONReader::Read("{\"a\":{\"b\":2},\"a.b\":1}", false)); + ASSERT_TRUE(root.get()); + ASSERT_TRUE(root->IsType(Value::TYPE_DICTIONARY)); + dict_val = static_cast<DictionaryValue*>(root.get()); + EXPECT_TRUE(dict_val->GetInteger(L"a.b", &integer_value)); + EXPECT_EQ(2, integer_value); + EXPECT_TRUE(dict_val->GetIntegerWithoutPathExpansion(L"a.b", &integer_value)); + EXPECT_EQ(1, integer_value); + // Invalid, no closing brace root.reset(JSONReader::Read("{\"a\": true", false)); ASSERT_FALSE(root.get()); diff --git a/base/json/json_writer.cc b/base/json/json_writer.cc index 133b625..ffdad76 100644 --- a/base/json/json_writer.cc +++ b/base/json/json_writer.cc @@ -157,7 +157,7 @@ void JSONWriter::BuildJSONString(const Value* const node, } Value* value = NULL; - bool result = dict->Get(*key_itr, &value); + bool result = dict->GetWithoutPathExpansion(*key_itr, &value); DCHECK(result); if (pretty_print_) diff --git a/base/json/json_writer_unittest.cc b/base/json/json_writer_unittest.cc index cd2a6fe..e7d0f05 100644 --- a/base/json/json_writer_unittest.cc +++ b/base/json/json_writer_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2009 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. @@ -76,6 +76,23 @@ TEST(JSONWriterTest, Writing) { "}" JSON_NEWLINE, output_js); #undef JSON_NEWLINE + + // Test keys with periods + DictionaryValue period_dict; + period_dict.SetWithoutPathExpansion(L"a.b", Value::CreateIntegerValue(3)); + period_dict.SetWithoutPathExpansion(L"c", Value::CreateIntegerValue(2)); + DictionaryValue* period_dict2 = new DictionaryValue; + period_dict2->SetWithoutPathExpansion(L"g.h.i.j", + Value::CreateIntegerValue(1)); + period_dict.SetWithoutPathExpansion(L"d.e.f", period_dict2); + JSONWriter::Write(&period_dict, false, &output_js); + ASSERT_EQ("{\"a.b\":3,\"c\":2,\"d.e.f\":{\"g.h.i.j\":1}}", output_js); + + DictionaryValue period_dict3; + period_dict3.Set(L"a.b", Value::CreateIntegerValue(2)); + period_dict3.SetWithoutPathExpansion(L"a.b", Value::CreateIntegerValue(1)); + JSONWriter::Write(&period_dict3, false, &output_js); + ASSERT_EQ("{\"a\":{\"b\":2},\"a.b\":1}", output_js); } } // namespace base diff --git a/base/values.cc b/base/values.cc index 305f1cb..f66b5968 100644 --- a/base/values.cc +++ b/base/values.cc @@ -253,92 +253,79 @@ bool DictionaryValue::HasKey(const std::wstring& key) const { return current_entry != dictionary_.end(); } -void DictionaryValue::SetInCurrentNode(const std::wstring& key, - Value* in_value) { - // If there's an existing value here, we need to delete it, because - // we own all our children. - if (HasKey(key)) { - DCHECK(dictionary_[key] != in_value); // This would be bogus - delete dictionary_[key]; - } - - dictionary_[key] = in_value; -} - -bool DictionaryValue::Set(const std::wstring& path, Value* in_value) { +void DictionaryValue::Set(const std::wstring& path, Value* in_value) { DCHECK(in_value); - std::wstring key = path; - - size_t delimiter_position = path.find_first_of(L".", 0); - // If there isn't a dictionary delimiter in the path, we're done. - if (delimiter_position == std::wstring::npos) { - SetInCurrentNode(key, in_value); - return true; - } else { - key = path.substr(0, delimiter_position); - } + std::wstring current_path(path); + DictionaryValue* current_dictionary = this; + for (size_t delimiter_position = current_path.find('.'); + delimiter_position != std::wstring::npos; + delimiter_position = current_path.find('.')) { + // Assume that we're indexing into a dictionary. + std::wstring key(current_path, 0, delimiter_position); + DictionaryValue* child_dictionary; + if (!current_dictionary->GetDictionary(key, &child_dictionary)) { + child_dictionary = new DictionaryValue; + current_dictionary->SetWithoutPathExpansion(key, child_dictionary); + } - // Assume that we're indexing into a dictionary. - DictionaryValue* entry = NULL; - if (!HasKey(key) || (dictionary_[key]->GetType() != TYPE_DICTIONARY)) { - entry = new DictionaryValue; - SetInCurrentNode(key, entry); - } else { - entry = static_cast<DictionaryValue*>(dictionary_[key]); + current_dictionary = child_dictionary; + current_path.erase(0, delimiter_position + 1); } - std::wstring remaining_path = path.substr(delimiter_position + 1); - return entry->Set(remaining_path, in_value); + current_dictionary->SetWithoutPathExpansion(current_path, in_value); } -bool DictionaryValue::SetBoolean(const std::wstring& path, bool in_value) { - return Set(path, CreateBooleanValue(in_value)); +void DictionaryValue::SetBoolean(const std::wstring& path, bool in_value) { + Set(path, CreateBooleanValue(in_value)); } -bool DictionaryValue::SetInteger(const std::wstring& path, int in_value) { - return Set(path, CreateIntegerValue(in_value)); +void DictionaryValue::SetInteger(const std::wstring& path, int in_value) { + Set(path, CreateIntegerValue(in_value)); } -bool DictionaryValue::SetReal(const std::wstring& path, double in_value) { - return Set(path, CreateRealValue(in_value)); +void DictionaryValue::SetReal(const std::wstring& path, double in_value) { + Set(path, CreateRealValue(in_value)); } -bool DictionaryValue::SetString(const std::wstring& path, +void DictionaryValue::SetString(const std::wstring& path, const std::string& in_value) { - return Set(path, CreateStringValue(in_value)); + Set(path, CreateStringValue(in_value)); } -bool DictionaryValue::SetString(const std::wstring& path, +void DictionaryValue::SetString(const std::wstring& path, const std::wstring& in_value) { - return Set(path, CreateStringValue(in_value)); + Set(path, CreateStringValue(in_value)); } -bool DictionaryValue::Get(const std::wstring& path, Value** out_value) const { - std::wstring key = path; - - size_t delimiter_position = path.find_first_of(L".", 0); - if (delimiter_position != std::wstring::npos) { - key = path.substr(0, delimiter_position); +void DictionaryValue::SetWithoutPathExpansion(const std::wstring& key, + Value* in_value) { + // If there's an existing value here, we need to delete it, because + // we own all our children. + if (HasKey(key)) { + DCHECK(dictionary_[key] != in_value); // This would be bogus + delete dictionary_[key]; } - ValueMap::const_iterator entry_iterator = dictionary_.find(key); - if (entry_iterator == dictionary_.end()) - return false; - Value* entry = entry_iterator->second; + dictionary_[key] = in_value; +} - if (delimiter_position == std::wstring::npos) { - if (out_value) - *out_value = entry; - return true; - } +bool DictionaryValue::Get(const std::wstring& path, Value** out_value) const { + std::wstring current_path(path); + const DictionaryValue* current_dictionary = this; + for (size_t delimiter_position = current_path.find('.'); + delimiter_position != std::wstring::npos; + delimiter_position = current_path.find('.')) { + DictionaryValue* child_dictionary; + if (!current_dictionary->GetDictionary( + current_path.substr(0, delimiter_position), &child_dictionary)) + return false; - if (entry->IsType(TYPE_DICTIONARY)) { - DictionaryValue* dictionary = static_cast<DictionaryValue*>(entry); - return dictionary->Get(path.substr(delimiter_position + 1), out_value); + current_dictionary = child_dictionary; + current_path.erase(0, delimiter_position + 1); } - return false; + return current_dictionary->GetWithoutPathExpansion(current_path, out_value); } bool DictionaryValue::GetBoolean(const std::wstring& path, @@ -425,44 +412,111 @@ bool DictionaryValue::GetList(const std::wstring& path, return true; } -bool DictionaryValue::Remove(const std::wstring& path, Value** out_value) { - std::wstring key = path; - - size_t delimiter_position = path.find_first_of(L".", 0); - if (delimiter_position != std::wstring::npos) { - key = path.substr(0, delimiter_position); - } - - ValueMap::iterator entry_iterator = dictionary_.find(key); +bool DictionaryValue::GetWithoutPathExpansion(const std::wstring& key, + Value** out_value) const { + ValueMap::const_iterator entry_iterator = dictionary_.find(key); if (entry_iterator == dictionary_.end()) return false; + Value* entry = entry_iterator->second; + if (out_value) + *out_value = entry; + return true; +} - if (delimiter_position == std::wstring::npos) { - if (out_value) - *out_value = entry; - else - delete entry; +bool DictionaryValue::GetIntegerWithoutPathExpansion(const std::wstring& path, + int* out_value) const { + Value* value; + if (!GetWithoutPathExpansion(path, &value)) + return false; - dictionary_.erase(entry_iterator); - return true; - } + return value->GetAsInteger(out_value); +} + +bool DictionaryValue::GetStringWithoutPathExpansion( + const std::wstring& path, + std::string* out_value) const { + Value* value; + if (!GetWithoutPathExpansion(path, &value)) + return false; + + return value->GetAsString(out_value); +} + +bool DictionaryValue::GetStringWithoutPathExpansion( + const std::wstring& path, + std::wstring* out_value) const { + Value* value; + if (!GetWithoutPathExpansion(path, &value)) + return false; + + return value->GetAsString(out_value); +} + +bool DictionaryValue::GetDictionaryWithoutPathExpansion( + const std::wstring& path, + DictionaryValue** out_value) const { + Value* value; + bool result = GetWithoutPathExpansion(path, &value); + if (!result || !value->IsType(TYPE_DICTIONARY)) + return false; + + if (out_value) + *out_value = static_cast<DictionaryValue*>(value); + + return true; +} - if (entry->IsType(TYPE_DICTIONARY)) { - DictionaryValue* dictionary = static_cast<DictionaryValue*>(entry); - return dictionary->Remove(path.substr(delimiter_position + 1), out_value); +bool DictionaryValue::GetListWithoutPathExpansion(const std::wstring& path, + ListValue** out_value) const { + Value* value; + bool result = GetWithoutPathExpansion(path, &value); + if (!result || !value->IsType(TYPE_LIST)) + return false; + + if (out_value) + *out_value = static_cast<ListValue*>(value); + + return true; +} + +bool DictionaryValue::Remove(const std::wstring& path, Value** out_value) { + std::wstring current_path(path); + DictionaryValue* current_dictionary = this; + size_t delimiter_position = current_path.rfind('.'); + if (delimiter_position != std::wstring::npos) { + if (!GetDictionary(current_path.substr(0, delimiter_position), + ¤t_dictionary)) + return false; + current_path.erase(0, delimiter_position + 1); } - return false; + return current_dictionary->RemoveWithoutPathExpansion(current_path, + out_value); +} + +bool DictionaryValue::RemoveWithoutPathExpansion(const std::wstring& key, + Value** out_value) { + ValueMap::iterator entry_iterator = dictionary_.find(key); + if (entry_iterator == dictionary_.end()) + return false; + + Value* entry = entry_iterator->second; + if (out_value) + *out_value = entry; + else + delete entry; + dictionary_.erase(entry_iterator); + return true; } Value* DictionaryValue::DeepCopy() const { DictionaryValue* result = new DictionaryValue; - ValueMap::const_iterator current_entry = dictionary_.begin(); - while (current_entry != dictionary_.end()) { - result->Set(current_entry->first, current_entry->second->DeepCopy()); - ++current_entry; + for (ValueMap::const_iterator current_entry(dictionary_.begin()); + current_entry != dictionary_.end(); ++current_entry) { + result->SetWithoutPathExpansion(current_entry->first, + current_entry->second->DeepCopy()); } return result; @@ -479,7 +533,8 @@ bool DictionaryValue::Equals(const Value* other) const { while (lhs_it != end_keys() && rhs_it != other_dict->end_keys()) { Value* lhs; Value* rhs; - if (!Get(*lhs_it, &lhs) || !other_dict->Get(*rhs_it, &rhs) || + if (!GetWithoutPathExpansion(*lhs_it, &lhs) || + !other_dict->GetWithoutPathExpansion(*rhs_it, &rhs) || !lhs->Equals(rhs)) { return false; } diff --git a/base/values.h b/base/values.h index b3c380c..49ce287 100644 --- a/base/values.h +++ b/base/values.h @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2009 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. @@ -207,7 +207,10 @@ class DictionaryValue : public Value { bool HasKey(const std::wstring& key) const; // Returns the number of Values in this dictionary. - size_t GetSize() const { return dictionary_.size(); } + size_t size() const { return dictionary_.size(); } + + // Returns whether the dictionary is empty. + bool empty() const { return dictionary_.empty(); } // Clears any current contents of this dictionary. void Clear(); @@ -220,16 +223,20 @@ class DictionaryValue : public Value { // a DictionaryValue, a new DictionaryValue will be created and attached // to the path in that location. // Note that the dictionary takes ownership of the value referenced by - // |in_value|. - bool Set(const std::wstring& path, Value* in_value); + // |in_value|, and therefore |in_value| must be non-NULL. + void Set(const std::wstring& path, Value* in_value); // Convenience forms of Set(). These methods will replace any existing // value at that path, even if it has a different type. - bool SetBoolean(const std::wstring& path, bool in_value); - bool SetInteger(const std::wstring& path, int in_value); - bool SetReal(const std::wstring& path, double in_value); - bool SetString(const std::wstring& path, const std::string& in_value); - bool SetString(const std::wstring& path, const std::wstring& in_value); + void SetBoolean(const std::wstring& path, bool in_value); + void SetInteger(const std::wstring& path, int in_value); + void SetReal(const std::wstring& path, double in_value); + void SetString(const std::wstring& path, const std::string& in_value); + void SetString(const std::wstring& path, const std::wstring& in_value); + + // Like Set(), but without special treatment of '.'. This allows e.g. URLs to + // be used as paths. + void SetWithoutPathExpansion(const std::wstring& key, Value* in_value); // Gets the Value associated with the given path starting from this object. // A path has the form "<key>" or "<key>.<key>.[...]", where "." indexes @@ -253,6 +260,21 @@ class DictionaryValue : public Value { DictionaryValue** out_value) const; bool GetList(const std::wstring& path, ListValue** out_value) const; + // Like Get(), but without special treatment of '.'. This allows e.g. URLs to + // be used as paths. + bool GetWithoutPathExpansion(const std::wstring& key, + Value** out_value) const; + bool GetIntegerWithoutPathExpansion(const std::wstring& path, + int* out_value) const; + bool GetStringWithoutPathExpansion(const std::wstring& path, + std::string* out_value) const; + bool GetStringWithoutPathExpansion(const std::wstring& path, + std::wstring* out_value) const; + bool GetDictionaryWithoutPathExpansion(const std::wstring& path, + DictionaryValue** out_value) const; + bool GetListWithoutPathExpansion(const std::wstring& path, + ListValue** out_value) const; + // Removes the Value with the specified path from this dictionary (or one // of its child dictionaries, if the path is more than just a local key). // If |out_value| is non-NULL, the removed Value AND ITS OWNERSHIP will be @@ -261,8 +283,16 @@ class DictionaryValue : public Value { // it will return false and the DictionaryValue object will be unchanged. bool Remove(const std::wstring& path, Value** out_value); + // Like Remove(), but without special treatment of '.'. This allows e.g. URLs + // to be used as paths. + bool RemoveWithoutPathExpansion(const std::wstring& key, Value** out_value); + // This class provides an iterator for the keys in the dictionary. // It can't be used to modify the dictionary. + // + // YOU SHOULD ALWAYS USE THE XXXWithoutPathExpansion() APIs WITH THESE, NOT + // THE NORMAL XXX() APIs. This makes sure things will work correctly if any + // keys have '.'s in them. class key_iterator : private std::iterator<std::input_iterator_tag, const std::wstring> { public: @@ -280,11 +310,6 @@ class DictionaryValue : public Value { key_iterator end_keys() const { return key_iterator(dictionary_.end()); } private: - // Associates the value |in_value| with the |key|. This method should be - // used instead of "dictionary_[key] = foo" so that any previous value can - // be properly deleted. - void SetInCurrentNode(const std::wstring& key, Value* in_value); - ValueMap dictionary_; DISALLOW_COPY_AND_ASSIGN(DictionaryValue); diff --git a/base/values_unittest.cc b/base/values_unittest.cc index 5253eca..0182d76 100644 --- a/base/values_unittest.cc +++ b/base/values_unittest.cc @@ -20,9 +20,9 @@ TEST(ValuesTest, Basic) { ASSERT_EQ(std::wstring(L"http://google.com"), homepage); ASSERT_FALSE(settings.Get(L"global", NULL)); - ASSERT_TRUE(settings.Set(L"global", Value::CreateBooleanValue(true))); + settings.Set(L"global", Value::CreateBooleanValue(true)); ASSERT_TRUE(settings.Get(L"global", NULL)); - ASSERT_TRUE(settings.SetString(L"global.homepage", L"http://scurvy.com")); + settings.SetString(L"global.homepage", L"http://scurvy.com"); ASSERT_TRUE(settings.Get(L"global", NULL)); homepage = L"http://google.com"; ASSERT_TRUE(settings.GetString(L"global.homepage", &homepage)); @@ -285,6 +285,28 @@ TEST(ValuesTest, DictionaryRemoval) { } } +TEST(ValuesTest, DictionaryWithoutPathExpansion) { + DictionaryValue dict; + dict.Set(L"this.is.expanded", Value::CreateNullValue()); + dict.SetWithoutPathExpansion(L"this.isnt.expanded", Value::CreateNullValue()); + + EXPECT_FALSE(dict.HasKey(L"this.is.expanded")); + EXPECT_TRUE(dict.HasKey(L"this")); + Value* value1; + EXPECT_TRUE(dict.Get(L"this", &value1)); + DictionaryValue* value2; + ASSERT_TRUE(dict.GetDictionaryWithoutPathExpansion(L"this", &value2)); + EXPECT_EQ(value1, value2); + EXPECT_EQ(1U, value2->size()); + + EXPECT_TRUE(dict.HasKey(L"this.isnt.expanded")); + Value* value3; + EXPECT_FALSE(dict.Get(L"this.isnt.expanded", &value3)); + Value* value4; + ASSERT_TRUE(dict.GetWithoutPathExpansion(L"this.isnt.expanded", &value4)); + EXPECT_EQ(Value::TYPE_NULL, value4->GetType()); +} + TEST(ValuesTest, DeepCopy) { DictionaryValue original_dict; Value* original_null = Value::CreateNullValue(); diff --git a/chrome/browser/blocked_popup_container.cc b/chrome/browser/blocked_popup_container.cc index 2422c2b..e4c2184 100644 --- a/chrome/browser/blocked_popup_container.cc +++ b/chrome/browser/blocked_popup_container.cc @@ -391,13 +391,13 @@ BlockedPopupContainer::BlockedPopupContainer(TabContents* owner, const ListValue* whitelist_pref = prefs_->GetList(prefs::kPopupWhitelistedHosts); // Careful: The returned value could be NULL if the pref has never been set. - if (whitelist_pref != NULL) { - for (ListValue::const_iterator i(whitelist_pref->begin()); - i != whitelist_pref->end(); ++i) { - std::string host; - (*i)->GetAsString(&host); - whitelist_.insert(host); - } + if (whitelist_pref == NULL) + return; + for (ListValue::const_iterator i(whitelist_pref->begin()); + i != whitelist_pref->end(); ++i) { + std::string host; + (*i)->GetAsString(&host); + whitelist_.insert(host); } } diff --git a/chrome/browser/bookmarks/bookmark_codec_unittest.cc b/chrome/browser/bookmarks/bookmark_codec_unittest.cc index 0ba16a7..8272270 100644 --- a/chrome/browser/bookmarks/bookmark_codec_unittest.cc +++ b/chrome/browser/bookmarks/bookmark_codec_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2009 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. @@ -202,14 +202,14 @@ TEST_F(BookmarkCodecTest, ChecksumManualEditTest) { GetBookmarksBarChildValue(value.get(), 0, &child1_value); std::wstring title; ASSERT_TRUE(child1_value->GetString(BookmarkCodec::kNameKey, &title)); - ASSERT_TRUE(child1_value->SetString(BookmarkCodec::kNameKey, title + L"1")); + child1_value->SetString(BookmarkCodec::kNameKey, title + L"1"); std::string dec_checksum; scoped_ptr<BookmarkModel> decoded_model1(DecodeHelper( *value.get(), enc_checksum, &dec_checksum, true)); // Undo the change and make sure the checksum is same as original. - ASSERT_TRUE(child1_value->SetString(BookmarkCodec::kNameKey, title)); + child1_value->SetString(BookmarkCodec::kNameKey, title); scoped_ptr<BookmarkModel> decoded_model2(DecodeHelper( *value.get(), enc_checksum, &dec_checksum, false)); } @@ -233,7 +233,7 @@ TEST_F(BookmarkCodecTest, ChecksumManualEditIDsTest) { GetBookmarksBarChildValue(value.get(), i, &child_value); std::string id; ASSERT_TRUE(child_value->GetString(BookmarkCodec::kIdKey, &id)); - ASSERT_TRUE(child_value->SetString(BookmarkCodec::kIdKey, "1")); + child_value->SetString(BookmarkCodec::kIdKey, "1"); } std::string dec_checksum; diff --git a/chrome/browser/browser_theme_provider.cc b/chrome/browser/browser_theme_provider.cc index 6bc9e97..be81770 100644 --- a/chrome/browser/browser_theme_provider.cc +++ b/chrome/browser/browser_theme_provider.cc @@ -1075,7 +1075,7 @@ void BrowserThemeProvider::SetImageData(DictionaryValue* images_value, for (DictionaryValue::key_iterator iter(images_value->begin_keys()); iter != images_value->end_keys(); ++iter) { std::string val; - if (images_value->GetString(*iter, &val)) { + if (images_value->GetStringWithoutPathExpansion(*iter, &val)) { int id = ThemeResourcesUtil::GetId(WideToUTF8(*iter)); if (id != -1) { if (!images_path.empty()) { @@ -1099,7 +1099,7 @@ void BrowserThemeProvider::SetColorData(DictionaryValue* colors_value) { for (DictionaryValue::key_iterator iter(colors_value->begin_keys()); iter != colors_value->end_keys(); ++iter) { ListValue* color_list; - if (colors_value->GetList(*iter, &color_list) && + if (colors_value->GetListWithoutPathExpansion(*iter, &color_list) && ((color_list->GetSize() == 3) || (color_list->GetSize() == 4))) { int r, g, b; color_list->GetInteger(0, &r); @@ -1132,7 +1132,7 @@ void BrowserThemeProvider::SetTintData(DictionaryValue* tints_value) { for (DictionaryValue::key_iterator iter(tints_value->begin_keys()); iter != tints_value->end_keys(); ++iter) { ListValue* tint_list; - if (tints_value->GetList(*iter, &tint_list) && + if (tints_value->GetListWithoutPathExpansion(*iter, &tint_list) && (tint_list->GetSize() == 3)) { color_utils::HSL hsl = { -1, -1, -1 }; int value = 0; @@ -1162,22 +1162,21 @@ void BrowserThemeProvider::SetDisplayPropertyData( if (base::strcasecmp(WideToUTF8(*iter).c_str(), kDisplayPropertyNTPAlignment) == 0) { std::string val; - if (display_properties_value->GetString(*iter, &val)) { + if (display_properties_value->GetStringWithoutPathExpansion(*iter, + &val)) { display_properties_[kDisplayPropertyNTPAlignment] = StringToAlignment(val); } } else if (base::strcasecmp(WideToUTF8(*iter).c_str(), kDisplayPropertyNTPTiling) == 0) { std::string val; - if (display_properties_value->GetString(*iter, &val)) { - display_properties_[kDisplayPropertyNTPTiling] = - StringToTiling(val); - } + if (display_properties_value->GetStringWithoutPathExpansion(*iter, &val)) + display_properties_[kDisplayPropertyNTPTiling] = StringToTiling(val); } if (base::strcasecmp(WideToUTF8(*iter).c_str(), kDisplayPropertyNTPInverseLogo) == 0) { int val = 0; - if (display_properties_value->GetInteger(*iter, &val)) + if (display_properties_value->GetIntegerWithoutPathExpansion(*iter, &val)) display_properties_[kDisplayPropertyNTPInverseLogo] = val; } } @@ -1222,10 +1221,12 @@ void BrowserThemeProvider::SaveImageData(DictionaryValue* images_value) const { for (DictionaryValue::key_iterator iter(images_value->begin_keys()); iter != images_value->end_keys(); ++iter) { std::string val; - if (images_value->GetString(*iter, &val)) { + if (images_value->GetStringWithoutPathExpansion(*iter, &val)) { int id = ThemeResourcesUtil::GetId(WideToUTF8(*iter)); - if (id != -1) - pref_images->SetString(*iter, images_.find(id)->second); + if (id != -1) { + pref_images->SetWithoutPathExpansion(*iter, + Value::CreateStringValue(images_.find(id)->second)); + } } } } diff --git a/chrome/browser/dom_ui/most_visited_handler.cc b/chrome/browser/dom_ui/most_visited_handler.cc index 8901d3f9..853a3c9 100644 --- a/chrome/browser/dom_ui/most_visited_handler.cc +++ b/chrome/browser/dom_ui/most_visited_handler.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2009 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. @@ -126,7 +126,7 @@ void MostVisitedHandler::StartQueryForMostVisited() { // we'll be filtering-out the returned list with the blacklist URLs. // We do not subtract the number of pinned URLs we have because the // HistoryService does not know about those. - const int result_count = page_count + url_blacklist_->GetSize(); + const int result_count = page_count + url_blacklist_->size(); HistoryService* hs = dom_ui_->GetProfile()->GetHistoryService(Profile::EXPLICIT_ACCESS); // |hs| may be null during unit tests. @@ -230,12 +230,8 @@ void MostVisitedHandler::AddPinnedURL(const MostVisitedPage& page, int index) { DictionaryValue* new_value = new DictionaryValue(); SetMostVisistedPage(new_value, page); - bool r = new_value->SetInteger(L"index", index); - DCHECK(r) << "Failed to set the index for a pinned URL from the NTP Most " - << "Visited."; - - r = pinned_urls_->Set(GetDictionaryKeyForURL(page.url.spec()), new_value); - DCHECK(r) << "Failed to add pinned URL from the NTP Most Visited."; + new_value->SetInteger(L"index", index); + pinned_urls_->Set(GetDictionaryKeyForURL(page.url.spec()), new_value); // TODO(arv): Notify observers? @@ -276,7 +272,7 @@ const bool MostVisitedHandler::GetPinnedURLAtIndex(const int index, for (DictionaryValue::key_iterator it = pinned_urls_->begin_keys(); it != pinned_urls_->end_keys(); ++it) { Value* value; - if (pinned_urls_->Get(*it, &value)) { + if (pinned_urls_->GetWithoutPathExpansion(*it, &value)) { if (!value->IsType(DictionaryValue::TYPE_DICTIONARY)) { NOTREACHED(); return false; diff --git a/chrome/browser/dom_ui/tips_handler.cc b/chrome/browser/dom_ui/tips_handler.cc index 35d3cc2..ee429f7 100644 --- a/chrome/browser/dom_ui/tips_handler.cc +++ b/chrome/browser/dom_ui/tips_handler.cc @@ -52,7 +52,7 @@ void TipsHandler::HandleGetTips(const Value* content) { } } - if (tips_cache_ != NULL && tips_cache_->GetSize() != 0) { + if (tips_cache_ != NULL && !tips_cache_->empty()) { if (tips_cache_->GetInteger( WebResourceService::kCurrentTipPrefName, ¤t_tip_index) && tips_cache_->GetList( diff --git a/chrome/browser/extensions/execute_code_in_tab_function.cc b/chrome/browser/extensions/execute_code_in_tab_function.cc index 3fb0e86..98757d2 100644 --- a/chrome/browser/extensions/execute_code_in_tab_function.cc +++ b/chrome/browser/extensions/execute_code_in_tab_function.cc @@ -25,7 +25,7 @@ bool ExecuteCodeInTabFunction::RunImpl() { DictionaryValue* script_info; EXTENSION_FUNCTION_VALIDATE(args->GetDictionary(1, &script_info)); - size_t number_of_value = script_info->GetSize(); + size_t number_of_value = script_info->size(); if (number_of_value == 0) { error_ = keys::kNoCodeOrFileToExecuteError; return false; diff --git a/chrome/browser/extensions/extension_file_util.cc b/chrome/browser/extensions/extension_file_util.cc index e7ecd08..f61dfef 100644 --- a/chrome/browser/extensions/extension_file_util.cc +++ b/chrome/browser/extensions/extension_file_util.cc @@ -169,7 +169,7 @@ bool ValidateExtension(Extension* extension, std::string* error) { for (DictionaryValue::key_iterator iter = images_value->begin_keys(); iter != images_value->end_keys(); ++iter) { std::string val; - if (images_value->GetString(*iter, &val)) { + if (images_value->GetStringWithoutPathExpansion(*iter, &val)) { FilePath image_path = extension->path().AppendASCII(val); if (!file_util::PathExists(image_path)) { *error = StringPrintf( diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc index d638847..c793fb3b 100644 --- a/chrome/browser/extensions/extension_prefs.cc +++ b/chrome/browser/extensions/extension_prefs.cc @@ -50,10 +50,12 @@ InstalledExtensions::~InstalledExtensions() { void InstalledExtensions::VisitInstalledExtensions( InstalledExtensions::Callback *callback) { scoped_ptr<InstalledExtensions::Callback> cleanup(callback); - DictionaryValue::key_iterator extension_id = extension_data_->begin_keys(); - for (; extension_id != extension_data_->end_keys(); ++extension_id) { + for (DictionaryValue::key_iterator extension_id( + extension_data_->begin_keys()); + extension_id != extension_data_->end_keys(); ++extension_id) { DictionaryValue* ext; - if (!extension_data_->GetDictionary(*extension_id, &ext)) { + if (!extension_data_->GetDictionaryWithoutPathExpansion(*extension_id, + &ext)) { LOG(WARNING) << "Invalid pref for extension " << *extension_id; NOTREACHED(); continue; @@ -125,13 +127,13 @@ static FilePath::StringType MakePathRelative(const FilePath& parent, void ExtensionPrefs::MakePathsRelative() { bool dirty = false; const DictionaryValue* dict = prefs_->GetMutableDictionary(kExtensionsPref); - if (!dict || dict->GetSize() == 0) + if (!dict || dict->empty()) return; for (DictionaryValue::key_iterator i = dict->begin_keys(); i != dict->end_keys(); ++i) { DictionaryValue* extension_dict; - if (!dict->GetDictionary(*i, &extension_dict)) + if (!dict->GetDictionaryWithoutPathExpansion(*i, &extension_dict)) continue; FilePath::StringType path_string; if (!extension_dict->GetString(kPrefPath, &path_string)) @@ -147,13 +149,13 @@ void ExtensionPrefs::MakePathsRelative() { } void ExtensionPrefs::MakePathsAbsolute(DictionaryValue* dict) { - if (!dict || dict->GetSize() == 0) + if (!dict || dict->empty()) return; for (DictionaryValue::key_iterator i = dict->begin_keys(); i != dict->end_keys(); ++i) { DictionaryValue* extension_dict; - if (!dict->GetDictionary(*i, &extension_dict)) { + if (!dict->GetDictionaryWithoutPathExpansion(*i, &extension_dict)) { NOTREACHED(); continue; } @@ -211,21 +213,21 @@ void ExtensionPrefs::UpdateBlacklist( std::set<std::string> used_id_set; const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref); DCHECK(extensions); - DictionaryValue::key_iterator extension_id = extensions->begin_keys(); - for (; extension_id != extensions->end_keys(); ++extension_id) { + for (DictionaryValue::key_iterator extension_id = extensions->begin_keys(); + extension_id != extensions->end_keys(); ++extension_id) { DictionaryValue* ext; - std::string id = WideToASCII(*extension_id); - if (!extensions->GetDictionary(*extension_id, &ext)) { + if (!extensions->GetDictionaryWithoutPathExpansion(*extension_id, &ext)) { NOTREACHED() << "Invalid pref for extension " << *extension_id; continue; } + std::string id = WideToASCII(*extension_id); if (blacklist_set.find(id) == blacklist_set.end()) { if (!IsBlacklistBitSet(ext)) { // This extension is not in blacklist. And it was not blacklisted // before. continue; } else { - if (ext->GetSize() == 1) { + if (ext->size() == 1) { // We should remove the entry if the only flag here is blacklist. remove_pref_ids.push_back(id); } else { @@ -261,7 +263,7 @@ void ExtensionPrefs::UpdateBlacklist( void ExtensionPrefs::GetKilledExtensionIds(std::set<std::string>* killed_ids) { const DictionaryValue* dict = prefs_->GetDictionary(kExtensionsPref); - if (!dict || dict->GetSize() == 0) + if (!dict || dict->empty()) return; for (DictionaryValue::key_iterator i = dict->begin_keys(); @@ -273,7 +275,7 @@ void ExtensionPrefs::GetKilledExtensionIds(std::set<std::string>* killed_ids) { continue; } - DictionaryValue* extension = NULL; + DictionaryValue* extension; if (!dict->GetDictionary(key_name, &extension)) { NOTREACHED(); continue; @@ -391,7 +393,7 @@ void ExtensionPrefs::MigrateToPrefs(Extension* extension) { FilePath ExtensionPrefs::GetExtensionPath(const std::string& extension_id) { const DictionaryValue* dict = prefs_->GetDictionary(kExtensionsPref); - if (!dict || dict->GetSize() == 0) + if (!dict || dict->empty()) return FilePath(); std::wstring path; @@ -401,16 +403,11 @@ FilePath ExtensionPrefs::GetExtensionPath(const std::string& extension_id) { return install_directory_.Append(FilePath::FromWStringHack(path)); } -bool ExtensionPrefs::UpdateExtensionPref(const std::string& extension_id, +void ExtensionPrefs::UpdateExtensionPref(const std::string& extension_id, const std::wstring& key, Value* data_value) { DictionaryValue* extension = GetOrCreateExtensionPref(extension_id); - if (!extension->Set(key, data_value)) { - NOTREACHED() << "Cannot modify key: '" << key.c_str() - << "' for extension: '" << extension_id.c_str() << "'"; - return false; - } - return true; + extension->Set(key, data_value); } void ExtensionPrefs::DeleteExtensionPrefs(const std::string& extension_id) { diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h index 11c7e31..2bbd239 100644 --- a/chrome/browser/extensions/extension_prefs.h +++ b/chrome/browser/extensions/extension_prefs.h @@ -79,7 +79,7 @@ class ExtensionPrefs { void MakePathsAbsolute(DictionaryValue* dict); // Sets the pref |key| for extension |id| to |value|. - bool UpdateExtensionPref(const std::string& id, + void UpdateExtensionPref(const std::string& id, const std::wstring& key, Value* value); diff --git a/chrome/browser/extensions/extension_uitest.cc b/chrome/browser/extensions/extension_uitest.cc index 2cfcd7c..42e2459 100644 --- a/chrome/browser/extensions/extension_uitest.cc +++ b/chrome/browser/extensions/extension_uitest.cc @@ -261,25 +261,19 @@ class RoundtripAutomationProxy : public MultiMessageAutomationProxy { EXPECT_TRUE(has_callback); DictionaryValue response_dict; - EXPECT_TRUE(response_dict.SetInteger(keys::kAutomationRequestIdKey, - request_id)); + response_dict.SetInteger(keys::kAutomationRequestIdKey, request_id); DictionaryValue tab_dict; - EXPECT_TRUE(tab_dict.SetInteger(extension_tabs_module_constants::kIdKey, - 42)); - EXPECT_TRUE(tab_dict.SetInteger( - extension_tabs_module_constants::kIndexKey, 1)); - EXPECT_TRUE(tab_dict.SetInteger( - extension_tabs_module_constants::kWindowIdKey, 1)); - EXPECT_TRUE(tab_dict.SetBoolean( - extension_tabs_module_constants::kSelectedKey, true)); - EXPECT_TRUE(tab_dict.SetString( - extension_tabs_module_constants::kUrlKey, "http://www.google.com")); + tab_dict.SetInteger(extension_tabs_module_constants::kIdKey, 42); + tab_dict.SetInteger(extension_tabs_module_constants::kIndexKey, 1); + tab_dict.SetInteger(extension_tabs_module_constants::kWindowIdKey, 1); + tab_dict.SetBoolean(extension_tabs_module_constants::kSelectedKey, true); + tab_dict.SetString(extension_tabs_module_constants::kUrlKey, + "http://www.google.com"); std::string tab_json; base::JSONWriter::Write(&tab_dict, false, &tab_json); - EXPECT_TRUE(response_dict.SetString(keys::kAutomationResponseKey, - tab_json)); + response_dict.SetString(keys::kAutomationResponseKey, tab_json); std::string response_json; base::JSONWriter::Write(&response_dict, false, &response_json); diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc index 69e0c86..12b11cf 100644 --- a/chrome/browser/extensions/extensions_service_unittest.cc +++ b/chrome/browser/extensions/extensions_service_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2009 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. @@ -389,7 +389,7 @@ class ExtensionsServiceTest DictionaryValue* dict = prefs_->GetMutableDictionary(L"extensions.settings"); ASSERT_TRUE(dict != NULL); - EXPECT_EQ(count, dict->GetSize()); + EXPECT_EQ(count, dict->size()); } void ValidateBooleanPref(const std::string& extension_id, diff --git a/chrome/browser/extensions/external_pref_extension_provider.cc b/chrome/browser/extensions/external_pref_extension_provider.cc index e0f491f..841f156 100644 --- a/chrome/browser/extensions/external_pref_extension_provider.cc +++ b/chrome/browser/extensions/external_pref_extension_provider.cc @@ -44,10 +44,9 @@ void ExternalPrefExtensionProvider::VisitRegisteredExtension( if (ids_to_ignore.find(WideToASCII(extension_id)) != ids_to_ignore.end()) continue; - DictionaryValue* extension = NULL; - if (!prefs_->GetDictionary(extension_id, &extension)) { + DictionaryValue* extension; + if (!prefs_->GetDictionaryWithoutPathExpansion(extension_id, &extension)) continue; - } FilePath::StringType external_crx; std::string external_version; diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.cc b/chrome/browser/extensions/sandboxed_extension_unpacker.cc index a32304e..46167ad 100644 --- a/chrome/browser/extensions/sandboxed_extension_unpacker.cc +++ b/chrome/browser/extensions/sandboxed_extension_unpacker.cc @@ -336,10 +336,10 @@ bool SandboxedExtensionUnpacker::RewriteImageFiles() { bool SandboxedExtensionUnpacker::RewriteCatalogFiles( const DictionaryValue& catalogs) { // Write our parsed catalogs back to disk. - DictionaryValue::key_iterator key_it = catalogs.begin_keys(); - for (; key_it != catalogs.end_keys(); ++key_it) { + for (DictionaryValue::key_iterator key_it = catalogs.begin_keys(); + key_it != catalogs.end_keys(); ++key_it) { DictionaryValue* catalog; - if (!catalogs.GetDictionary(*key_it, &catalog)) { + if (!catalogs.GetDictionaryWithoutPathExpansion(*key_it, &catalog)) { ReportFailure("Invalid catalog data."); return false; } diff --git a/chrome/browser/metrics/metrics_log.cc b/chrome/browser/metrics/metrics_log.cc index 922b004..fd889d5 100644 --- a/chrome/browser/metrics/metrics_log.cc +++ b/chrome/browser/metrics/metrics_log.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2009 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. @@ -571,7 +571,8 @@ void MetricsLog::WriteAllProfilesMetrics( const std::wstring& key_name = *i; if (key_name.compare(0, profile_prefix.size(), profile_prefix) == 0) { DictionaryValue* profile; - if (all_profiles_metrics.GetDictionary(key_name, &profile)) + if (all_profiles_metrics.GetDictionaryWithoutPathExpansion(key_name, + &profile)) WriteProfileMetrics(key_name.substr(profile_prefix.size()), *profile); } } @@ -584,7 +585,7 @@ void MetricsLog::WriteProfileMetrics(const std::wstring& profileidhash, for (DictionaryValue::key_iterator i = profile_metrics.begin_keys(); i != profile_metrics.end_keys(); ++i) { Value* value; - if (profile_metrics.Get(*i, &value)) { + if (profile_metrics.GetWithoutPathExpansion(*i, &value)) { DCHECK(*i != L"id"); switch (value->GetType()) { case Value::TYPE_STRING: { diff --git a/chrome/browser/sync/sync_setup_wizard_unittest.cc b/chrome/browser/sync/sync_setup_wizard_unittest.cc index 4fa299f..b1ea56b 100644 --- a/chrome/browser/sync/sync_setup_wizard_unittest.cc +++ b/chrome/browser/sync/sync_setup_wizard_unittest.cc @@ -228,7 +228,7 @@ TEST_F(SyncSetupWizardTest, InitialStepLogin) { EXPECT_EQ(SyncSetupWizard::GAIA_LOGIN, test_window_->flow()->current_state_); dialog_args.Clear(); SyncSetupFlow::GetArgsForGaiaLogin(service_, &dialog_args); - EXPECT_TRUE(3 == dialog_args.GetSize()); + EXPECT_EQ(3U, dialog_args.size()); std::string actual_user; dialog_args.GetString(L"user", &actual_user); EXPECT_EQ(kTestUser, actual_user); @@ -243,7 +243,7 @@ TEST_F(SyncSetupWizardTest, InitialStepLogin) { service_->set_auth_state(kTestUser, captcha_error); wizard_->Step(SyncSetupWizard::GAIA_LOGIN); SyncSetupFlow::GetArgsForGaiaLogin(service_, &dialog_args); - EXPECT_TRUE(3 == dialog_args.GetSize()); + EXPECT_EQ(3U, dialog_args.size()); std::string captcha_url; dialog_args.GetString(L"captchaUrl", &captcha_url); EXPECT_EQ(kTestCaptchaUrl, GURL(captcha_url).spec()); @@ -394,7 +394,7 @@ TEST_F(SyncSetupWizardTest, DiscreteRun) { wizard_->Step(SyncSetupWizard::GAIA_LOGIN); EXPECT_TRUE(wizard_->IsVisible()); SyncSetupFlow::GetArgsForGaiaLogin(service_, &dialog_args); - EXPECT_TRUE(3 == dialog_args.GetSize()); + EXPECT_EQ(3U, dialog_args.size()); std::string actual_user; dialog_args.GetString(L"user", &actual_user); EXPECT_EQ(kTestUser, actual_user); diff --git a/chrome/browser/thumbnail_store.cc b/chrome/browser/thumbnail_store.cc index 9c16072..2a937dd 100644 --- a/chrome/browser/thumbnail_store.cc +++ b/chrome/browser/thumbnail_store.cc @@ -218,7 +218,7 @@ void ThumbnailStore::NotifyThumbnailStoreReady() { void ThumbnailStore::UpdateURLData() { DCHECK(url_blacklist_); - int result_count = ThumbnailStore::kMaxCacheSize + url_blacklist_->GetSize(); + int result_count = ThumbnailStore::kMaxCacheSize + url_blacklist_->size(); hs_->QueryTopURLsAndRedirects(result_count, &consumer_, NewCallback(this, &ThumbnailStore::OnURLDataAvailable)); } diff --git a/chrome/browser/webdata/web_database_unittest.cc b/chrome/browser/webdata/web_database_unittest.cc index 4a54d76..beb716d 100644 --- a/chrome/browser/webdata/web_database_unittest.cc +++ b/chrome/browser/webdata/web_database_unittest.cc @@ -57,8 +57,8 @@ class WebDatabaseTest : public testing::Test { DictionaryValue::key_iterator e(a.end_keys()); std::wstring av, bv; while (i != e) { - if (!(a.GetString(*i, &av)) || - !(b.GetString(*i, &bv)) || + if (!(a.GetStringWithoutPathExpansion(*i, &av)) || + !(b.GetStringWithoutPathExpansion(*i, &bv)) || av != bv) return false; diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc index 9b69756..94d6de2 100644 --- a/chrome/common/extensions/extension.cc +++ b/chrome/common/extensions/extension.cc @@ -480,12 +480,10 @@ bool Extension::ContainsNonThemeKeys(const DictionaryValue& source) { // Go through all the root level keys and verify that they're in the map // of keys allowable by themes. If they're not, then make a not of it for // later. - DictionaryValue::key_iterator iter = source.begin_keys(); - while (iter != source.end_keys()) { - std::wstring key = (*iter); - if (theme_keys.find(key) == theme_keys.end()) + for (DictionaryValue::key_iterator iter = source.begin_keys(); + iter != source.end_keys(); ++iter) { + if (theme_keys.find(*iter) == theme_keys.end()) return true; - ++iter; } return false; } @@ -770,14 +768,13 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id, DictionaryValue* images_value; if (theme_value->GetDictionary(keys::kThemeImages, &images_value)) { // Validate that the images are all strings - DictionaryValue::key_iterator iter = images_value->begin_keys(); - while (iter != images_value->end_keys()) { + for (DictionaryValue::key_iterator iter = images_value->begin_keys(); + iter != images_value->end_keys(); ++iter) { std::string val; if (!images_value->GetString(*iter, &val)) { *error = errors::kInvalidThemeImages; return false; } - ++iter; } theme_images_.reset( static_cast<DictionaryValue*>(images_value->DeepCopy())); @@ -785,35 +782,28 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id, DictionaryValue* colors_value; if (theme_value->GetDictionary(keys::kThemeColors, &colors_value)) { - // Validate that the colors are all three-item lists - DictionaryValue::key_iterator iter = colors_value->begin_keys(); - while (iter != colors_value->end_keys()) { - std::string val; - int color = 0; + // Validate that the colors are RGB or RGBA lists + for (DictionaryValue::key_iterator iter = colors_value->begin_keys(); + iter != colors_value->end_keys(); ++iter) { ListValue* color_list; - if (colors_value->GetList(*iter, &color_list)) { - if (color_list->GetSize() == 3 || - color_list->GetSize() == 4) { - if (color_list->GetInteger(0, &color) && - color_list->GetInteger(1, &color) && - color_list->GetInteger(2, &color)) { - if (color_list->GetSize() == 4) { - double alpha; - int alpha_int; - if (color_list->GetReal(3, &alpha) || - color_list->GetInteger(3, &alpha_int)) { - ++iter; - continue; - } - } else { - ++iter; - continue; - } - } - } + double alpha; + int alpha_int; + int color; + // The color must be a list + if (!colors_value->GetListWithoutPathExpansion(*iter, &color_list) || + // And either 3 items (RGB) or 4 (RGBA) + ((color_list->GetSize() != 3) && + ((color_list->GetSize() != 4) || + // For RGBA, the fourth item must be a real or int alpha value + (!color_list->GetReal(3, &alpha) && + !color_list->GetInteger(3, &alpha_int)))) || + // For both RGB and RGBA, the first three items must be ints (R,G,B) + !color_list->GetInteger(0, &color) || + !color_list->GetInteger(1, &color) || + !color_list->GetInteger(2, &color)) { + *error = errors::kInvalidThemeColors; + return false; } - *error = errors::kInvalidThemeColors; - return false; } theme_colors_.reset( static_cast<DictionaryValue*>(colors_value->DeepCopy())); @@ -822,12 +812,12 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id, DictionaryValue* tints_value; if (theme_value->GetDictionary(keys::kThemeTints, &tints_value)) { // Validate that the tints are all reals. - DictionaryValue::key_iterator iter = tints_value->begin_keys(); - while (iter != tints_value->end_keys()) { + for (DictionaryValue::key_iterator iter = tints_value->begin_keys(); + iter != tints_value->end_keys(); ++iter) { ListValue* tint_list; - double v = 0; - int vi = 0; - if (!tints_value->GetList(*iter, &tint_list) || + double v; + int vi; + if (!tints_value->GetListWithoutPathExpansion(*iter, &tint_list) || tint_list->GetSize() != 3 || !(tint_list->GetReal(0, &v) || tint_list->GetInteger(0, &vi)) || !(tint_list->GetReal(1, &v) || tint_list->GetInteger(1, &vi)) || @@ -835,7 +825,6 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id, *error = errors::kInvalidThemeTints; return false; } - ++iter; } theme_tints_.reset( static_cast<DictionaryValue*>(tints_value->DeepCopy())); @@ -1132,24 +1121,20 @@ bool Extension::InitFromValue(const DictionaryValue& source, bool require_id, } // Validate that the overrides are all strings - DictionaryValue::key_iterator iter = overrides->begin_keys(); - while (iter != overrides->end_keys()) { + for (DictionaryValue::key_iterator iter = overrides->begin_keys(); + iter != overrides->end_keys(); ++iter) { std::string page = WideToUTF8(*iter); // For now, only allow the new tab page. Others will work when we remove // this check, but let's keep it simple for now. // TODO(erikkay) enable other pages as well - if (page != chrome::kChromeUINewTabHost) { - *error = errors::kInvalidChromeURLOverrides; - return false; - } std::string val; - if (!overrides->GetString(*iter, &val)) { + if ((page != chrome::kChromeUINewTabHost) || + !overrides->GetStringWithoutPathExpansion(*iter, &val)) { *error = errors::kInvalidChromeURLOverrides; return false; } // Replace the entry with a fully qualified chrome-extension:// URL. chrome_url_overrides_[page] = GetResourceURL(val); - ++iter; } } @@ -1178,9 +1163,8 @@ std::set<FilePath> Extension::GetBrowserImages() { for (DictionaryValue::key_iterator it = theme_images->begin_keys(); it != theme_images->end_keys(); ++it) { std::wstring val; - if (theme_images->GetString(*it, &val)) { + if (theme_images->GetStringWithoutPathExpansion(*it, &val)) image_paths.insert(FilePath::FromWStringHack(val)); - } } } diff --git a/chrome/common/extensions/extension_message_bundle.cc b/chrome/common/extensions/extension_message_bundle.cc index 9e3a7df..a076d44 100644 --- a/chrome/common/extensions/extension_message_bundle.cc +++ b/chrome/common/extensions/extension_message_bundle.cc @@ -50,11 +50,11 @@ bool ExtensionMessageBundle::Init(const CatalogVector& locale_catalogs, std::string* error) { dictionary_.clear(); - CatalogVector::const_reverse_iterator it = locale_catalogs.rbegin(); - for (; it != locale_catalogs.rend(); ++it) { + for (CatalogVector::const_reverse_iterator it = locale_catalogs.rbegin(); + it != locale_catalogs.rend(); ++it) { DictionaryValue* catalog = (*it).get(); - DictionaryValue::key_iterator key_it = catalog->begin_keys(); - for (; key_it != catalog->end_keys(); ++key_it) { + for (DictionaryValue::key_iterator key_it = catalog->begin_keys(); + key_it != catalog->end_keys(); ++key_it) { std::string key(StringToLowerASCII(WideToUTF8(*key_it))); if (!IsValidName(*key_it)) return BadKeyMessage(key, error); @@ -76,7 +76,7 @@ bool ExtensionMessageBundle::GetMessageValue(const std::wstring& wkey, std::string key(WideToUTF8(wkey)); // Get the top level tree for given key (name part). DictionaryValue* name_tree; - if (!catalog.GetDictionary(wkey, &name_tree)) { + if (!catalog.GetDictionaryWithoutPathExpansion(wkey, &name_tree)) { *error = StringPrintf("Not a valid tree for key %s.", key.c_str()); return false; } @@ -117,13 +117,13 @@ bool ExtensionMessageBundle::GetPlaceholders(const DictionaryValue& name_tree, } for (DictionaryValue::key_iterator key_it = placeholders_tree->begin_keys(); - key_it != placeholders_tree->end_keys(); - ++key_it) { + key_it != placeholders_tree->end_keys(); ++key_it) { DictionaryValue* placeholder; std::string content_key = WideToUTF8(*key_it); if (!IsValidName(*key_it)) return BadKeyMessage(content_key, error); - if (!placeholders_tree->GetDictionary(*key_it, &placeholder)) { + if (!placeholders_tree->GetDictionaryWithoutPathExpansion(*key_it, + &placeholder)) { *error = StringPrintf("Invalid placeholder %s for key %s", content_key.c_str(), name_key.c_str()); diff --git a/chrome/common/extensions/extension_unpacker_unittest.cc b/chrome/common/extensions/extension_unpacker_unittest.cc index fe3272e..747827d 100644 --- a/chrome/common/extensions/extension_unpacker_unittest.cc +++ b/chrome/common/extensions/extension_unpacker_unittest.cc @@ -107,12 +107,12 @@ TEST_F(ExtensionUnpackerTest, GoodL10n) { SetupUnpacker("good_l10n.crx"); EXPECT_TRUE(unpacker_->Run()); EXPECT_TRUE(unpacker_->error_message().empty()); - ASSERT_EQ(2U, unpacker_->parsed_catalogs()->GetSize()); + ASSERT_EQ(2U, unpacker_->parsed_catalogs()->size()); } TEST_F(ExtensionUnpackerTest, NoL10n) { SetupUnpacker("no_l10n.crx"); EXPECT_TRUE(unpacker_->Run()); EXPECT_TRUE(unpacker_->error_message().empty()); - EXPECT_EQ(0U, unpacker_->parsed_catalogs()->GetSize()); + EXPECT_EQ(0U, unpacker_->parsed_catalogs()->size()); } diff --git a/chrome/common/pref_service.cc b/chrome/common/pref_service.cc index 691fb77..ac44166 100644 --- a/chrome/common/pref_service.cc +++ b/chrome/common/pref_service.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2009 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. @@ -435,8 +435,7 @@ void PrefService::SetBoolean(const wchar_t* path, bool value) { } scoped_ptr<Value> old_value(GetPrefCopy(path)); - bool rv = persistent_->SetBoolean(path, value); - DCHECK(rv); + persistent_->SetBoolean(path, value); FireObserversIfChanged(path, old_value.get()); } @@ -455,8 +454,7 @@ void PrefService::SetInteger(const wchar_t* path, int value) { } scoped_ptr<Value> old_value(GetPrefCopy(path)); - bool rv = persistent_->SetInteger(path, value); - DCHECK(rv); + persistent_->SetInteger(path, value); FireObserversIfChanged(path, old_value.get()); } @@ -475,8 +473,7 @@ void PrefService::SetReal(const wchar_t* path, double value) { } scoped_ptr<Value> old_value(GetPrefCopy(path)); - bool rv = persistent_->SetReal(path, value); - DCHECK(rv); + persistent_->SetReal(path, value); FireObserversIfChanged(path, old_value.get()); } @@ -495,8 +492,7 @@ void PrefService::SetString(const wchar_t* path, const std::wstring& value) { } scoped_ptr<Value> old_value(GetPrefCopy(path)); - bool rv = persistent_->SetString(path, value); - DCHECK(rv); + persistent_->SetString(path, value); FireObserversIfChanged(path, old_value.get()); } @@ -519,11 +515,10 @@ void PrefService::SetFilePath(const wchar_t* path, const FilePath& value) { // Value::SetString only knows about UTF8 strings, so convert the path from // the system native value to UTF8. std::string path_utf8 = WideToUTF8(base::SysNativeMBToWide(value.value())); - bool rv = persistent_->SetString(path, path_utf8); + persistent_->SetString(path, path_utf8); #else - bool rv = persistent_->SetString(path, value.value()); + persistent_->SetString(path, value.value()); #endif - DCHECK(rv); FireObserversIfChanged(path, old_value.get()); } @@ -542,8 +537,7 @@ void PrefService::SetInt64(const wchar_t* path, int64 value) { } scoped_ptr<Value> old_value(GetPrefCopy(path)); - bool rv = persistent_->SetString(path, Int64ToWString(value)); - DCHECK(rv); + persistent_->SetString(path, Int64ToWString(value)); FireObserversIfChanged(path, old_value.get()); } @@ -585,11 +579,9 @@ DictionaryValue* PrefService::GetMutableDictionary(const wchar_t* path) { } DictionaryValue* dict = NULL; - bool rv = persistent_->GetDictionary(path, &dict); - if (!rv) { + if (!persistent_->GetDictionary(path, &dict)) { dict = new DictionaryValue; - rv = persistent_->Set(path, dict); - DCHECK(rv); + persistent_->Set(path, dict); } return dict; } @@ -608,11 +600,9 @@ ListValue* PrefService::GetMutableList(const wchar_t* path) { } ListValue* list = NULL; - bool rv = persistent_->GetList(path, &list); - if (!rv) { + if (!persistent_->GetList(path, &list)) { list = new ListValue; - rv = persistent_->Set(path, list); - DCHECK(rv); + persistent_->Set(path, list); } return list; } diff --git a/chrome/common/pref_service_unittest.cc b/chrome/common/pref_service_unittest.cc index 29b19c3..bfb96b0 100644 --- a/chrome/common/pref_service_unittest.cc +++ b/chrome/common/pref_service_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2009 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. @@ -117,7 +117,7 @@ TEST_F(PrefServiceTest, Basic) { // Now test that the transient value overrides the persistent value, // without actually altering the persistent store. - EXPECT_TRUE(prefs.transient()->SetString(prefs::kHomePage, microsoft)); + prefs.transient()->SetString(prefs::kHomePage, microsoft); EXPECT_TRUE(prefs.transient()->GetString(prefs::kHomePage, &homepage)); EXPECT_EQ(microsoft, homepage); diff --git a/chrome/installer/util/google_chrome_distribution.cc b/chrome/installer/util/google_chrome_distribution.cc index faf574b..d501701 100644 --- a/chrome/installer/util/google_chrome_distribution.cc +++ b/chrome/installer/util/google_chrome_distribution.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2009 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. // @@ -112,15 +112,15 @@ bool GoogleChromeDistribution::BuildUninstallMetricsString( DCHECK(NULL != metrics); bool has_values = false; - DictionaryValue::key_iterator iter(uninstall_metrics_dict->begin_keys()); - for (; iter != uninstall_metrics_dict->end_keys(); ++iter) { + for (DictionaryValue::key_iterator iter(uninstall_metrics_dict->begin_keys()); + iter != uninstall_metrics_dict->end_keys(); ++iter) { has_values = true; metrics->append(L"&"); metrics->append(*iter); metrics->append(L"="); std::wstring value; - uninstall_metrics_dict->GetString(*iter, &value); + uninstall_metrics_dict->GetStringWithoutPathExpansion(*iter, &value); metrics->append(value); } diff --git a/chrome/installer/util/master_preferences.cc b/chrome/installer/util/master_preferences.cc index 8eb3ad1..176c2fb 100644 --- a/chrome/installer/util/master_preferences.cc +++ b/chrome/installer/util/master_preferences.cc @@ -11,9 +11,7 @@ namespace { const wchar_t* kDistroDict = L"distribution"; - - -} // namespace +} namespace installer_util { namespace master_preferences { @@ -116,14 +114,10 @@ bool SetDistroBooleanPreference(DictionaryValue* prefs, const std::wstring& name, bool value) { - bool ret = false; - if (prefs && !name.empty()) { - std::wstring key(kDistroDict); - key.append(L"." + name); - if (prefs->SetBoolean(key, value)) - ret = true; - } - return ret; + if (!prefs || name.empty()) + return false; + prefs->SetBoolean(std::wstring(kDistroDict) + L"." + name, value); + return true; } } // installer_util diff --git a/chrome/test/ui/javascript_test_util.cc b/chrome/test/ui/javascript_test_util.cc index 17928a2..a2694ae 100644 --- a/chrome/test/ui/javascript_test_util.cc +++ b/chrome/test/ui/javascript_test_util.cc @@ -29,10 +29,10 @@ bool JsonDictionaryToMap(const std::string& json, DictionaryValue* dict = static_cast<DictionaryValue*>(root.get()); - DictionaryValue::key_iterator it = dict->begin_keys(); - for (; it != dict->end_keys(); ++it) { + for (DictionaryValue::key_iterator it = dict->begin_keys(); + it != dict->end_keys(); ++it) { Value* value = NULL; - bool succeeded = dict->Get(*it, &value); + bool succeeded = dict->GetWithoutPathExpansion(*it, &value); EXPECT_TRUE(succeeded); if (!succeeded) diff --git a/ipc/ipc_message_utils.cc b/ipc/ipc_message_utils.cc index acb2452..464dd26 100644 --- a/ipc/ipc_message_utils.cc +++ b/ipc/ipc_message_utils.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2009 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. @@ -61,12 +61,12 @@ static void WriteValue(Message* m, const Value* value, int recursion) { case Value::TYPE_DICTIONARY: { const DictionaryValue* dict = static_cast<const DictionaryValue*>(value); - WriteParam(m, static_cast<int>(dict->GetSize())); + WriteParam(m, static_cast<int>(dict->size())); for (DictionaryValue::key_iterator it = dict->begin_keys(); it != dict->end_keys(); ++it) { Value* subval; - if (dict->Get(*it, &subval)) { + if (dict->GetWithoutPathExpansion(*it, &subval)) { WriteParam(m, *it); WriteValue(m, subval, recursion + 1); } else { diff --git a/net/base/strict_transport_security_state.cc b/net/base/strict_transport_security_state.cc index 609241c..29f892f 100644 --- a/net/base/strict_transport_security_state.cc +++ b/net/base/strict_transport_security_state.cc @@ -245,10 +245,10 @@ bool StrictTransportSecurityState::Deserialise(const std::string& input) { DictionaryValue* dict_value = reinterpret_cast<DictionaryValue*>(value.get()); const base::Time current_time(base::Time::Now()); - for (DictionaryValue::key_iterator - i = dict_value->begin_keys(); i != dict_value->end_keys(); ++i) { + for (DictionaryValue::key_iterator i = dict_value->begin_keys(); + i != dict_value->end_keys(); ++i) { DictionaryValue* state; - if (!dict_value->GetDictionary(*i, &state)) + if (!dict_value->GetDictionaryWithoutPathExpansion(*i, &state)) continue; bool include_subdomains; |