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

1 files changed, 35 insertions, 51 deletions

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