Skip to content

Commit 891761c

Browse files
dayeung1Chromium LUCI CQ
authored and
Chromium LUCI CQ
committed
Changed from base::Value to base::Value::Dict in MakeExtension
This is part of Code Health to change base::DictionaryValue to base::Value::Dict for a few UTs. As a side effect, this change will also force the change from base::Value to base::Value::Dict in test_environment MakeExtension and all subsequent call sites. Bug: 1187061 Change-Id: Ic122c85393299a3ee9449aed896d1af4b2168c7b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4118895 Reviewed-by: Dominick Ng <[email protected]> Reviewed-by: Avi Drissman <[email protected]> Commit-Queue: David Yeung <[email protected]> Cr-Commit-Position: refs/heads/main@{#1085722}
1 parent 0a4d05f commit 891761c

11 files changed

+36
-37
lines changed

apps/saved_files_service_unittest.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class SavedFilesServiceUnitTest : public testing::Test {
5050
" ]"
5151
"}";
5252
testing::Test::SetUp();
53-
extension_ = env_.MakeExtension(base::test::ParseJson(kManifest));
53+
extension_ = env_.MakeExtension(base::test::ParseJsonDict(kManifest));
5454
service_ = SavedFilesService::Get(env_.profile());
5555
path_ = base::FilePath(FILE_PATH_LITERAL("filename.ext"));
5656
}
@@ -152,7 +152,7 @@ TEST_F(SavedFilesServiceUnitTest, NoRetainEntriesPermissionTest) {
152152
static const char kManifest[] =
153153
"{\"app\": {\"background\": {\"scripts\": [\"background.js\"]}},"
154154
"\"permissions\": [\"fileSystem\"]}";
155-
extension_ = env_.MakeExtension(base::test::ParseJson(kManifest));
155+
extension_ = env_.MakeExtension(base::test::ParseJsonDict(kManifest));
156156
service_->RegisterFileEntry(extension_->id(), GenerateId(1), path_, true);
157157
TRACE_CALL(CheckEntrySequenceNumber(1, 0));
158158
SavedFileEntry entry;

chrome/browser/extensions/api/declarative/rules_registry_with_cache_unittest.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -55,16 +55,16 @@ class RulesRegistryWithCacheTest : public testing::Test {
5555
void SetUp() override {
5656
// Note that env_.MakeExtension below also forces the creation of
5757
// ExtensionService.
58-
base::DictionaryValue manifest_extra;
58+
base::Value::Dict manifest_extra;
5959
std::string key;
6060
CHECK(Extension::ProducePEM("test extension 1", &key));
61-
manifest_extra.SetStringKey(manifest_keys::kPublicKey, key);
61+
manifest_extra.Set(manifest_keys::kPublicKey, key);
6262
extension1_ = env_.MakeExtension(manifest_extra);
6363
CHECK(extension1_.get());
6464

6565
// Different "key" values for the two extensions ensure a different ID.
6666
CHECK(Extension::ProducePEM("test extension 2", &key));
67-
manifest_extra.SetStringKey(manifest_keys::kPublicKey, key);
67+
manifest_extra.Set(manifest_keys::kPublicKey, key);
6868
extension2_ = env_.MakeExtension(manifest_extra);
6969
CHECK(extension2_.get());
7070
CHECK_NE(extension2_->id(), extension1_->id());

chrome/browser/extensions/api/declarative_content/chrome_content_rules_registry_unittest.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ TEST_F(DeclarativeChromeContentRulesRegistryTest, ActiveRulesDoesntGrow) {
187187
std::vector<const api::events::Rule*> rules({&rule});
188188

189189
const Extension* extension =
190-
env()->MakeExtension(base::test::ParseJson("{\"page_action\": {}}"));
190+
env()->MakeExtension(base::test::ParseJsonDict("{\"page_action\": {}}"));
191191
registry->AddRulesImpl(extension->id(), rules);
192192

193193
registry->DidFinishNavigation(tab.get(), &navigation_handle);

chrome/browser/extensions/api/declarative_content/content_action_unittest.cc

+3-2
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ namespace extensions {
3737
namespace {
3838

3939
using base::test::ParseJson;
40+
using base::test::ParseJsonDict;
4041
using testing::HasSubstr;
4142
using ContentActionType = declarative_content_constants::ContentActionType;
4243
using extensions::mojom::ManifestLocation;
@@ -264,7 +265,7 @@ TEST(DeclarativeContentActionTest, SetIcon) {
264265
base::Value::Dict dict = builder.BuildDict();
265266

266267
const Extension* extension = env.MakeExtension(
267-
ParseJson(R"({"page_action": {"default_title": "Extension"}})"));
268+
ParseJsonDict(R"({"page_action": {"default_title": "Extension"}})"));
268269
base::HistogramTester histogram_tester;
269270
TestingProfile profile;
270271
std::string error;
@@ -322,7 +323,7 @@ TEST(DeclarativeContentActionTest, SetInvisibleIcon) {
322323

323324
// Expect an error and no instance to be created.
324325
const Extension* extension = env.MakeExtension(
325-
ParseJson(R"({"page_action": {"default_title": "Extension"}})"));
326+
ParseJsonDict(R"({"page_action": {"default_title": "Extension"}})"));
326327
base::HistogramTester histogram_tester;
327328
TestingProfile profile;
328329
std::string error;

chrome/browser/extensions/api/device_permissions_manager_unittest.cc

+8-8
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,14 @@ class DevicePermissionsManagerTest : public testing::Test {
4545
testing::Test::SetUp();
4646
env_ = std::make_unique<extensions::TestExtensionEnvironment>();
4747
extension_ = env_->MakeExtension(
48-
base::test::ParseJson("{"
49-
" \"app\": {"
50-
" \"background\": {"
51-
" \"scripts\": [\"background.js\"]"
52-
" }"
53-
" },"
54-
" \"permissions\": [ \"hid\", \"usb\" ]"
55-
"}"));
48+
base::test::ParseJsonDict("{"
49+
" \"app\": {"
50+
" \"background\": {"
51+
" \"scripts\": [\"background.js\"]"
52+
" }"
53+
" },"
54+
" \"permissions\": [ \"hid\", \"usb\" ]"
55+
"}"));
5656

5757
// Set fake device manager for extensions::UsbDeviceManager.
5858
mojo::PendingRemote<device::mojom::UsbDeviceManager> usb_manager;

chrome/browser/extensions/permission_message_combinations_unittest.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class PermissionMessageCombinationsUnittest : public testing::Test {
5151
std::replace(json_manifest_with_double_quotes.begin(),
5252
json_manifest_with_double_quotes.end(), '\'', '"');
5353
app_ = env_.MakeExtension(
54-
base::test::ParseJson(json_manifest_with_double_quotes),
54+
base::test::ParseJsonDict(json_manifest_with_double_quotes),
5555
kAllowlistedExtensionID);
5656
}
5757

chrome/browser/extensions/test_extension_environment.cc

+5-10
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,14 @@ using content::BrowserThread;
3535

3636
namespace {
3737

38-
base::Value::Dict MakeExtensionManifest(const base::Value& manifest_extra) {
38+
base::Value::Dict MakeExtensionManifest(
39+
const base::Value::Dict& manifest_extra) {
3940
base::Value::Dict manifest = DictionaryBuilder()
4041
.Set("name", "Extension")
4142
.Set("version", "1.0")
4243
.Set("manifest_version", 2)
4344
.BuildDict();
44-
if (manifest_extra.is_dict()) {
45-
manifest.Merge(manifest_extra.GetDict().Clone());
46-
} else {
47-
std::string manifest_json;
48-
base::JSONWriter::Write(manifest_extra, &manifest_json);
49-
ADD_FAILURE() << "Expected dictionary; got \"" << manifest_json << "\"";
50-
}
45+
manifest.Merge(manifest_extra.Clone());
5146
return manifest;
5247
}
5348

@@ -128,7 +123,7 @@ ExtensionPrefs* TestExtensionEnvironment::GetExtensionPrefs() {
128123
}
129124

130125
const Extension* TestExtensionEnvironment::MakeExtension(
131-
const base::Value& manifest_extra) {
126+
const base::Value::Dict& manifest_extra) {
132127
base::Value::Dict manifest = MakeExtensionManifest(manifest_extra);
133128
scoped_refptr<const Extension> result =
134129
ExtensionBuilder().SetManifest(std::move(manifest)).Build();
@@ -137,7 +132,7 @@ const Extension* TestExtensionEnvironment::MakeExtension(
137132
}
138133

139134
const Extension* TestExtensionEnvironment::MakeExtension(
140-
const base::Value& manifest_extra,
135+
const base::Value::Dict& manifest_extra,
141136
const std::string& id) {
142137
base::Value::Dict manifest = MakeExtensionManifest(manifest_extra);
143138
scoped_refptr<const Extension> result =

chrome/browser/extensions/test_extension_environment.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,11 @@ class TestExtensionEnvironment {
7878
// The Extension has a default manifest of {name: "Extension",
7979
// version: "1.0", manifest_version: 2}, and values in
8080
// manifest_extra override these defaults.
81-
const Extension* MakeExtension(const base::Value& manifest_extra);
81+
const Extension* MakeExtension(const base::Value::Dict& manifest_extra);
8282

8383
// Use a specific extension ID instead of the default generated in
8484
// Extension::Create.
85-
const Extension* MakeExtension(const base::Value& manifest_extra,
85+
const Extension* MakeExtension(const base::Value::Dict& manifest_extra,
8686
const std::string& id);
8787

8888
// Generates a valid packaged app manifest with the given ID. If |install|

chrome/browser/renderer_context_menu/render_view_context_menu_unittest.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -423,9 +423,9 @@ TEST_F(RenderViewContextMenuExtensionsTest,
423423
&MenuManagerFactory::BuildServiceInstanceForTesting))));
424424

425425
const Extension* extension1 = environment().MakeExtension(
426-
base::DictionaryValue(), "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
426+
base::Value::Dict(), "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
427427
const Extension* extension2 = environment().MakeExtension(
428-
base::DictionaryValue(), "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
428+
base::Value::Dict(), "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
429429

430430
// Create two items in two extensions with same title.
431431
ASSERT_TRUE(

chrome/browser/ui/webui/customize_themes/chrome_customize_themes_handler_unittest.cc

+3-2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "base/run_loop.h"
1515
#include "base/test/bind.h"
1616
#include "base/test/metrics/histogram_tester.h"
17+
#include "base/test/values_test_util.h"
1718
#include "base/values.h"
1819
#include "chrome/browser/extensions/test_extension_environment.h"
1920
#include "chrome/browser/new_tab_page/chrome_colors/generated_colors_info.h"
@@ -228,8 +229,8 @@ TEST_F(ChromeCustomizeThemesHandlerTest, InstallThirdPartyTheme) {
228229
test_data_dir.AppendASCII("extensions/theme_minimal/manifest.json");
229230
std::string config_contents;
230231
ASSERT_TRUE(base::ReadFileToString(manifest_path, &config_contents));
231-
absl::optional<base::Value> manifest =
232-
base::JSONReader::Read(config_contents);
232+
absl::optional<base::Value::Dict> manifest =
233+
base::test::ParseJsonDict(config_contents);
233234
ASSERT_TRUE(manifest.has_value());
234235

235236
test::ThemeServiceChangedWaiter waiter(theme_service());

chrome/browser/ui/webui/print_preview/extension_printer_handler_unittest.cc

+6-4
Original file line numberDiff line numberDiff line change
@@ -569,10 +569,12 @@ TEST_F(ExtensionPrinterHandlerTest, GetUsbPrinters) {
569569
fake_usb_manager_.CreateAndAddDevice(0, 1, "Google", "USB Printer", "");
570570
base::RunLoop().RunUntilIdle();
571571

572-
const Extension* extension_1 = env_.MakeExtension(
573-
base::test::ParseJson(kExtension1), "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
574-
const Extension* extension_2 = env_.MakeExtension(
575-
base::test::ParseJson(kExtension2), "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
572+
const Extension* extension_1 =
573+
env_.MakeExtension(base::test::ParseJsonDict(kExtension1),
574+
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
575+
const Extension* extension_2 =
576+
env_.MakeExtension(base::test::ParseJsonDict(kExtension2),
577+
"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
576578

577579
extensions::DevicePermissionsManager* permissions_manager =
578580
extensions::DevicePermissionsManager::Get(env_.profile());

0 commit comments

Comments
 (0)