Removing base::DictionaryValue::key_iterator which was displaced by DictionaryValue::Iterator.

BUG=162611
TBR=kkania@chromium.org (for webdriver), kalman@chromium.org (for extensions)

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

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

10 files changed, 69 insertions, 123 deletions

diff --git a/base/values.cc b/base/values.cc
index 4768774..316463fb 100644
--- a/base/values.cc
+++ b/base/values.cc
@@ -756,22 +756,19 @@ DictionaryValue* DictionaryValue::DeepCopyWithoutEmptyChildren() {
 }
 
 void DictionaryValue::MergeDictionary(const DictionaryValue* dictionary) {
-  for (DictionaryValue::key_iterator key(dictionary->begin_keys());
-       key != dictionary->end_keys(); ++key) {
-    const Value* merge_value;
-    if (dictionary->GetWithoutPathExpansion(*key, &merge_value)) {
-      // Check whether we have to merge dictionaries.
-      if (merge_value->IsType(Value::TYPE_DICTIONARY)) {
-        DictionaryValue* sub_dict;
-        if (GetDictionaryWithoutPathExpansion(*key, &sub_dict)) {
-          sub_dict->MergeDictionary(
-              static_cast<const DictionaryValue*>(merge_value));
-          continue;
-        }
+  for (DictionaryValue::Iterator it(*dictionary); !it.IsAtEnd(); it.Advance()) {
+    const Value* merge_value = &it.value();
+    // Check whether we have to merge dictionaries.
+    if (merge_value->IsType(Value::TYPE_DICTIONARY)) {
+      DictionaryValue* sub_dict;
+      if (GetDictionaryWithoutPathExpansion(it.key(), &sub_dict)) {
+        sub_dict->MergeDictionary(
+            static_cast<const DictionaryValue*>(merge_value));
+        continue;
       }
-      // All other cases: Make a copy and hook it up.
-      SetWithoutPathExpansion(*key, merge_value->DeepCopy());
     }
+    // All other cases: Make a copy and hook it up.
+    SetWithoutPathExpansion(it.key(), merge_value->DeepCopy());
   }
 }
 
@@ -779,14 +776,6 @@ void DictionaryValue::Swap(DictionaryValue* other) {
   dictionary_.swap(other->dictionary_);
 }
 
-DictionaryValue::key_iterator::key_iterator(ValueMap::const_iterator itr) {
-  itr_ = itr;
-}
-
-DictionaryValue::key_iterator::key_iterator(const key_iterator& rhs) {
-  itr_ = rhs.itr_;
-}
-
 DictionaryValue::Iterator::Iterator(const DictionaryValue& target)
     : target_(target),
       it_(target.dictionary_.begin()) {}
@@ -809,21 +798,17 @@ bool DictionaryValue::Equals(const Value* other) const {
 
   const DictionaryValue* other_dict =
       static_cast<const DictionaryValue*>(other);
-  key_iterator lhs_it(begin_keys());
-  key_iterator rhs_it(other_dict->begin_keys());
-  while (lhs_it != end_keys() && rhs_it != other_dict->end_keys()) {
-    const Value* lhs;
-    const Value* rhs;
-    if (*lhs_it != *rhs_it ||
-        !GetWithoutPathExpansion(*lhs_it, &lhs) ||
-        !other_dict->GetWithoutPathExpansion(*rhs_it, &rhs) ||
-        !lhs->Equals(rhs)) {
+  Iterator lhs_it(*this);
+  Iterator rhs_it(*other_dict);
+  while (!lhs_it.IsAtEnd() && !rhs_it.IsAtEnd()) {
+    if (lhs_it.key() != rhs_it.key() ||
+        !lhs_it.value().Equals(&rhs_it.value())) {
       return false;
     }
-    ++lhs_it;
-    ++rhs_it;
+    lhs_it.Advance();
+    rhs_it.Advance();
   }
-  if (lhs_it != end_keys() || rhs_it != other_dict->end_keys())
+  if (!lhs_it.IsAtEnd() || !rhs_it.IsAtEnd())
     return false;
 
   return true;
diff --git a/base/values.h b/base/values.h
index 8c87844..ce2f77f 100644
--- a/base/values.h
+++ b/base/values.h
@@ -336,33 +336,6 @@ class BASE_EXPORT DictionaryValue : public Value {
   // Swaps contents with the |other| dictionary.
   virtual void Swap(DictionaryValue* other);
 
-  // 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 BASE_EXPORT key_iterator
-      : private std::iterator<std::input_iterator_tag, const std::string> {
-   public:
-    explicit key_iterator(ValueMap::const_iterator itr);
-    // Not explicit, because this is a copy constructor.
-    key_iterator(const key_iterator& rhs);
-    key_iterator operator++() {
-      ++itr_;
-      return *this;
-    }
-    const std::string& operator*() { return itr_->first; }
-    bool operator!=(const key_iterator& other) { return itr_ != other.itr_; }
-    bool operator==(const key_iterator& other) { return itr_ == other.itr_; }
-
-   private:
-    ValueMap::const_iterator itr_;
-  };
-
-  key_iterator begin_keys() const { return key_iterator(dictionary_.begin()); }
-  key_iterator end_keys() const { return key_iterator(dictionary_.end()); }
-
   // This class provides an iterator over both keys and values in the
   // dictionary.  It can't be used to modify the dictionary.
   class BASE_EXPORT Iterator {
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 0d560fe..02b405e 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -1140,11 +1140,10 @@ void ExtensionPrefs::GetSavedFileEntries(
   const DictionaryValue* file_entries = NULL;
   if (!prefs->GetDictionary(kFileEntries, &file_entries))
     return;
-  for (DictionaryValue::key_iterator it = file_entries->begin_keys();
-       it != file_entries->end_keys(); ++it) {
-    std::string id = *it;
+  for (DictionaryValue::Iterator it(*file_entries); !it.IsAtEnd();
+       it.Advance()) {
     const DictionaryValue* file_entry = NULL;
-    if (!file_entries->GetDictionaryWithoutPathExpansion(id, &file_entry))
+    if (!it.value().GetAsDictionary(&file_entry))
       continue;
     base::FilePath::StringType path_string;
     if (!file_entry->GetString(kFileEntryPath, &path_string))
@@ -1154,7 +1153,7 @@ void ExtensionPrefs::GetSavedFileEntries(
       continue;
     base::FilePath file_path(path_string);
     out->push_back(app_file_handler_util::SavedFileEntry(
-        id, file_path, writable));
+        it.key(), file_path, writable));
   }
 }
 
@@ -2010,7 +2009,7 @@ void ExtensionPrefs::LoadExtensionControlledPrefs(
   if (!source_dict->GetDictionary(key, &preferences))
     return;
 
-  for (DictionaryValue::Iterator i(*preferences); i.HasNext(); i.Advance()) {
+  for (DictionaryValue::Iterator i(*preferences); !i.IsAtEnd(); i.Advance()) {
     extension_pref_value_map_->SetExtensionPref(
         extension_id, i.key(), scope, i.value().DeepCopy());
   }
diff --git a/chrome/browser/net/http_server_properties_manager.cc b/chrome/browser/net/http_server_properties_manager.cc
index 7796e6c..0f2de16 100644
--- a/chrome/browser/net/http_server_properties_manager.cc
+++ b/chrome/browser/net/http_server_properties_manager.cc
@@ -281,11 +281,10 @@ void HttpServerPropertiesManager::UpdateCacheFromPrefsOnUI() {
   scoped_ptr<net::AlternateProtocolMap> alternate_protocol_map(
       new net::AlternateProtocolMap);
 
-  for (base::DictionaryValue::key_iterator it = servers_dict->begin_keys();
-       it != servers_dict->end_keys();
-       ++it) {
+  for (base::DictionaryValue::Iterator it(*servers_dict); !it.IsAtEnd();
+       it.Advance()) {
     // Get server's host/pair.
-    const std::string& server_str = *it;
+    const std::string& server_str = it.key();
     net::HostPortPair server = net::HostPortPair::FromString(server_str);
     if (server.host().empty()) {
       DVLOG(1) << "Malformed http_server_properties for server: " << server_str;
@@ -294,8 +293,7 @@ void HttpServerPropertiesManager::UpdateCacheFromPrefsOnUI() {
     }
 
     const base::DictionaryValue* server_pref_dict = NULL;
-    if (!servers_dict->GetDictionaryWithoutPathExpansion(
-        server_str, &server_pref_dict)) {
+    if (!it.value().GetAsDictionary(&server_pref_dict)) {
       DVLOG(1) << "Malformed http_server_properties server: " << server_str;
       detected_corrupted_prefs = true;
       continue;
@@ -315,10 +313,9 @@ void HttpServerPropertiesManager::UpdateCacheFromPrefsOnUI() {
       if (server_pref_dict->GetDictionaryWithoutPathExpansion(
           "settings", &spdy_settings_dict)) {
         net::SettingsMap settings_map;
-        for (base::DictionaryValue::key_iterator dict_it =
-             spdy_settings_dict->begin_keys();
-             dict_it != spdy_settings_dict->end_keys(); ++dict_it) {
-          const std::string& id_str = *dict_it;
+        for (base::DictionaryValue::Iterator dict_it(*spdy_settings_dict);
+             !dict_it.IsAtEnd(); dict_it.Advance()) {
+          const std::string& id_str = dict_it.key();
           int id = 0;
           if (!base::StringToInt(id_str, &id)) {
             DVLOG(1) << "Malformed id in SpdySettings for server: " <<
@@ -327,8 +324,7 @@ void HttpServerPropertiesManager::UpdateCacheFromPrefsOnUI() {
             continue;
           }
           int value = 0;
-          if (!spdy_settings_dict->GetIntegerWithoutPathExpansion(id_str,
-                                                                  &value)) {
+          if (!dict_it.value().GetAsInteger(&value)) {
             DVLOG(1) << "Malformed value in SpdySettings for server: " <<
                 server_str;
             NOTREACHED();
diff --git a/chrome/browser/net/transport_security_persister.cc b/chrome/browser/net/transport_security_persister.cc
index e241d24..4d1cf40 100644
--- a/chrome/browser/net/transport_security_persister.cc
+++ b/chrome/browser/net/transport_security_persister.cc
@@ -226,18 +226,17 @@ bool TransportSecurityPersister::Deserialize(const std::string& serialized,
                                              bool* dirty,
                                              TransportSecurityState* state) {
   scoped_ptr<Value> value(base::JSONReader::Read(serialized));
-  DictionaryValue* dict_value;
+  DictionaryValue* dict_value = NULL;
   if (!value.get() || !value->GetAsDictionary(&dict_value))
     return false;
 
   const base::Time current_time(base::Time::Now());
   bool dirtied = false;
 
-  for (DictionaryValue::key_iterator i = dict_value->begin_keys();
-       i != dict_value->end_keys(); ++i) {
-    DictionaryValue* parsed;
-    if (!dict_value->GetDictionaryWithoutPathExpansion(*i, &parsed)) {
-      LOG(WARNING) << "Could not parse entry " << *i << "; skipping entry";
+  for (DictionaryValue::Iterator i(*dict_value); !i.IsAtEnd(); i.Advance()) {
+    const DictionaryValue* parsed = NULL;
+    if (!i.value().GetAsDictionary(&parsed)) {
+      LOG(WARNING) << "Could not parse entry " << i.key() << "; skipping entry";
       continue;
     }
 
@@ -251,7 +250,7 @@ bool TransportSecurityPersister::Deserialize(const std::string& serialized,
                             &domain_state.include_subdomains) ||
         !parsed->GetString(kMode, &mode_string) ||
         !parsed->GetDouble(kExpiry, &expiry)) {
-      LOG(WARNING) << "Could not parse some elements of entry " << *i
+      LOG(WARNING) << "Could not parse some elements of entry " << i.key()
                    << "; skipping entry";
       continue;
     }
@@ -260,7 +259,7 @@ bool TransportSecurityPersister::Deserialize(const std::string& serialized,
     parsed->GetDouble(kDynamicSPKIHashesExpiry,
                       &dynamic_spki_hashes_expiry);
 
-    ListValue* pins_list = NULL;
+    const ListValue* pins_list = NULL;
     // preloaded_spki_hashes is a legacy synonym for static_spki_hashes.
     if (parsed->GetList(kStaticSPKIHashes, &pins_list))
       SPKIHashesFromListValue(*pins_list, &domain_state.static_spki_hashes);
@@ -278,7 +277,7 @@ bool TransportSecurityPersister::Deserialize(const std::string& serialized,
           TransportSecurityState::DomainState::MODE_DEFAULT;
     } else {
       LOG(WARNING) << "Unknown TransportSecurityState mode string "
-                   << mode_string << " found for entry " << *i
+                   << mode_string << " found for entry " << i.key()
                    << "; skipping entry";
       continue;
     }
@@ -302,7 +301,7 @@ bool TransportSecurityPersister::Deserialize(const std::string& serialized,
       continue;
     }
 
-    std::string hashed = ExternalStringToHashedDomain(*i);
+    std::string hashed = ExternalStringToHashedDomain(i.key());
     if (hashed.empty()) {
       dirtied = true;
       continue;
diff --git a/chrome/browser/policy/component_cloud_policy_store.cc b/chrome/browser/policy/component_cloud_policy_store.cc
index e027832..98637a6 100644
--- a/chrome/browser/policy/component_cloud_policy_store.cc
+++ b/chrome/browser/policy/component_cloud_policy_store.cc
@@ -315,19 +315,18 @@ bool ComponentCloudPolicyStore::ParsePolicy(const std::string& data,
   // Each description is an object that contains the policy value under the
   // "Value" key. The optional "Level" key is either "Mandatory" (default) or
   // "Recommended".
-  for (base::DictionaryValue::key_iterator it = dict->begin_keys();
-       it != dict->end_keys(); ++it) {
+  for (base::DictionaryValue::Iterator it(*dict); !it.IsAtEnd(); it.Advance()) {
     base::DictionaryValue* description = NULL;
-    if (!dict->GetDictionary(*it, &description))
+    if (!dict->GetDictionaryWithoutPathExpansion(it.key(), &description))
       return false;
 
     base::Value* value = NULL;
-    if (!description->Remove(kValue, &value))
+    if (!description->RemoveWithoutPathExpansion(kValue, &value))
       return false;
 
     PolicyLevel level = POLICY_LEVEL_MANDATORY;
     std::string level_string;
-    if (description->GetString(kLevel, &level_string) &&
+    if (description->GetStringWithoutPathExpansion(kLevel, &level_string) &&
         level_string == kRecommended) {
       level = POLICY_LEVEL_RECOMMENDED;
     }
@@ -335,7 +334,7 @@ bool ComponentCloudPolicyStore::ParsePolicy(const std::string& data,
     // If policy for components is ever used for device-level settings then
     // this must support a configurable scope; assuming POLICY_SCOPE_USER is
     // fine for now.
-    policy->Set(*it, level, POLICY_SCOPE_USER, value);
+    policy->Set(it.key(), level, POLICY_SCOPE_USER, value);
   }
 
   return true;
diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc
index 2b1020d..df78ae67 100644
--- a/chrome/browser/profiles/profile_impl.cc
+++ b/chrome/browser/profiles/profile_impl.cc
@@ -577,13 +577,12 @@ void ProfileImpl::InitHostZoomMap() {
       prefs_->GetDictionary(prefs::kPerHostZoomLevels);
   // Careful: The returned value could be NULL if the pref has never been set.
   if (host_zoom_dictionary != NULL) {
-    for (DictionaryValue::key_iterator i(host_zoom_dictionary->begin_keys());
-         i != host_zoom_dictionary->end_keys(); ++i) {
-      const std::string& host(*i);
+    for (DictionaryValue::Iterator i(*host_zoom_dictionary); !i.IsAtEnd();
+         i.Advance()) {
+      const std::string& host(i.key());
       double zoom_level = 0;
 
-      bool success = host_zoom_dictionary->GetDoubleWithoutPathExpansion(
-          host, &zoom_level);
+      bool success = i.value().GetAsDouble(&zoom_level);
       DCHECK(success);
       host_zoom_map->SetZoomLevel(host, zoom_level);
     }
diff --git a/chrome/browser/profiles/profile_info_cache.cc b/chrome/browser/profiles/profile_info_cache.cc
index bccb8e6..8770f10 100644
--- a/chrome/browser/profiles/profile_info_cache.cc
+++ b/chrome/browser/profiles/profile_info_cache.cc
@@ -176,14 +176,12 @@ ProfileInfoCache::ProfileInfoCache(PrefService* prefs,
   // Populate the cache
   const DictionaryValue* cache =
       prefs_->GetDictionary(prefs::kProfileInfoCache);
-  for (DictionaryValue::key_iterator it = cache->begin_keys();
-       it != cache->end_keys(); ++it) {
-    std::string key = *it;
+  for (DictionaryValue::Iterator it(*cache); !it.IsAtEnd(); it.Advance()) {
     const DictionaryValue* info = NULL;
-    cache->GetDictionary(key, &info);
+    it.value().GetAsDictionary(&info);
     string16 name;
     info->GetString(kNameKey, &name);
-    sorted_keys_.insert(FindPositionForProfile(key, name), key);
+    sorted_keys_.insert(FindPositionForProfile(it.key(), name), it.key());
   }
 }
 
@@ -813,11 +811,10 @@ std::vector<string16> ProfileInfoCache::GetProfileNames() {
   const DictionaryValue* cache = local_state->GetDictionary(
       prefs::kProfileInfoCache);
   string16 name;
-  for (base::DictionaryValue::key_iterator it = cache->begin_keys();
-       it != cache->end_keys();
-       ++it) {
+  for (base::DictionaryValue::Iterator it(*cache); !it.IsAtEnd();
+       it.Advance()) {
     const base::DictionaryValue* info = NULL;
-    cache->GetDictionary(*it, &info);
+    it.value().GetAsDictionary(&info);
     info->GetString(kNameKey, &name);
     names.push_back(name);
   }
diff --git a/chrome/test/webdriver/webdriver_capabilities_parser.cc b/chrome/test/webdriver/webdriver_capabilities_parser.cc
index 606c598..ccca4d0 100644
--- a/chrome/test/webdriver/webdriver_capabilities_parser.cc
+++ b/chrome/test/webdriver/webdriver_capabilities_parser.cc
@@ -296,10 +296,11 @@ Error* CapabilitiesParser::ParseProxy(const base::Value* option) {
   proxy_options.insert("sslProxy");
   proxy_options.insert("class");  // Created by BeanToJSONConverter.
 
-  DictionaryValue::key_iterator key_iter = options->begin_keys();
-  for (; key_iter != options->end_keys(); ++key_iter) {
-    if (proxy_options.find(*key_iter) == proxy_options.end()) {
-      logger_.Log(kInfoLogLevel, "Unrecognized proxy capability: " + *key_iter);
+  for (DictionaryValue::Iterator iter(*options); !iter.IsAtEnd();
+       iter.Advance()) {
+    if (proxy_options.find(iter.key()) == proxy_options.end()) {
+      logger_.Log(kInfoLogLevel,
+                  "Unrecognized proxy capability: " + iter.key());
     }
   }
 
diff --git a/chrome/test/webdriver/webdriver_util.cc b/chrome/test/webdriver/webdriver_util.cc
index b17abb2..568e842 100644
--- a/chrome/test/webdriver/webdriver_util.cc
+++ b/chrome/test/webdriver/webdriver_util.cc
@@ -409,19 +409,17 @@ void TruncateString(std::string* data) {
 
 // Truncates all strings contained in the given value.
 void TruncateContainedStrings(Value* value) {
-  ListValue* list;
-  if (value->IsType(Value::TYPE_DICTIONARY)) {
-    DictionaryValue* dict = static_cast<DictionaryValue*>(value);
-    DictionaryValue::key_iterator key = dict->begin_keys();
-    for (; key != dict->end_keys(); ++key) {
-      Value* child;
-      if (!dict->GetWithoutPathExpansion(*key, &child))
-        continue;
+  ListValue* list = NULL;
+  DictionaryValue* dict = NULL;
+  if (value->GetAsDictionary(&dict)) {
+    for (DictionaryValue::Iterator it(*dict); !it.IsAtEnd(); it.Advance()) {
       std::string data;
-      if (child->GetAsString(&data)) {
+      if (it.value().GetAsString(&data)) {
         TruncateString(&data);
-        dict->SetWithoutPathExpansion(*key, new base::StringValue(data));
+        dict->SetWithoutPathExpansion(it.key(), new base::StringValue(data));
       } else {
+        Value* child = NULL;
+        dict->GetWithoutPathExpansion(it.key(), &child);
         TruncateContainedStrings(child);
       }
     }