Skip to content

Commit

Permalink
[CodeHealth] Removes DictionaryValue::Iterator
Browse files Browse the repository at this point in the history 
Bug: 1187056
Change-Id: I2c56e6f9076c4c9579137f523c7ed53b81f2fdbb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3868805
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Reviewed-by: Jonathan Metzman <metzman@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1045000}
  • Loading branch information
yero authored and Chromium LUCI CQ committed Sep 9, 2022
1 parent 7cf9b9d commit af27b4f
Show file tree
Hide file tree
Showing 43 changed files with 242 additions and 344 deletions.
24 changes: 13 additions & 11 deletions apps/saved_files_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,29 +113,31 @@ std::vector<SavedFileEntry> GetSavedFileEntries(
ExtensionPrefs* prefs,
const std::string& extension_id) {
std::vector<SavedFileEntry> result;
const base::DictionaryValue* file_entries = NULL;
if (!prefs->ReadPrefAsDictionary(extension_id, kFileEntries, &file_entries))

const auto* dict = prefs->ReadPrefAsDict(extension_id, kFileEntries);
if (!dict) {
return result;
}

for (base::DictionaryValue::Iterator it(*file_entries); !it.IsAtEnd();
it.Advance()) {
const base::DictionaryValue* file_entry = NULL;
if (!it.value().GetAsDictionary(&file_entry))
for (const auto item : *dict) {
const auto* file_entry = item.second.GetIfDict();
if (!file_entry)
continue;
const base::Value* path_value;
if (!file_entry->Get(kFileEntryPath, &path_value))

const base::Value* path_value = file_entry->Find(kFileEntryPath);
if (!path_value)
continue;
absl::optional<base::FilePath> file_path =
base::ValueToFilePath(*path_value);
if (!file_path)
continue;
bool is_directory =
file_entry->FindBoolPath(kFileEntryIsDirectory).value_or(false);
file_entry->FindBool(kFileEntryIsDirectory).value_or(false);
const absl::optional<int> sequence_number =
file_entry->GetDict().FindInt(kFileEntrySequenceNumber);
file_entry->FindInt(kFileEntrySequenceNumber);
if (!sequence_number || sequence_number.value() == 0)
continue;
result.emplace_back(it.key(), *file_path, is_directory,
result.emplace_back(item.first, *file_path, is_directory,
sequence_number.value());
}
return result;
Expand Down
7 changes: 0 additions & 7 deletions base/values.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1782,13 +1782,6 @@ void DictionaryValue::Swap(DictionaryValue* other) {
dict().swap(other->dict());
}

DictionaryValue::Iterator::Iterator(const DictionaryValue& target)
: target_(target), it_(target.DictItems().begin()) {}

DictionaryValue::Iterator::Iterator(const Iterator& other) = default;

DictionaryValue::Iterator::~Iterator() = default;

std::unique_ptr<DictionaryValue> DictionaryValue::CreateDeepCopy() const {
return std::make_unique<DictionaryValue>(dict());
}
Expand Down
22 changes: 0 additions & 22 deletions base/values.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1399,28 +1399,6 @@ class BASE_EXPORT DictionaryValue : public Value {
// Swaps contents with the `other` dictionary.
void Swap(DictionaryValue* other);

// This class provides an iterator over both keys and values in the
// dictionary. It can't be used to modify the dictionary.
//
// DEPRECATED: Use a range-based for loop over `base::Value::Dict` directly
// instead.
class BASE_EXPORT Iterator {
public:
explicit Iterator(const DictionaryValue& target);
Iterator(const Iterator& other);
~Iterator();

bool IsAtEnd() const { return it_ == target_.DictItems().end(); }
void Advance() { ++it_; }

const std::string& key() const { return it_->first; }
const Value& value() const { return it_->second; }

private:
const DictionaryValue& target_;
detail::const_dict_iterator it_;
};

// DEPRECATED, use `Value::Dict::Clone()` instead.
// TODO(crbug.com/646113): Delete this and migrate callsites.
std::unique_ptr<DictionaryValue> CreateDeepCopy() const;
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ash/policy/core/device_local_account_extension_tracker.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ void DeviceLocalAccountExtensionTracker::UpdateFromStore() {
return;
}

for (base::DictionaryValue::Iterator it(*dict); !it.IsAtEnd(); it.Advance()) {
PolicyNamespace ns(POLICY_DOMAIN_EXTENSIONS, it.key());
for (const auto item : dict->GetDict()) {
PolicyNamespace ns(POLICY_DOMAIN_EXTENSIONS, item.first);
if (schema_registry_->schema_map()->GetSchema(ns)) {
// Important detail: Don't register the component again if it already
// has a schema! If the session already started for this public session
Expand Down
9 changes: 4 additions & 5 deletions chrome/browser/chromeos/extensions/external_cache_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,19 +102,18 @@ void ExternalCacheImpl::UpdateExtensionsList(
}

void ExternalCacheImpl::OnDamagedFileDetected(const base::FilePath& path) {
for (base::DictionaryValue::Iterator it(*cached_extensions_.get());
!it.IsAtEnd(); it.Advance()) {
for (const auto item : cached_extensions_->GetDict()) {
const base::DictionaryValue* entry = nullptr;
if (!it.value().GetAsDictionary(&entry)) {
if (!item.second.GetAsDictionary(&entry)) {
NOTREACHED() << "ExternalCacheImpl found bad entry with type "
<< it.value().type();
<< item.second.type();
continue;
}

const std::string* external_crx = entry->GetDict().FindString(
extensions::ExternalProviderImpl::kExternalCrx);
if (external_crx && *external_crx == path.value()) {
extensions::ExtensionId id = it.key();
extensions::ExtensionId id = item.first;
LOG(ERROR) << "ExternalCacheImpl extension at " << path.value()
<< " failed to install, deleting it.";
RemoveCachedExtension(id);
Expand Down
7 changes: 3 additions & 4 deletions chrome/browser/extensions/api/downloads/downloads_api.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -621,12 +621,11 @@ void RunDownloadQuery(const downloads::DownloadQuery& query_in,
}

std::unique_ptr<base::DictionaryValue> query_in_value(query_in.ToValue());
for (base::DictionaryValue::Iterator query_json_field(*query_in_value);
!query_json_field.IsAtEnd(); query_json_field.Advance()) {
for (const auto query_json_field : query_in_value->GetDict()) {
FilterTypeMap::const_iterator filter_type =
filter_types.Get().find(query_json_field.key());
filter_types.Get().find(query_json_field.first);
if (filter_type != filter_types.Get().end()) {
if (!query_out.AddFilter(filter_type->second, query_json_field.value())) {
if (!query_out.AddFilter(filter_type->second, query_json_field.second)) {
*error = download_extension_errors::kInvalidFilter;
return;
}
Expand Down
13 changes: 6 additions & 7 deletions chrome/browser/extensions/api/terminal/terminal_private_api.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -784,17 +784,16 @@ ExtensionFunction::ResponseAction TerminalPrivateSetPrefsFunction::Run() {
kAllowList{{{guest_os::prefs::kGuestOsTerminalSettings,
base::Value::Type::DICTIONARY}}};

for (base::DictionaryValue::Iterator it(params->prefs.additional_properties);
!it.IsAtEnd(); it.Advance()) {
for (const auto item : params->prefs.additional_properties.GetDict()) {
// Write prefs if they are allowed, and match expected type, else ignore.
auto allow_it = kAllowList->find(it.key());
auto allow_it = kAllowList->find(item.first);
if (allow_it == kAllowList->end() ||
allow_it->second != it.value().type()) {
LOG(WARNING) << "Ignoring non-allowed SetPrefs path=" << it.key()
<< ", type=" << it.value().type();
allow_it->second != item.second.type()) {
LOG(WARNING) << "Ignoring non-allowed SetPrefs path=" << item.first
<< ", type=" << item.second.type();
continue;
}
service->Set(it.key(), it.value());
service->Set(item.first, item.second);
}
return RespondNow(NoArguments());
}
Expand Down
7 changes: 3 additions & 4 deletions chrome/browser/profile_resetter/brandcoded_default_settings.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,9 @@ bool BrandcodedDefaultSettings::GetExtensions(
if (master_dictionary_ &&
master_dictionary_->GetDictionary(
installer::initial_preferences::kExtensionsBlock, &extensions)) {
for (base::DictionaryValue::Iterator extension_id(*extensions);
!extension_id.IsAtEnd(); extension_id.Advance()) {
if (crx_file::id_util::IdIsValid(extension_id.key()))
extension_ids->push_back(extension_id.key());
for (const auto extension_id : extensions->GetDict()) {
if (crx_file::id_util::IdIsValid(extension_id.first))
extension_ids->push_back(extension_id.first);
}
return true;
}
Expand Down
16 changes: 7 additions & 9 deletions chrome/browser/safe_browsing/incident_reporting/platform_state_store.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,16 @@ void KeysAndDigestsToProtobuf(
const base::DictionaryValue& keys_and_digests,
RepeatedPtrField<StateStoreData::Incidents::KeyDigestMapFieldEntry>*
key_digest_pairs) {
for (base::DictionaryValue::Iterator iter(keys_and_digests); !iter.IsAtEnd();
iter.Advance()) {
for (const auto item : keys_and_digests.GetDict()) {
uint32_t digest = 0;
if (!iter.value().is_string() ||
!base::StringToUint(iter.value().GetString(), &digest)) {
if (!item.second.is_string() ||
!base::StringToUint(item.second.GetString(), &digest)) {
NOTREACHED();
continue;
}
StateStoreData::Incidents::KeyDigestMapFieldEntry* key_digest =
key_digest_pairs->Add();
key_digest->set_key(iter.key());
key_digest->set_key(item.first);
key_digest->set_digest(digest);
}
}
Expand All @@ -65,17 +64,16 @@ void IncidentsSentToProtobuf(
const base::DictionaryValue& incidents_sent,
RepeatedPtrField<StateStoreData::TypeIncidentsMapFieldEntry>*
type_incidents_pairs) {
for (base::DictionaryValue::Iterator iter(incidents_sent); !iter.IsAtEnd();
iter.Advance()) {
for (const auto item : incidents_sent.GetDict()) {
const base::DictionaryValue* keys_and_digests = nullptr;
if (!iter.value().GetAsDictionary(&keys_and_digests)) {
if (!item.second.GetAsDictionary(&keys_and_digests)) {
NOTREACHED();
continue;
}
if (keys_and_digests->DictEmpty())
continue;
int incident_type = 0;
if (!base::StringToInt(iter.key(), &incident_type)) {
if (!base::StringToInt(item.first, &incident_type)) {
NOTREACHED();
continue;
}
Expand Down
7 changes: 3 additions & 4 deletions chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,8 @@ SettingsResetPromptConfig::ParseDomainHashes(
// Each key in the hash should be a 64-byte long string and each
// integer ID should fit in an int.
domain_hashes_.clear();
for (base::DictionaryValue::Iterator iter(*domains_dict); !iter.IsAtEnd();
iter.Advance()) {
const std::string& hash_string = iter.key();
for (const auto item : domains_dict->GetDict()) {
const std::string& hash_string = item.first;
if (hash_string.size() != crypto::kSHA256Length * 2)
return CONFIG_ERROR_BAD_DOMAIN_HASH;

Expand All @@ -221,7 +220,7 @@ SettingsResetPromptConfig::ParseDomainHashes(
return CONFIG_ERROR_BAD_DOMAIN_HASH;

// Convert the ID string to an integer.
const std::string* domain_id_string = iter.value().GetIfString();
const std::string* domain_id_string = item.second.GetIfString();
int domain_id = -1;
if (!domain_id_string ||
!base::StringToInt(*domain_id_string, &domain_id) || domain_id < 0) {
Expand Down
20 changes: 9 additions & 11 deletions chrome/browser/supervised_user/supervised_user_settings_service_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,12 +225,12 @@ TEST_F(SupervisedUserSettingsServiceTest, ProcessSplitSetting) {

settings_.reset();
syncer::SyncChangeList change_list;
for (base::DictionaryValue::Iterator it(dict); !it.IsAtEnd(); it.Advance()) {
for (const auto item : dict.GetDict()) {
syncer::SyncData data =
SupervisedUserSettingsService::CreateSyncDataForSetting(
SupervisedUserSettingsService::MakeSplitSettingKey(kSettingsName,
it.key()),
it.value());
item.first),
item.second);
change_list.push_back(
syncer::SyncChange(FROM_HERE, syncer::SyncChange::ACTION_ADD, data));
}
Expand Down Expand Up @@ -309,13 +309,12 @@ TEST_F(SupervisedUserSettingsServiceTest, Merge) {
base::DictionaryValue dict;
dict.SetStringKey("foo", "bar");
dict.SetIntKey("eaudecologne", 4711);
for (base::DictionaryValue::Iterator it(dict); !it.IsAtEnd();
it.Advance()) {
for (const auto item : dict.GetDict()) {
sync_data.push_back(
SupervisedUserSettingsService::CreateSyncDataForSetting(
SupervisedUserSettingsService::MakeSplitSettingKey(kSplitItemName,
it.key()),
it.value()));
item.first),
item.second));
}
StartSyncing(sync_data);
EXPECT_EQ(3u,
Expand All @@ -338,13 +337,12 @@ TEST_F(SupervisedUserSettingsServiceTest, Merge) {
dict.SetStringKey("foo", "burp");
dict.SetStringKey("item", "first");
// Adding 2 SplitSettings from dictionary.
for (base::DictionaryValue::Iterator it(dict); !it.IsAtEnd();
it.Advance()) {
for (const auto item : dict.GetDict()) {
sync_data.push_back(
SupervisedUserSettingsService::CreateSyncDataForSetting(
SupervisedUserSettingsService::MakeSplitSettingKey(kSplitItemName,
it.key()),
it.value()));
item.first),
item.second));
}
StartSyncing(sync_data);
EXPECT_EQ(4u,
Expand Down
60 changes: 26 additions & 34 deletions chrome/browser/themes/browser_theme_pack.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1277,12 +1277,11 @@ void BrowserThemePack::SetTintsFromJSON(

// Parse the incoming data from |tints_value| into an intermediary structure.
std::map<int, color_utils::HSL> temp_tints;
for (base::DictionaryValue::Iterator iter(*tints_value); !iter.IsAtEnd();
iter.Advance()) {
if (!iter.value().is_list())
for (const auto item : tints_value->GetDict()) {
if (!item.second.is_list())
continue;

base::Value::ConstListView tint_list = iter.value().GetListDeprecated();
base::Value::ConstListView tint_list = item.second.GetListDeprecated();
if (tint_list.size() != 3)
continue;

Expand All @@ -1295,7 +1294,7 @@ void BrowserThemePack::SetTintsFromJSON(
color_utils::HSL hsl = {*h, *s, *l};
MakeHSLShiftValid(&hsl);

int id = GetIntForString(iter.key(), kTintTable, kTintTableLength);
int id = GetIntForString(item.first, kTintTable, kTintTableLength);
if (id != -1)
temp_tints[id] = hsl;
}
Expand Down Expand Up @@ -1398,28 +1397,27 @@ void BrowserThemePack::SetDisplayPropertiesFromJSON(
return;

std::map<int, int> temp_properties;
for (base::DictionaryValue::Iterator iter(*display_properties_value);
!iter.IsAtEnd(); iter.Advance()) {
int property_id = GetIntForString(iter.key(), kDisplayProperties,
kDisplayPropertiesSize);
for (const auto item : display_properties_value->GetDict()) {
int property_id =
GetIntForString(item.first, kDisplayProperties, kDisplayPropertiesSize);
switch (property_id) {
case TP::NTP_BACKGROUND_ALIGNMENT: {
if (iter.value().is_string()) {
if (item.second.is_string()) {
temp_properties[TP::NTP_BACKGROUND_ALIGNMENT] =
TP::StringToAlignment(iter.value().GetString());
TP::StringToAlignment(item.second.GetString());
}
break;
}
case TP::NTP_BACKGROUND_TILING: {
if (iter.value().is_string()) {
if (item.second.is_string()) {
temp_properties[TP::NTP_BACKGROUND_TILING] =
TP::StringToTiling(iter.value().GetString());
TP::StringToTiling(item.second.GetString());
}
break;
}
case TP::NTP_LOGO_ALTERNATE: {
if (iter.value().is_int())
temp_properties[TP::NTP_LOGO_ALTERNATE] = iter.value().GetInt();
if (item.second.is_int())
temp_properties[TP::NTP_LOGO_ALTERNATE] = item.second.GetInt();
break;
}
}
Expand All @@ -1442,27 +1440,21 @@ void BrowserThemePack::ParseImageNamesFromJSON(
if (!images_value)
return;

for (base::DictionaryValue::Iterator iter(*images_value); !iter.IsAtEnd();
iter.Advance()) {
if (iter.value().is_dict()) {
const base::DictionaryValue* inner_value = nullptr;
if (iter.value().GetAsDictionary(&inner_value)) {
for (base::DictionaryValue::Iterator inner_iter(*inner_value);
!inner_iter.IsAtEnd();
inner_iter.Advance()) {
ui::ResourceScaleFactor scale_factor = ui::kScaleFactorNone;
if (GetScaleFactorFromManifestKey(inner_iter.key(), &scale_factor) &&
inner_iter.value().is_string()) {
AddFileAtScaleToMap(
iter.key(), scale_factor,
images_path.AppendASCII(inner_iter.value().GetString()),
file_paths);
}
for (const auto item : images_value->GetDict()) {
if (item.second.is_dict()) {
for (const auto inner_item : item.second.GetDict()) {
ui::ResourceScaleFactor scale_factor = ui::kScaleFactorNone;
if (GetScaleFactorFromManifestKey(inner_item.first, &scale_factor) &&
inner_item.second.is_string()) {
AddFileAtScaleToMap(
item.first, scale_factor,
images_path.AppendASCII(inner_item.second.GetString()),
file_paths);
}
}
} else if (iter.value().is_string()) {
AddFileAtScaleToMap(iter.key(), ui::k100Percent,
images_path.AppendASCII(iter.value().GetString()),
} else if (item.second.is_string()) {
AddFileAtScaleToMap(item.first, ui::k100Percent,
images_path.AppendASCII(item.second.GetString()),
file_paths);
}
}
Expand Down
Loading

0 comments on commit af27b4f

Please sign in to comment.