Replaced DictionaryValue::key_iterator by DictionaryValue::Iterator o…

…utside of chrome/browser.

Marked Iterator::HasNext() as deprecated and added a method Iterator::CanAdvance() as replacement.

As DictionaryValue::Iterator is actually a const iterator, I had to add several missing const annotations, mostly, in json_schema_validator.* and command.*

BUG=162611
TEST=No new tests. Only semantically equivalent refactorings.

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@177673 0039d316-1c4b-4281-b951-d872f2087c98

base/debug/trace_event_unittest.cc

@@ -242,17 +242,12 @@ bool TraceEventTestFixture::FindNonMatchingValue(const char* key,
 }
 
 bool IsStringInDict(const char* string_to_match, const DictionaryValue* dict) {
-  for (DictionaryValue::key_iterator ikey = dict->begin_keys();
-       ikey != dict->end_keys(); ++ikey) {
-    const Value* child = NULL;
-    if (!dict->GetWithoutPathExpansion(*ikey, &child))
-      continue;
-
-    if ((*ikey).find(string_to_match) != std::string::npos)
+  for (DictionaryValue::Iterator it(*dict); !it.IsAtEnd(); it.Advance()) {
+    if (it.key().find(string_to_match) != std::string::npos)
       return true;
 
     std::string value_str;
-    child->GetAsString(&value_str);
+    it.value().GetAsString(&value_str);
     if (value_str.find(string_to_match) != std::string::npos)
       return true;
   }

base/json/json_writer.cc

@@ -170,32 +170,29 @@ void JSONWriter::BuildJSONString(const Value* const node, int depth) {
 
         const DictionaryValue* dict =
           static_cast<const DictionaryValue*>(node);
-        for (DictionaryValue::key_iterator key_itr = dict->begin_keys();
-             key_itr != dict->end_keys();
-             ++key_itr) {
-          const Value* value = NULL;
-          bool result = dict->GetWithoutPathExpansion(*key_itr, &value);
-          DCHECK(result);
-
-          if (omit_binary_values_ && value->GetType() == Value::TYPE_BINARY) {
+        bool first_entry = true;
+        for (DictionaryValue::Iterator itr(*dict); !itr.IsAtEnd();
+             itr.Advance(), first_entry = false) {
+          if (omit_binary_values_ &&
+              itr.value().GetType() == Value::TYPE_BINARY) {
             continue;
           }
 
-          if (key_itr != dict->begin_keys()) {
+          if (!first_entry) {
             json_string_->append(",");
             if (pretty_print_)
               json_string_->append(kPrettyPrintLineEnding);
           }
 
           if (pretty_print_)
             IndentLine(depth + 1);
-          AppendQuotedString(*key_itr);
+          AppendQuotedString(itr.key());
           if (pretty_print_) {
             json_string_->append(": ");
           } else {
             json_string_->append(":");
           }
-          BuildJSONString(value, depth + 1);
+          BuildJSONString(&itr.value(), depth + 1);
         }
 
         if (pretty_print_) {

base/test/trace_event_analyzer.cc

@@ -79,27 +79,24 @@ bool TraceEvent::SetFromJSON(const base::Value* event_value) {
   }
 
   // For each argument, copy the type and create a trace_analyzer::TraceValue.
-  base::DictionaryValue::key_iterator keyi = args->begin_keys();
-  for (; keyi != args->end_keys(); ++keyi) {
+  for (base::DictionaryValue::Iterator it(*args); !it.IsAtEnd();
+       it.Advance()) {
     std::string str;
     bool boolean = false;
     int int_num = 0;
     double double_num = 0.0;
-    const Value* value = NULL;
-    if (args->GetWithoutPathExpansion(*keyi, &value)) {
-      if (value->GetAsString(&str))
-        arg_strings[*keyi] = str;
-      else if (value->GetAsInteger(&int_num))
-        arg_numbers[*keyi] = static_cast<double>(int_num);
-      else if (value->GetAsBoolean(&boolean))
-        arg_numbers[*keyi] = static_cast<double>(boolean ? 1 : 0);
-      else if (value->GetAsDouble(&double_num))
-        arg_numbers[*keyi] = double_num;
-      else {
-        LOG(ERROR) << "Value type of argument is not supported: " <<
-                      static_cast<int>(value->GetType());
-        return false;  // Invalid trace event JSON format.
-      }
+    if (it.value().GetAsString(&str))
+      arg_strings[it.key()] = str;
+    else if (it.value().GetAsInteger(&int_num))
+      arg_numbers[it.key()] = static_cast<double>(int_num);
+    else if (it.value().GetAsBoolean(&boolean))
+      arg_numbers[it.key()] = static_cast<double>(boolean ? 1 : 0);
+    else if (it.value().GetAsDouble(&double_num))
+      arg_numbers[it.key()] = double_num;
+    else {
+      LOG(ERROR) << "Value type of argument is not supported: " <<
+          static_cast<int>(it.value().GetType());
+      return false;  // Invalid trace event JSON format.
     }
   }
 

base/values.cc

@@ -18,11 +18,11 @@ namespace {
 // Make a deep copy of |node|, but don't include empty lists or dictionaries
 // in the copy. It's possible for this function to return NULL and it
 // expects |node| to always be non-NULL.
-Value* CopyWithoutEmptyChildren(Value* node) {
+Value* CopyWithoutEmptyChildren(const Value* node) {
   DCHECK(node);
   switch (node->GetType()) {
     case Value::TYPE_LIST: {
-      ListValue* list = static_cast<ListValue*>(node);
+      const ListValue* list = static_cast<const ListValue*>(node);
       ListValue* copy = new ListValue;
       for (ListValue::const_iterator it = list->begin(); it != list->end();
            ++it) {
@@ -38,16 +38,12 @@ Value* CopyWithoutEmptyChildren(Value* node) {
     }
 
     case Value::TYPE_DICTIONARY: {
-      DictionaryValue* dict = static_cast<DictionaryValue*>(node);
+      const DictionaryValue* dict = static_cast<const DictionaryValue*>(node);
       DictionaryValue* copy = new DictionaryValue;
-      for (DictionaryValue::key_iterator it = dict->begin_keys();
-           it != dict->end_keys(); ++it) {
-        Value* child = NULL;
-        bool rv = dict->GetWithoutPathExpansion(*it, &child);
-        DCHECK(rv);
-        Value* child_copy = CopyWithoutEmptyChildren(child);
+      for (DictionaryValue::Iterator it(*dict); !it.IsAtEnd(); it.Advance()) {
+        Value* child_copy = CopyWithoutEmptyChildren(&it.value());
         if (child_copy)
-          copy->SetWithoutPathExpansion(*it, child_copy);
+          copy->SetWithoutPathExpansion(it.key(), child_copy);
       }
       if (!copy->empty())
         return copy;

base/values.h

@@ -369,7 +369,10 @@ class BASE_EXPORT DictionaryValue : public Value {
    public:
     explicit Iterator(const DictionaryValue& target);
 
+    // DEPRECATED: use !IsAtEnd() instead.
     bool HasNext() const { return it_ != target_.dictionary_.end(); }
+
+    bool IsAtEnd() const { return it_ == target_.dictionary_.end(); }
     void Advance() { ++it_; }
 
     const std::string& key() const { return it_->first; }

chrome/common/extensions/command.cc

@@ -202,7 +202,7 @@ ui::Accelerator Command::StringToAccelerator(const std::string& accelerator) {
   return parsed;
 }
 
-bool Command::Parse(DictionaryValue* command,
+bool Command::Parse(const DictionaryValue* command,
                     const std::string& command_name,
                     int index,
                     string16* error) {
@@ -213,16 +213,16 @@ bool Command::Parse(DictionaryValue* command,
   SuggestionMap suggestions;
 
   // First try to parse the |suggested_key| as a dictionary.
-  DictionaryValue* suggested_key_dict;
+  const DictionaryValue* suggested_key_dict;
   if (command->GetDictionary(keys::kSuggestedKey, &suggested_key_dict)) {
-    DictionaryValue::key_iterator iter = suggested_key_dict->begin_keys();
-    for ( ; iter != suggested_key_dict->end_keys(); ++iter) {
+    for (DictionaryValue::Iterator iter(*suggested_key_dict); !iter.IsAtEnd();
+         iter.Advance()) {
       // For each item in the dictionary, extract the platforms specified.
       std::string suggested_key_string;
-      if (suggested_key_dict->GetString(*iter, &suggested_key_string) &&
+      if (iter.value().GetAsString(&suggested_key_string) &&
           !suggested_key_string.empty()) {
         // Found a platform, add it to the suggestions list.
-        suggestions[*iter] = suggested_key_string;
+        suggestions[iter.key()] = suggested_key_string;
       } else {
         *error = ErrorUtils::FormatErrorMessageUTF16(
             errors::kInvalidKeyBinding,

chrome/common/extensions/command.h

@@ -37,7 +37,7 @@ class Command {
   static ui::Accelerator StringToAccelerator(const std::string& accelerator);
 
   // Parse the command.
-  bool Parse(base::DictionaryValue* command,
+  bool Parse(const base::DictionaryValue* command,
              const std::string& command_name,
              int index,
              string16* error);

chrome/common/extensions/extension.cc

@@ -899,10 +899,10 @@ std::set<FilePath> Extension::GetBrowserImages() const {
   // Theme images.
   DictionaryValue* theme_images = GetThemeImages();
   if (theme_images) {
-    for (DictionaryValue::key_iterator it = theme_images->begin_keys();
-         it != theme_images->end_keys(); ++it) {
+    for (DictionaryValue::Iterator it(*theme_images); !it.IsAtEnd();
+         it.Advance()) {
       std::string val;
-      if (theme_images->GetStringWithoutPathExpansion(*it, &val))
+      if (it.value().GetAsString(&val))
         image_paths.insert(FilePath::FromWStringHack(UTF8ToWide(val)));
     }
   }
@@ -2003,20 +2003,20 @@ bool Extension::LoadCommands(string16* error) {
     }
 
     int command_index = 0;
-    for (DictionaryValue::key_iterator iter = commands->begin_keys();
-         iter != commands->end_keys(); ++iter) {
+    for (DictionaryValue::Iterator iter(*commands); !iter.IsAtEnd();
+         iter.Advance()) {
       ++command_index;
 
-      DictionaryValue* command = NULL;
-      if (!commands->GetDictionary(*iter, &command)) {
+      const DictionaryValue* command = NULL;
+      if (!iter.value().GetAsDictionary(&command)) {
         *error = ErrorUtils::FormatErrorMessageUTF16(
             errors::kInvalidKeyBindingDictionary,
             base::IntToString(command_index));
         return false;
       }
 
       scoped_ptr<extensions::Command> binding(new extensions::Command());
-      if (!binding->Parse(command, *iter, command_index, error))
+      if (!binding->Parse(command, iter.key(), command_index, error))
         return false;  // |error| already set.
 
       std::string command_name = binding->command_name();
@@ -2206,45 +2206,43 @@ bool Extension::LoadRequirements(string16* error) {
     return false;
   }
 
-  for (DictionaryValue::key_iterator it = requirements_value->begin_keys();
-       it != requirements_value->end_keys(); ++it) {
-    DictionaryValue* requirement_value;
-    if (!requirements_value->GetDictionaryWithoutPathExpansion(
-        *it, &requirement_value)) {
+  for (DictionaryValue::Iterator it(*requirements_value); !it.IsAtEnd();
+       it.Advance()) {
+    const DictionaryValue* requirement_value;
+    if (!it.value().GetAsDictionary(&requirement_value)) {
       *error = ErrorUtils::FormatErrorMessageUTF16(
-          errors::kInvalidRequirement, *it);
+          errors::kInvalidRequirement, it.key());
       return false;
     }
 
-    if (*it == "plugins") {
-      for (DictionaryValue::key_iterator plugin_it =
-              requirement_value->begin_keys();
-           plugin_it != requirement_value->end_keys(); ++plugin_it) {
+    if (it.key() == "plugins") {
+      for (DictionaryValue::Iterator plugin_it(*requirement_value);
+           !plugin_it.IsAtEnd(); plugin_it.Advance()) {
         bool plugin_required = false;
-        if (!requirement_value->GetBoolean(*plugin_it, &plugin_required)) {
+        if (!plugin_it.value().GetAsBoolean(&plugin_required)) {
           *error = ErrorUtils::FormatErrorMessageUTF16(
-              errors::kInvalidRequirement, *it);
+              errors::kInvalidRequirement, it.key());
           return false;
         }
-        if (*plugin_it == "npapi") {
+        if (plugin_it.key() == "npapi") {
           requirements_.npapi = plugin_required;
         } else {
           *error = ErrorUtils::FormatErrorMessageUTF16(
-              errors::kInvalidRequirement, *it);
+              errors::kInvalidRequirement, it.key());
           return false;
         }
       }
-    } else if (*it == "3D") {
-      ListValue* features = NULL;
+    } else if (it.key() == "3D") {
+      const ListValue* features = NULL;
       if (!requirement_value->GetListWithoutPathExpansion("features",
                                                           &features) ||
           !features) {
         *error = ErrorUtils::FormatErrorMessageUTF16(
-            errors::kInvalidRequirement, *it);
+            errors::kInvalidRequirement, it.key());
         return false;
       }
 
-      for (base::ListValue::iterator feature_it = features->begin();
+      for (base::ListValue::const_iterator feature_it = features->begin();
            feature_it != features->end();
            ++feature_it) {
         std::string feature;
@@ -2255,7 +2253,7 @@ bool Extension::LoadRequirements(string16* error) {
             requirements_.css3d = true;
           } else {
             *error = ErrorUtils::FormatErrorMessageUTF16(
-                errors::kInvalidRequirement, *it);
+                errors::kInvalidRequirement, it.key());
             return false;
           }
         }
@@ -2657,10 +2655,10 @@ bool Extension::LoadThemeImages(const DictionaryValue* theme_value,
   const DictionaryValue* images_value = NULL;
   if (theme_value->GetDictionary(keys::kThemeImages, &images_value)) {
     // Validate that the images are all strings
-    for (DictionaryValue::key_iterator iter = images_value->begin_keys();
-         iter != images_value->end_keys(); ++iter) {
+    for (DictionaryValue::Iterator iter(*images_value); !iter.IsAtEnd();
+         iter.Advance()) {
       std::string val;
-      if (!images_value->GetString(*iter, &val)) {
+      if (!iter.value().GetAsString(&val)) {
         *error = ASCIIToUTF16(errors::kInvalidThemeImages);
         return false;
       }
@@ -2675,13 +2673,13 @@ bool Extension::LoadThemeColors(const DictionaryValue* theme_value,
   const DictionaryValue* colors_value = NULL;
   if (theme_value->GetDictionary(keys::kThemeColors, &colors_value)) {
     // Validate that the colors are RGB or RGBA lists
-    for (DictionaryValue::key_iterator iter = colors_value->begin_keys();
-         iter != colors_value->end_keys(); ++iter) {
+    for (DictionaryValue::Iterator iter(*colors_value); !iter.IsAtEnd();
+         iter.Advance()) {
       const ListValue* color_list = NULL;
       double alpha = 0.0;
       int color = 0;
       // The color must be a list
-      if (!colors_value->GetListWithoutPathExpansion(*iter, &color_list) ||
+      if (!iter.value().GetAsList(&color_list) ||
           // And either 3 items (RGB) or 4 (RGBA)
           ((color_list->GetSize() != 3) &&
            ((color_list->GetSize() != 4) ||
@@ -2708,11 +2706,11 @@ bool Extension::LoadThemeTints(const DictionaryValue* theme_value,
     return true;
 
   // Validate that the tints are all reals.
-  for (DictionaryValue::key_iterator iter = tints_value->begin_keys();
-       iter != tints_value->end_keys(); ++iter) {
+  for (DictionaryValue::Iterator iter(*tints_value); !iter.IsAtEnd();
+       iter.Advance()) {
     const ListValue* tint_list = NULL;
     double v = 0.0;
-    if (!tints_value->GetListWithoutPathExpansion(*iter, &tint_list) ||
+    if (!iter.value().GetAsList(&tint_list) ||
         tint_list->GetSize() != 3 ||
         !tint_list->GetDouble(0, &v) ||
         !tint_list->GetDouble(1, &v) ||

chrome/common/extensions/extension_file_util.cc

@@ -263,10 +263,10 @@ bool ValidateExtension(const Extension* extension,
   if (extension->is_theme()) {
     DictionaryValue* images_value = extension->GetThemeImages();
     if (images_value) {
-      for (DictionaryValue::key_iterator iter = images_value->begin_keys();
-           iter != images_value->end_keys(); ++iter) {
+      for (DictionaryValue::Iterator iter(*images_value); !iter.IsAtEnd();
+           iter.Advance()) {
         std::string val;
-        if (images_value->GetStringWithoutPathExpansion(*iter, &val)) {
+        if (iter.value().GetAsString(&val)) {
           FilePath image_path = extension->path().Append(
               FilePath::FromUTF8Unsafe(val));
           if (!file_util::PathExists(image_path)) {