Skip to content

Commit

Permalink
CodeHealth: Remove use of DictionaryValue::Set
Browse files Browse the repository at this point in the history
This change removes the use of DictionaryValue::Set and replaces with
Value::SetKey() or Value::SetPath().

Bug: 1187023
Change-Id: Ia4c1062c321c2ded7d903422ffe9f74612d2e816
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2943857
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Daichi Hirono <hirono@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: manuk hovanesian <manukh@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Auto-Submit: Fangzhen Song <songfangzhen@bytedance.com>
Commit-Queue: Fangzhen Song <songfangzhen@bytedance.com>
Cr-Commit-Position: refs/heads/master@{#896805}
  • Loading branch information
song-fangzhen authored and Chromium LUCI CQ committed Jun 29, 2021
1 parent dbaf529 commit 6ff811d
Show file tree
Hide file tree
Showing 47 changed files with 487 additions and 472 deletions.
97 changes: 46 additions & 51 deletions base/values_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1666,24 +1666,20 @@ TEST(ValuesTest, DictionaryRemovePath) {

TEST(ValuesTest, DeepCopy) {
DictionaryValue original_dict;
Value* null_weak = original_dict.Set("null", std::make_unique<Value>());
Value* bool_weak = original_dict.Set("bool", std::make_unique<Value>(true));
Value* int_weak = original_dict.Set("int", std::make_unique<Value>(42));
Value* double_weak =
original_dict.Set("double", std::make_unique<Value>(3.14));
Value* string_weak =
original_dict.Set("string", std::make_unique<Value>("hello"));
Value* string16_weak =
original_dict.Set("string16", std::make_unique<Value>(u"hello16"));

Value* binary_weak = original_dict.Set(
"binary", std::make_unique<Value>(Value::BlobStorage(42, '!')));
Value* null_weak = original_dict.SetKey("null", Value());
Value* bool_weak = original_dict.SetKey("bool", Value(true));
Value* int_weak = original_dict.SetKey("int", Value(42));
Value* double_weak = original_dict.SetKey("double", Value(3.14));
Value* string_weak = original_dict.SetKey("string", Value("hello"));
Value* string16_weak = original_dict.SetKey("string16", Value(u"hello16"));

Value* binary_weak =
original_dict.SetKey("binary", Value(Value::BlobStorage(42, '!')));

Value::ListStorage storage;
storage.emplace_back(0);
storage.emplace_back(1);
Value* list_weak =
original_dict.Set("list", std::make_unique<Value>(std::move(storage)));
Value* list_weak = original_dict.SetKey("list", Value(std::move(storage)));
Value* list_element_0_weak = &list_weak->GetList()[0];
Value* list_element_1_weak = &list_weak->GetList()[1];

Expand Down Expand Up @@ -1810,19 +1806,19 @@ TEST(ValuesTest, Equals) {
dv.SetDoubleKey("c", 2.5);
dv.SetStringKey("d1", "string");
dv.SetStringKey("d2", u"http://google.com");
dv.Set("e", std::make_unique<Value>());
dv.SetKey("e", Value());

auto copy = dv.CreateDeepCopy();
EXPECT_EQ(dv, *copy);

std::unique_ptr<ListValue> list(new ListValue);
list->Append(std::make_unique<Value>());
list->Append(std::make_unique<DictionaryValue>());
auto list_copy = std::make_unique<Value>(list->Clone());
Value list_copy(list->Clone());

ListValue* list_weak = dv.SetList("f", std::move(list));
EXPECT_NE(dv, *copy);
copy->Set("f", std::move(list_copy));
copy->SetKey("f", std::move(list_copy));
EXPECT_EQ(dv, *copy);

list_weak->Append(std::make_unique<Value>(true));
Expand Down Expand Up @@ -2003,15 +1999,15 @@ TEST(ValuesTest, DeepCopyCovariantReturnTypes) {
TEST(ValuesTest, RemoveEmptyChildren) {
auto root = std::make_unique<DictionaryValue>();
// Remove empty lists and dictionaries.
root->Set("empty_dict", std::make_unique<DictionaryValue>());
root->Set("empty_list", std::make_unique<ListValue>());
root->SetKey("a.b.c.d.e", DictionaryValue());
root->SetKey("empty_dict", DictionaryValue());
root->SetKey("empty_list", ListValue());
root->SetPath("a.b.c.d.e", DictionaryValue());
root = root->DeepCopyWithoutEmptyChildren();
EXPECT_TRUE(root->DictEmpty());

// Make sure we don't prune too much.
root->SetBoolKey("bool", true);
root->Set("empty_dict", std::make_unique<DictionaryValue>());
root->SetKey("empty_dict", DictionaryValue());
root->SetStringKey("empty_string", std::string());
root = root->DeepCopyWithoutEmptyChildren();
EXPECT_EQ(2U, root->DictSize());
Expand All @@ -2023,49 +2019,49 @@ TEST(ValuesTest, RemoveEmptyChildren) {
// Nested test cases. These should all reduce back to the bool and string
// set above.
{
root->Set("a.b.c.d.e", std::make_unique<DictionaryValue>());
root->SetPath("a.b.c.d.e", DictionaryValue());
root = root->DeepCopyWithoutEmptyChildren();
EXPECT_EQ(2U, root->DictSize());
}
{
auto inner = std::make_unique<DictionaryValue>();
inner->Set("empty_dict", std::make_unique<DictionaryValue>());
inner->Set("empty_list", std::make_unique<ListValue>());
root->Set("dict_with_empty_children", std::move(inner));
Value inner(Value::Type::DICTIONARY);
inner.SetKey("empty_dict", DictionaryValue());
inner.SetKey("empty_list", ListValue());
root->SetKey("dict_with_empty_children", std::move(inner));
root = root->DeepCopyWithoutEmptyChildren();
EXPECT_EQ(2U, root->DictSize());
}
{
auto inner = std::make_unique<ListValue>();
inner->Append(std::make_unique<DictionaryValue>());
inner->Append(std::make_unique<ListValue>());
root->Set("list_with_empty_children", std::move(inner));
ListValue inner;
inner.Append(std::make_unique<DictionaryValue>());
inner.Append(std::make_unique<ListValue>());
root->SetKey("list_with_empty_children", std::move(inner));
root = root->DeepCopyWithoutEmptyChildren();
EXPECT_EQ(2U, root->DictSize());
}

// Nested with siblings.
{
auto inner = std::make_unique<ListValue>();
inner->Append(std::make_unique<DictionaryValue>());
inner->Append(std::make_unique<ListValue>());
root->Set("list_with_empty_children", std::move(inner));
auto inner2 = std::make_unique<DictionaryValue>();
inner2->Set("empty_dict", std::make_unique<DictionaryValue>());
inner2->Set("empty_list", std::make_unique<ListValue>());
root->Set("dict_with_empty_children", std::move(inner2));
ListValue inner;
inner.Append(std::make_unique<DictionaryValue>());
inner.Append(std::make_unique<ListValue>());
root->SetKey("list_with_empty_children", std::move(inner));
DictionaryValue inner2;
inner2.SetKey("empty_dict", DictionaryValue());
inner2.SetKey("empty_list", ListValue());
root->SetKey("dict_with_empty_children", std::move(inner2));
root = root->DeepCopyWithoutEmptyChildren();
EXPECT_EQ(2U, root->DictSize());
}

// Make sure nested values don't get pruned.
{
auto inner = std::make_unique<ListValue>();
ListValue inner;
auto inner2 = std::make_unique<ListValue>();
inner2->Append(std::make_unique<Value>("hello"));
inner->Append(std::make_unique<DictionaryValue>());
inner->Append(std::move(inner2));
root->Set("list_with_empty_children", std::move(inner));
inner.Append(std::make_unique<DictionaryValue>());
inner.Append(std::move(inner2));
root->SetKey("list_with_empty_children", std::move(inner));
root = root->DeepCopyWithoutEmptyChildren();
EXPECT_EQ(3U, root->DictSize());

Expand All @@ -2082,19 +2078,18 @@ TEST(ValuesTest, MergeDictionary) {
std::unique_ptr<DictionaryValue> base(new DictionaryValue);
base->SetStringKey("base_key", "base_key_value_base");
base->SetStringKey("collide_key", "collide_key_value_base");
std::unique_ptr<DictionaryValue> base_sub_dict(new DictionaryValue);
base_sub_dict->SetStringKey("sub_base_key", "sub_base_key_value_base");
base_sub_dict->SetStringKey("sub_collide_key", "sub_collide_key_value_base");
base->Set("sub_dict_key", std::move(base_sub_dict));
DictionaryValue base_sub_dict;
base_sub_dict.SetStringKey("sub_base_key", "sub_base_key_value_base");
base_sub_dict.SetStringKey("sub_collide_key", "sub_collide_key_value_base");
base->SetKey("sub_dict_key", std::move(base_sub_dict));

std::unique_ptr<DictionaryValue> merge(new DictionaryValue);
merge->SetStringKey("merge_key", "merge_key_value_merge");
merge->SetStringKey("collide_key", "collide_key_value_merge");
std::unique_ptr<DictionaryValue> merge_sub_dict(new DictionaryValue);
merge_sub_dict->SetStringKey("sub_merge_key", "sub_merge_key_value_merge");
merge_sub_dict->SetStringKey("sub_collide_key",
"sub_collide_key_value_merge");
merge->Set("sub_dict_key", std::move(merge_sub_dict));
DictionaryValue merge_sub_dict;
merge_sub_dict.SetStringKey("sub_merge_key", "sub_merge_key_value_merge");
merge_sub_dict.SetStringKey("sub_collide_key", "sub_collide_key_value_merge");
merge->SetKey("sub_dict_key", std::move(merge_sub_dict));

base->MergeDictionary(merge.get());

Expand Down
6 changes: 3 additions & 3 deletions chrome/renderer/chrome_content_renderer_client_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ scoped_refptr<const extensions::Extension> CreateTestExtension(
manifest.SetString("version", "1");
manifest.SetInteger("manifest_version", 2);
if (is_hosted_app) {
auto url_list = std::make_unique<base::ListValue>();
url_list->AppendString(app_url);
manifest.Set(extensions::manifest_keys::kWebURLs, std::move(url_list));
base::ListValue url_list;
url_list.AppendString(app_url);
manifest.SetPath(extensions::manifest_keys::kWebURLs, std::move(url_list));
manifest.SetString(extensions::manifest_keys::kLaunchWebURL, app_url);
}
std::string error;
Expand Down
7 changes: 3 additions & 4 deletions components/domain_reliability/beacon.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,9 @@ std::unique_ptr<Value> DomainReliabilityBeacon::ToValue(
if (!quic_error.empty())
beacon_value->SetString("quic_error", quic_error);
if (chrome_error != net::OK) {
auto failure_value = std::make_unique<DictionaryValue>();
failure_value->SetString("custom_error",
net::ErrorToString(chrome_error));
beacon_value->Set("failure_data", std::move(failure_value));
DictionaryValue failure_value;
failure_value.SetString("custom_error", net::ErrorToString(chrome_error));
beacon_value->SetKey("failure_data", std::move(failure_value));
}
beacon_value->SetString("server_ip", server_ip);
beacon_value->SetBoolean("was_proxied", was_proxied);
Expand Down
14 changes: 7 additions & 7 deletions components/domain_reliability/context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ std::unique_ptr<Value> DomainReliabilityContext::GetWebUIData() const {
context_value->SetInteger("beacon_count", static_cast<int>(beacons_.size()));
context_value->SetInteger("uploading_beacon_count",
static_cast<int>(uploading_beacons_size_));
context_value->Set("scheduler", scheduler_.GetWebUIData());
context_value->SetKey(
"scheduler", base::Value::FromUniquePtrValue(scheduler_.GetWebUIData()));

return std::unique_ptr<Value>(context_value);
}
Expand Down Expand Up @@ -202,18 +203,17 @@ std::unique_ptr<const Value> DomainReliabilityContext::CreateReport(

int max_upload_depth = 0;

std::unique_ptr<ListValue> beacons_value(new ListValue());
ListValue beacons_value;
for (const auto& beacon : beacons_) {
// Only include beacons with a matching NetworkIsolationKey in the report.
if (beacon->network_isolation_key !=
uploading_beacons_network_isolation_key_) {
continue;
}

beacons_value->Append(beacon->ToValue(upload_time,
*last_network_change_time_,
collector_url,
config().path_prefixes));
beacons_value.Append(
beacon->ToValue(upload_time, *last_network_change_time_, collector_url,
config().path_prefixes));
if (beacon->upload_depth > max_upload_depth)
max_upload_depth = beacon->upload_depth;
++uploading_beacons_size_;
Expand All @@ -223,7 +223,7 @@ std::unique_ptr<const Value> DomainReliabilityContext::CreateReport(

std::unique_ptr<DictionaryValue> report_value(new DictionaryValue());
report_value->SetString("reporter", upload_reporter_string_);
report_value->Set("entries", std::move(beacons_value));
report_value->SetKey("entries", std::move(beacons_value));

*max_upload_depth_out = max_upload_depth;
return std::move(report_value);
Expand Down
3 changes: 2 additions & 1 deletion components/domain_reliability/monitor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,8 @@ void DomainReliabilityMonitor::ClearBrowsingData(
std::unique_ptr<base::Value> DomainReliabilityMonitor::GetWebUIData() const {
std::unique_ptr<base::DictionaryValue> data_value(
new base::DictionaryValue());
data_value->Set("contexts", context_manager_.GetWebUIData());
data_value->SetKey("contexts", base::Value::FromUniquePtrValue(
context_manager_.GetWebUIData()));
return std::move(data_value);
}

Expand Down
20 changes: 10 additions & 10 deletions components/domain_reliability/scheduler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -174,24 +174,24 @@ std::unique_ptr<base::Value> DomainReliabilityScheduler::GetWebUIData() const {
data->SetInteger("collector_index", static_cast<int>(collector_index_));

if (last_upload_finished_) {
std::unique_ptr<base::DictionaryValue> last(new base::DictionaryValue());
last->SetInteger("start_time", (now - last_upload_start_time_).InSeconds());
last->SetInteger("end_time", (now - last_upload_end_time_).InSeconds());
last->SetInteger("collector_index",
static_cast<int>(last_upload_collector_index_));
last->SetBoolean("success", last_upload_success_);
data->Set("last_upload", std::move(last));
base::DictionaryValue last;
last.SetInteger("start_time", (now - last_upload_start_time_).InSeconds());
last.SetInteger("end_time", (now - last_upload_end_time_).InSeconds());
last.SetInteger("collector_index",
static_cast<int>(last_upload_collector_index_));
last.SetBoolean("success", last_upload_success_);
data->SetKey("last_upload", std::move(last));
}

std::unique_ptr<base::ListValue> collectors_value(new base::ListValue());
base::ListValue collectors_value;
for (const auto& collector : collectors_) {
std::unique_ptr<base::DictionaryValue> value(new base::DictionaryValue());
value->SetInteger("failures", collector->failure_count());
value->SetInteger("next_upload",
(collector->GetReleaseTime() - now).InSeconds());
collectors_value->Append(std::move(value));
collectors_value.Append(std::move(value));
}
data->Set("collectors", std::move(collectors_value));
data->SetKey("collectors", std::move(collectors_value));

return std::move(data);
}
Expand Down
6 changes: 3 additions & 3 deletions components/search_engines/default_search_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ void SetOverrides(sync_preferences::TestingPrefServiceSyncable* prefs,
entry->SetString("encoding", "UTF-8");
entry->SetInteger("id", 1001);
entry->SetString("suggest_url", "http://foo.com/suggest?q={searchTerms}");
auto alternate_urls = std::make_unique<base::ListValue>();
alternate_urls->AppendString("http://foo.com/alternate?q={searchTerms}");
entry->Set("alternate_urls", std::move(alternate_urls));
base::ListValue alternate_urls;
alternate_urls.AppendString("http://foo.com/alternate?q={searchTerms}");
entry->SetKey("alternate_urls", std::move(alternate_urls));
overrides->Append(std::move(entry));

entry = std::make_unique<base::DictionaryValue>();
Expand Down
14 changes: 7 additions & 7 deletions components/search_engines/template_url_data_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -169,17 +169,17 @@ std::unique_ptr<base::DictionaryValue> TemplateURLDataToDictionary(
base::NumberToString(data.last_visited.ToInternalValue()));
url_dict->SetInteger(DefaultSearchManager::kUsageCount, data.usage_count);

auto alternate_urls = std::make_unique<base::ListValue>();
base::ListValue alternate_urls;
for (const auto& alternate_url : data.alternate_urls)
alternate_urls->AppendString(alternate_url);
alternate_urls.AppendString(alternate_url);

url_dict->Set(DefaultSearchManager::kAlternateURLs,
std::move(alternate_urls));
url_dict->SetKey(DefaultSearchManager::kAlternateURLs,
std::move(alternate_urls));

auto encodings = std::make_unique<base::ListValue>();
base::ListValue encodings;
for (const auto& input_encoding : data.input_encodings)
encodings->AppendString(input_encoding);
url_dict->Set(DefaultSearchManager::kInputEncodings, std::move(encodings));
encodings.AppendString(input_encoding);
url_dict->SetKey(DefaultSearchManager::kInputEncodings, std::move(encodings));

url_dict->SetBoolean(DefaultSearchManager::kCreatedByPolicy,
data.created_by_policy);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@ TEST_F(TemplateURLPrepopulateDataTest, ProvidersFromPrefs) {

// Test the optional settings too.
entry->SetString("suggest_url", "http://foo.com/suggest?q={searchTerms}");
auto alternate_urls = std::make_unique<base::ListValue>();
alternate_urls->AppendString("http://foo.com/alternate?q={searchTerms}");
entry->Set("alternate_urls", std::move(alternate_urls));
base::ListValue alternate_urls;
alternate_urls.AppendString("http://foo.com/alternate?q={searchTerms}");
entry->SetKey("alternate_urls", std::move(alternate_urls));
overrides = std::make_unique<base::ListValue>();
overrides->Append(entry->CreateDeepCopy());
prefs_.SetUserPref(prefs::kSearchProviderOverrides, std::move(overrides));
Expand Down
14 changes: 7 additions & 7 deletions components/webcrypto/algorithms/aes_cbc_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ TEST_F(WebCryptoAesCbcTest, ImportKeyJwkEmptyKeyOps) {
dict.SetString("kty", "oct");
dict.SetBoolean("ext", false);
dict.SetString("k", "GADWrMRHwQfoNaXU5fZvTg");
dict.Set("key_ops", std::make_unique<base::ListValue>());
dict.SetKey("key_ops", base::ListValue());

// The JWK does not contain encrypt usages.
EXPECT_EQ(Status::ErrorJwkKeyopsInconsistent(),
Expand Down Expand Up @@ -301,9 +301,9 @@ TEST_F(WebCryptoAesCbcTest, ImportKeyJwkKeyOpsNotSuperset) {
base::DictionaryValue dict;
dict.SetString("kty", "oct");
dict.SetString("k", "GADWrMRHwQfoNaXU5fZvTg");
auto key_ops = std::make_unique<base::ListValue>();
key_ops->AppendString("encrypt");
dict.Set("key_ops", std::move(key_ops));
base::ListValue key_ops;
key_ops.AppendString("encrypt");
dict.SetKey("key_ops", std::move(key_ops));

EXPECT_EQ(
Status::ErrorJwkKeyopsInconsistent(),
Expand Down Expand Up @@ -366,9 +366,9 @@ TEST_F(WebCryptoAesCbcTest, ImportJwkKeyOpsLacksUsages) {
dict.SetString("kty", "oct");
dict.SetString("k", "GADWrMRHwQfoNaXU5fZvTg");

auto key_ops = std::make_unique<base::ListValue>();
key_ops->AppendString("foo");
dict.Set("key_ops", std::move(key_ops));
base::ListValue key_ops;
key_ops.AppendString("foo");
dict.SetKey("key_ops", std::move(key_ops));
EXPECT_EQ(Status::ErrorJwkKeyopsInconsistent(),
ImportKeyJwkFromDict(
dict, CreateAlgorithm(blink::kWebCryptoAlgorithmIdAesCbc),
Expand Down
10 changes: 5 additions & 5 deletions components/webcrypto/algorithms/hmac_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,11 @@ TEST_F(WebCryptoHmacTest, ImportKeyJwkUseInconsisteWithKeyOps) {
dict.SetString("alg", "HS256");
dict.SetString("use", "sig");

auto key_ops = std::make_unique<base::ListValue>();
key_ops->AppendString("sign");
key_ops->AppendString("verify");
key_ops->AppendString("encrypt");
dict.Set("key_ops", std::move(key_ops));
base::ListValue key_ops;
key_ops.AppendString("sign");
key_ops.AppendString("verify");
key_ops.AppendString("encrypt");
dict.SetKey("key_ops", std::move(key_ops));
EXPECT_EQ(
Status::ErrorJwkUseAndKeyopsInconsistent(),
ImportKeyJwkFromDict(
Expand Down
Loading

0 comments on commit 6ff811d

Please sign in to comment.