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

8 files changed, 37 insertions, 47 deletions

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;
     }