Skip to content

Commit

Permalink
CodeHealth: Migrate base::Value::FindBoolKey in chrome/browser/ tests
Browse files Browse the repository at this point in the history
Migrate usages of base::Value::FindBoolKey to base::Value::Dict::FindBool(), as well as any other
occurrence of the API v1 base::Value that can be found
in the affected tests.

Bug: 1187001
Change-Id: I47f62c7db259a108c0354f808be88fa534456ede
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4225012
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Commit-Queue: Anthi Orfanou <anthie@google.com>
Cr-Commit-Position: refs/heads/main@{#1103831}
  • Loading branch information
anthie@google.com authored and Chromium LUCI CQ committed Feb 10, 2023
1 parent 8d5132b commit efa645b
Show file tree
Hide file tree
Showing 11 changed files with 142 additions and 150 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -255,15 +255,15 @@ IN_PROC_BROWSER_TEST_F(NetworkSettingsServiceAshExtensionTest,
EXPECT_EQ(result->extension->id, kExtensionId);

EXPECT_EQ(*(proxy_pref->GetValue()), proxy_config);
EXPECT_EQ(
*extension_proxy_pref->GetValue()->FindStringKey(kPrefExtensionNameKey),
kExtensionName);
EXPECT_EQ(
*extension_proxy_pref->GetValue()->FindStringKey(kPrefExtensionIdKey),
kExtensionId);
EXPECT_EQ(
extension_proxy_pref->GetValue()->FindBoolKey(kPrefExtensionCanDisabled),
true);
EXPECT_EQ(*extension_proxy_pref->GetValue()->GetDict().FindString(
kPrefExtensionNameKey),
kExtensionName);
EXPECT_EQ(*extension_proxy_pref->GetValue()->GetDict().FindString(
kPrefExtensionIdKey),
kExtensionId);
EXPECT_EQ(extension_proxy_pref->GetValue()->GetDict().FindBool(
kPrefExtensionCanDisabled),
true);

network_service_ash_->ClearExtensionProxy();

Expand Down
7 changes: 4 additions & 3 deletions chrome/browser/devtools/devtools_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3206,9 +3206,10 @@ IN_PROC_BROWSER_TEST_F(DevToolsSyncTest, GetSyncInformation) {
})();
)"));
ASSERT_TRUE(result.value.is_dict());
EXPECT_TRUE(*result.value.FindBoolKey("isSyncActive"));
EXPECT_TRUE(*result.value.FindBoolKey("arePreferencesSynced"));
EXPECT_EQ(*result.value.FindStringKey("accountEmail"), "user@gmail.com");
EXPECT_TRUE(*result.value.GetDict().FindBool("isSyncActive"));
EXPECT_TRUE(*result.value.GetDict().FindBool("arePreferencesSynced"));
EXPECT_EQ(*result.value.GetDict().FindString("accountEmail"),
"user@gmail.com");
}

// Regression test for https://crbug.com/1270184.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1573,22 +1573,20 @@ IN_PROC_BROWSER_TEST_F(
ASSERT_TRUE(result_value->is_list());
ASSERT_EQ(2UL, result_value->GetList().size());
{
const base::Value& result_dict = result_value->GetList()[0];
ASSERT_TRUE(result_dict.is_dict());
const std::string* filename = result_dict.FindStringKey("filename");
const base::Value::Dict& result_dict = result_value->GetList()[0].GetDict();
const std::string* filename = result_dict.FindString("filename");
ASSERT_TRUE(filename);
absl::optional<bool> is_incognito = result_dict.FindBoolKey("incognito");
absl::optional<bool> is_incognito = result_dict.FindBool("incognito");
ASSERT_TRUE(is_incognito.has_value());
EXPECT_TRUE(on_item->GetTargetFilePath() ==
base::FilePath::FromUTF8Unsafe(*filename));
EXPECT_FALSE(is_incognito.value());
}
{
const base::Value& result_dict = result_value->GetList()[1];
ASSERT_TRUE(result_dict.is_dict());
const std::string* filename = result_dict.FindStringKey("filename");
const base::Value::Dict& result_dict = result_value->GetList()[1].GetDict();
const std::string* filename = result_dict.FindString("filename");
ASSERT_TRUE(filename);
absl::optional<bool> is_incognito = result_dict.FindBoolKey("incognito");
absl::optional<bool> is_incognito = result_dict.FindBool("incognito");
ASSERT_TRUE(is_incognito.has_value());
EXPECT_TRUE(off_item->GetTargetFilePath() ==
base::FilePath::FromUTF8Unsafe(*filename));
Expand All @@ -1604,13 +1602,12 @@ IN_PROC_BROWSER_TEST_F(
ASSERT_TRUE(result_value->is_list());
ASSERT_EQ(1UL, result_value->GetList().size());
{
const base::Value& result_dict = result_value->GetList()[0];
ASSERT_TRUE(result_dict.is_dict());
const std::string* filename = result_dict.FindStringKey("filename");
const base::Value::Dict& result_dict = result_value->GetList()[0].GetDict();
const std::string* filename = result_dict.FindString("filename");
ASSERT_TRUE(filename);
EXPECT_TRUE(on_item->GetTargetFilePath() ==
base::FilePath::FromUTF8Unsafe(*filename));
absl::optional<bool> is_incognito = result_dict.FindBoolKey("incognito");
absl::optional<bool> is_incognito = result_dict.FindBool("incognito");
ASSERT_TRUE(is_incognito.has_value());
EXPECT_FALSE(is_incognito.value());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,13 +392,13 @@ void LanguageSettingsPrivateApiTest::RunGetLanguageListTest() {
size_t languages_to_test_found_count = 0;
for (auto& language_val : result->GetList()) {
EXPECT_TRUE(language_val.is_dict());
std::string* language_code_ptr = language_val.FindStringKey("code");
std::string* language_code_ptr = language_val.GetDict().FindString("code");
ASSERT_NE(nullptr, language_code_ptr);
std::string language_code = *language_code_ptr;
EXPECT_FALSE(language_code.empty());

const absl::optional<bool> maybe_supports_spellcheck =
language_val.FindBoolKey("supportsSpellcheck");
language_val.GetDict().FindBool("supportsSpellcheck");
const bool supports_spellcheck = maybe_supports_spellcheck.has_value()
? maybe_supports_spellcheck.value()
: false;
Expand All @@ -417,7 +417,7 @@ void LanguageSettingsPrivateApiTest::RunGetLanguageListTest() {
// Check that zh and zh-HK aren't shown as supporting UI.
if (language_code == "zh" || language_code == "zh-HK") {
const absl::optional<bool> maybe_supports_ui =
language_val.FindBoolKey("supportsUI");
language_val.GetDict().FindBool("supportsUI");
const bool supports_ui =
maybe_supports_ui.has_value() ? maybe_supports_ui.value() : false;
EXPECT_FALSE(supports_ui) << language_code << " should not support UI";
Expand Down
15 changes: 8 additions & 7 deletions chrome/browser/extensions/menu_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -618,24 +618,25 @@ TEST_F(MenuManagerTest, ExecuteCommand) {

const base::Value& info = (*list)[0];
ASSERT_TRUE(info.is_dict());
const base::Value::Dict& info_dict = info.GetDict();

ASSERT_EQ(id.uid, info.FindIntKey("menuItemId"));
ASSERT_EQ(parent_id.uid, info.FindIntKey("parentMenuItemId"));
ASSERT_EQ(id.uid, info_dict.FindInt("menuItemId"));
ASSERT_EQ(parent_id.uid, info_dict.FindInt("parentMenuItemId"));

const std::string* tmp = info.FindStringKey("mediaType");
const std::string* tmp = info_dict.FindString("mediaType");
ASSERT_TRUE(tmp);
ASSERT_EQ("image", *tmp);
tmp = info.FindStringKey("srcUrl");
tmp = info_dict.FindString("srcUrl");
ASSERT_TRUE(tmp);
ASSERT_EQ(params.src_url.spec(), *tmp);
tmp = info.FindStringKey("pageUrl");
tmp = info_dict.FindString("pageUrl");
ASSERT_TRUE(tmp);
ASSERT_EQ(params.page_url.spec(), *tmp);
tmp = info.FindStringKey("selectionText");
tmp = info_dict.FindString("selectionText");
ASSERT_TRUE(tmp);
ASSERT_EQ(params.selection_text, base::UTF8ToUTF16(*tmp));

absl::optional<bool> editable = info.FindBoolKey("editable");
absl::optional<bool> editable = info_dict.FindBool("editable");
ASSERT_TRUE(editable.has_value());
ASSERT_EQ(params.is_editable, editable.value());

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/no_best_effort_tasks_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ IN_PROC_BROWSER_TEST_F(NoBestEffortTasksTest, LoadExtensionAndSendMessages) {
request_reply_javascript);
if (result.error.empty()) {
LOG(INFO) << "Got a response from the extension.";
EXPECT_TRUE(result.value.FindBoolKey("pong").value_or(false));
EXPECT_TRUE(result.value.GetDict().FindBool("pong").value_or(false));
break;
}
// An error indicates the extension's message listener isn't up yet. Wait a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "base/json/json_reader.h"
#include "base/test/bind.h"
#include "base/test/scoped_feature_list.h"
#include "base/values.h"
#include "build/build_config.h"
#include "chrome/browser/payments/secure_payment_confirmation_browsertest.h"
#include "components/autofill/core/browser/test_event_waiter.h"
Expand Down Expand Up @@ -570,37 +571,39 @@ IN_PROC_BROWSER_TEST_P(
absl::optional<base::Value> value = base::JSONReader::Read(response);
ASSERT_TRUE(value.has_value());
ASSERT_TRUE(value->is_dict());

std::string* type = value->FindStringKey("type");
const base::Value::Dict& value_dict = value->GetDict();
const std::string* type = value_dict.FindString("type");
ASSERT_NE(nullptr, type) << response;
EXPECT_EQ("payment.get", *type);

std::string* origin = value->FindStringKey("origin");
const std::string* origin = value_dict.FindString("origin");
ASSERT_NE(nullptr, origin) << response;
EXPECT_EQ(https_server()->GetURL("b.com", "/"), GURL(*origin));

absl::optional<bool> cross_origin = value->FindBoolKey("crossOrigin");
absl::optional<bool> cross_origin = value_dict.FindBool("crossOrigin");
ASSERT_TRUE(cross_origin.has_value()) << response;
EXPECT_TRUE(cross_origin.value());

std::string* payee_name = value->FindStringPath("payment.payeeName");
const std::string* payee_name =
value_dict.FindStringByDottedPath("payment.payeeName");
ASSERT_EQ(nullptr, payee_name) << response;

std::string* payee_origin = value->FindStringPath("payment.payeeOrigin");
const std::string* payee_origin =
value_dict.FindStringByDottedPath("payment.payeeOrigin");
ASSERT_NE(nullptr, payee_origin) << response;
EXPECT_EQ(GURL("https://example-payee-origin.test"), GURL(*payee_origin));

std::string* top_origin = value->FindStringPath("payment.topOrigin");
const std::string* top_origin =
value_dict.FindStringByDottedPath("payment.topOrigin");
ASSERT_NE(nullptr, top_origin) << response;
EXPECT_EQ(https_server()->GetURL("a.com", "/"), GURL(*top_origin));

std::string* rpId = value->FindStringPath("payment.rpId");
const std::string* rpId = value_dict.FindStringByDottedPath("payment.rpId");
ASSERT_NE(nullptr, rpId) << response;
EXPECT_EQ("a.com", *rpId);

// TODO(crbug.com/1356224): Remove legacy 'rp' parameter.
if (IsRpFieldEnabled()) {
std::string* rp = value->FindStringPath("payment.rp");
const std::string* rp = value_dict.FindStringByDottedPath("payment.rp");
ASSERT_NE(nullptr, rp) << response;
EXPECT_EQ("a.com", *rp);
}
Expand Down
109 changes: 47 additions & 62 deletions chrome/browser/ui/webui/extensions/extensions_internals_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,28 +71,30 @@ TEST_F(ExtensionsInternalsUnitTest, WriteToStringPermissions) {

base::Value* extension_1 = &extensions_list->GetList()[0];
ASSERT_TRUE(extension_1->is_dict());
base::Value* permissions = extension_1->FindDictKey("permissions");
base::Value::Dict* permissions =
extension_1->GetDict().FindDict("permissions");
ASSERT_TRUE(permissions);

// Permissions section should always have four elements: active, optional,
// tab-specific and withheld.
EXPECT_EQ(permissions->DictSize(), 4U);
EXPECT_EQ(permissions->size(), 4U);

base::Value* active = permissions->FindDictKey("active");
ASSERT_NE(active->FindListKey("api"), nullptr);
EXPECT_EQ(active->FindListKey("api")->GetList()[0].GetString(), "activeTab");
ASSERT_NE(active->FindListKey("manifest"), nullptr);
base::Value::Dict* active = permissions->FindDict("active");
ASSERT_NE(active->FindList("api"), nullptr);
EXPECT_EQ(active->FindList("api")->front().GetString(), "activeTab");
ASSERT_NE(active->FindList("manifest"), nullptr);
EXPECT_TRUE(active->FindList("manifest")->front().is_dict());
EXPECT_TRUE(
active->FindListKey("manifest")->GetList()[0].FindBoolKey("automation"));
ASSERT_NE(active->FindListKey("explicit_hosts"), nullptr);
EXPECT_EQ(active->FindListKey("explicit_hosts")->GetList()[0].GetString(),
active->FindList("manifest")->front().GetDict().FindBool("automation"));
ASSERT_NE(active->FindList("explicit_hosts"), nullptr);
EXPECT_EQ(active->FindList("explicit_hosts")->front().GetString(),
"https://example.com/*");
ASSERT_NE(active->FindListKey("scriptable_hosts"), nullptr);
EXPECT_EQ(active->FindListKey("scriptable_hosts")->GetList()[0].GetString(),
ASSERT_NE(active->FindList("scriptable_hosts"), nullptr);
EXPECT_EQ(active->FindList("scriptable_hosts")->front().GetString(),
"https://chromium.org/foo");

base::Value* optional = permissions->FindDictKey("optional");
EXPECT_EQ(optional->FindListKey("api")->GetList()[0].GetString(), "storage");
base::Value::Dict* optional = permissions->FindDict("optional");
EXPECT_EQ(optional->FindList("api")->front().GetString(), "storage");
}

// Test that tab-specific permissions show up correctly in the JSON returned by
Expand All @@ -109,11 +111,11 @@ TEST_F(ExtensionsInternalsUnitTest, WriteToStringTabSpecificPermissions) {
ExtensionsInternalsSource source(profile());
auto extensions_list = base::JSONReader::Read(source.WriteToString());
ASSERT_TRUE(extensions_list) << "Failed to parse extensions internals json.";
base::Value* permissions =
extensions_list->GetList()[0].FindDictKey("permissions");
base::Value::Dict* permissions =
extensions_list->GetList()[0].GetDict().FindDict("permissions");

// Check that initially there is no tab-scpecific data.
EXPECT_EQ(permissions->FindDictKey("tab_specific")->DictSize(), 0U);
EXPECT_EQ(permissions->FindDict("tab_specific")->size(), 0U);

// Grant a tab specific permission to the extension.
extensions::APIPermissionSet tab_api_permissions;
Expand All @@ -127,26 +129,23 @@ TEST_F(ExtensionsInternalsUnitTest, WriteToStringTabSpecificPermissions) {
extension->permissions_data()->UpdateTabSpecificPermissions(1,
tab_permissions);
extensions_list = base::JSONReader::Read(source.WriteToString());
permissions = extensions_list->GetList()[0].FindDictKey("permissions");
EXPECT_TRUE(extensions_list->GetList()[0].is_dict());
permissions = extensions_list->GetList()[0].GetDict().FindDict("permissions");

// Check the tab specific data is present now.
base::Value* tab_specific = permissions->FindDictKey("tab_specific");
EXPECT_TRUE(tab_specific->is_dict());
EXPECT_EQ(tab_specific->DictSize(), 1U);
EXPECT_EQ(tab_specific->FindDictKey("1")
->FindListKey("explicit_hosts")
->GetList()[0]
base::Value::Dict* tab_specific = permissions->FindDict("tab_specific");
EXPECT_EQ(tab_specific->size(), 1U);
EXPECT_EQ(tab_specific->FindDict("1")
->FindList("explicit_hosts")
->front()
.GetString(),
"https://google.com/*");
EXPECT_EQ(tab_specific->FindDictKey("1")
->FindListKey("scriptable_hosts")
->GetList()[0]
EXPECT_EQ(tab_specific->FindDict("1")
->FindList("scriptable_hosts")
->front()
.GetString(),
"https://google.com/*");
EXPECT_EQ(tab_specific->FindDictKey("1")
->FindListKey("api")
->GetList()[0]
.GetString(),
EXPECT_EQ(tab_specific->FindDict("1")->FindList("api")->front().GetString(),
"tabs");
}

Expand All @@ -166,52 +165,38 @@ TEST_F(ExtensionsInternalsUnitTest, WriteToStringWithheldPermissions) {
ExtensionsInternalsSource source(profile());
auto extensions_list = base::JSONReader::Read(source.WriteToString());
ASSERT_TRUE(extensions_list) << "Failed to parse extensions internals json.";
base::Value* permissions =
extensions_list->GetList()[0].FindDictKey("permissions");
base::Value::Dict* permissions =
extensions_list->GetList()[0].GetDict().FindDict("permissions");

// Check the host is initially in active hosts and there are no withheld
// entries.
EXPECT_EQ(permissions->FindDictKey("active")
->FindListKey("explicit_hosts")
->GetList()[0]
EXPECT_EQ(permissions->FindDict("active")
->FindList("explicit_hosts")
->front()
.GetString(),
"https://example.com/*");
EXPECT_EQ(permissions->FindDictKey("withheld")
->FindListKey("api")
->GetList()
.size(),
0U);
EXPECT_EQ(permissions->FindDictKey("withheld")
->FindListKey("manifest")
->GetList()
.size(),
0U);
EXPECT_EQ(permissions->FindDictKey("withheld")
->FindListKey("explicit_hosts")
->GetList()
.size(),
0U);
EXPECT_EQ(permissions->FindDictKey("withheld")
->FindListKey("scriptable_hosts")
->GetList()
.size(),
EXPECT_EQ(permissions->FindDict("withheld")->FindList("api")->size(), 0U);
EXPECT_EQ(permissions->FindDict("withheld")->FindList("manifest")->size(),
0U);
EXPECT_EQ(
permissions->FindDict("withheld")->FindList("explicit_hosts")->size(),
0U);
EXPECT_EQ(
permissions->FindDict("withheld")->FindList("scriptable_hosts")->size(),
0U);

// Change an active host to be withheld.
extensions::ScriptingPermissionsModifier modifier(profile(), extension);
modifier.SetWithholdHostPermissions(true);
extensions_list = base::JSONReader::Read(source.WriteToString());
permissions = extensions_list->GetList()[0].FindDictKey("permissions");
permissions = extensions_list->GetList()[0].GetDict().FindDict("permissions");

// Check the host that was active is now withheld.
EXPECT_EQ(permissions->FindDictKey("active")
->FindListKey("explicit_hosts")
->GetList()
.size(),
EXPECT_EQ(permissions->FindDict("active")->FindList("explicit_hosts")->size(),
0U);
EXPECT_EQ(permissions->FindDictKey("withheld")
->FindListKey("explicit_hosts")
->GetList()[0]
EXPECT_EQ(permissions->FindDict("withheld")
->FindList("explicit_hosts")
->front()
.GetString(),
"https://example.com/*");
}

0 comments on commit efa645b

Please sign in to comment.