diff --git a/chrome/common/extensions/api/extension_action/browser_action_manifest_unittest.cc b/chrome/common/extensions/api/extension_action/browser_action_manifest_unittest.cc index 6273ffe162744..4a2234ab4a9ba 100644 --- a/chrome/common/extensions/api/extension_action/browser_action_manifest_unittest.cc +++ b/chrome/common/extensions/api/extension_action/browser_action_manifest_unittest.cc @@ -127,11 +127,11 @@ TEST_F(BrowserActionManifestTest, } } })"); + ASSERT_TRUE(manifest_value.is_dict()); std::u16string error = ErrorUtils::FormatErrorMessageUTF16(errors::kInvalidIconPath, "19"); - LoadAndExpectError( - ManifestData(std::move(manifest_value), "Invalid default icon"), - errors::kInvalidIconPath); + LoadAndExpectError(ManifestData(std::move(manifest_value).TakeDict()), + errors::kInvalidIconPath); } } // namespace diff --git a/chrome/common/extensions/api/system_indicator/system_indicator_handler_unittest.cc b/chrome/common/extensions/api/system_indicator/system_indicator_handler_unittest.cc index e3d87af10f172..4bad7423fd6a7 100644 --- a/chrome/common/extensions/api/system_indicator/system_indicator_handler_unittest.cc +++ b/chrome/common/extensions/api/system_indicator/system_indicator_handler_unittest.cc @@ -38,8 +38,8 @@ TEST_F(SystemIndicatorHandlerTest, BasicTests) { "system_indicator": { "default_icon": "icon.png" } })"; scoped_refptr extension = LoadAndExpectSuccess( - ManifestData(base::test::ParseJson(base::StringPrintf(kManifest, kKey)), - "icon")); + ManifestData(base::test::ParseJson(base::StringPrintf(kManifest, kKey)) + .TakeDict())); ASSERT_TRUE(extension); const ExtensionIconSet* icon = SystemIndicatorHandler::GetSystemIndicatorIcon(*extension); @@ -69,8 +69,8 @@ TEST_F(SystemIndicatorHandlerTest, BasicTests) { } })"; scoped_refptr extension = LoadAndExpectSuccess( - ManifestData(base::test::ParseJson(base::StringPrintf(kManifest, kKey)), - "icon")); + ManifestData(base::test::ParseJson(base::StringPrintf(kManifest, kKey)) + .TakeDict())); ASSERT_TRUE(extension); const ExtensionIconSet* icon = SystemIndicatorHandler::GetSystemIndicatorIcon(*extension); @@ -95,8 +95,8 @@ TEST_F(SystemIndicatorHandlerTest, BasicTests) { "system_indicator": {} })"; scoped_refptr extension = LoadAndExpectSuccess( - ManifestData(base::test::ParseJson(base::StringPrintf(kManifest, kKey)), - "icon")); + ManifestData(base::test::ParseJson(base::StringPrintf(kManifest, kKey)) + .TakeDict())); ASSERT_TRUE(extension); const ExtensionIconSet* icon = SystemIndicatorHandler::GetSystemIndicatorIcon(*extension); diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_background_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_background_unittest.cc index aeefed385bc15..c7c7c2f00cdcb 100644 --- a/chrome/common/extensions/manifest_tests/extension_manifests_background_unittest.cc +++ b/chrome/common/extensions/manifest_tests/extension_manifests_background_unittest.cc @@ -36,11 +36,12 @@ TEST_F(ExtensionManifestBackgroundTest, BackgroundPermission) { TEST_F(ExtensionManifestBackgroundTest, BackgroundScripts) { std::string error; - base::Value manifest = LoadManifest("background_scripts.json", &error); - ASSERT_TRUE(manifest.is_dict()); + absl::optional manifest = + LoadManifest("background_scripts.json", &error); + ASSERT_TRUE(manifest); scoped_refptr extension( - LoadAndExpectSuccess(ManifestData(manifest.Clone(), ""))); + LoadAndExpectSuccess(ManifestData(manifest->Clone(), ""))); ASSERT_TRUE(extension.get()); const std::vector& background_scripts = BackgroundInfo::GetBackgroundScripts(extension.get()); @@ -53,26 +54,27 @@ TEST_F(ExtensionManifestBackgroundTest, BackgroundScripts) { std::string("/") + kGeneratedBackgroundPageFilename, BackgroundInfo::GetBackgroundURL(extension.get()).path()); - manifest.SetPath({"background", "page"}, base::Value("monkey.html")); - LoadAndExpectError(ManifestData(std::move(manifest), ""), + manifest->SetByDottedPath("background.page", "monkey.html"); + LoadAndExpectError(ManifestData(std::move(*manifest), ""), errors::kInvalidBackgroundCombination); } TEST_F(ExtensionManifestBackgroundTest, BackgroundServiceWorkerScript) { std::string error; - base::Value manifest = LoadManifest("background_script_sw.json", &error); - ASSERT_TRUE(manifest.is_dict()); + absl::optional manifest = + LoadManifest("background_script_sw.json", &error); + ASSERT_TRUE(manifest); scoped_refptr extension( - LoadAndExpectSuccess(ManifestData(manifest.Clone(), ""))); + LoadAndExpectSuccess(ManifestData(manifest->Clone(), ""))); ASSERT_TRUE(extension.get()); ASSERT_TRUE(BackgroundInfo::IsServiceWorkerBased(extension.get())); const std::string& service_worker_script = BackgroundInfo::GetBackgroundServiceWorkerScript(extension.get()); EXPECT_EQ("service_worker.js", service_worker_script); - manifest.SetPath({"background", "page"}, base::Value("monkey.html")); - LoadAndExpectError(ManifestData(std::move(manifest), ""), + manifest->SetByDottedPath("background.page", "monkey.html"); + LoadAndExpectError(ManifestData(std::move(*manifest), ""), errors::kInvalidBackgroundCombination); } @@ -100,19 +102,20 @@ TEST_F(ExtensionManifestBackgroundTest, BackgroundPageWebRequest) { ScopedCurrentChannel current_channel(version_info::Channel::DEV); std::string error; - base::Value manifest = LoadManifest("background_page.json", &error); - ASSERT_FALSE(manifest.is_none()); - manifest.SetPath({"background", "persistent"}, base::Value(false)); - manifest.SetKey(keys::kManifestVersion, base::Value(2)); + absl::optional manifest = + LoadManifest("background_page.json", &error); + ASSERT_TRUE(manifest); + manifest->SetByDottedPath("background.persistent", false); + manifest->Set(keys::kManifestVersion, 2); scoped_refptr extension( - LoadAndExpectSuccess(ManifestData(manifest.Clone(), ""))); + LoadAndExpectSuccess(ManifestData(manifest->Clone(), ""))); ASSERT_TRUE(extension.get()); EXPECT_TRUE(BackgroundInfo::HasLazyBackgroundPage(extension.get())); - base::Value permissions(base::Value::Type::LIST); - permissions.Append(base::Value("webRequest")); - manifest.SetKey(keys::kPermissions, std::move(permissions)); - LoadAndExpectError(ManifestData(std::move(manifest), ""), + base::Value::List permissions; + permissions.Append("webRequest"); + manifest->Set(keys::kPermissions, std::move(permissions)); + LoadAndExpectError(ManifestData(std::move(*manifest), ""), errors::kWebRequestConflictsWithLazyBackground); } @@ -128,8 +131,8 @@ TEST_F(ExtensionManifestBackgroundTest, BackgroundPageTransientBackground) { "background": { "page": "foo.html" } - })"), - ""))); + })") + .TakeDict()))); ASSERT_TRUE(extension.get()); EXPECT_TRUE(BackgroundInfo::HasPersistentBackgroundPage(extension.get())); @@ -145,8 +148,8 @@ TEST_F(ExtensionManifestBackgroundTest, BackgroundPageTransientBackground) { "background": { "page": "foo.html" } - })"), - ""), + })") + .TakeDict()), errors::kTransientBackgroundConflictsWithPersistentBackground); } @@ -232,7 +235,7 @@ TEST_F(ExtensionManifestBackgroundTest, ManifestV3Restrictions) { })"; base::Value manifest_value = base::test::ParseJson(kManifestBackgroundPage); LoadAndExpectError( - ManifestData(std::move(manifest_value), "background page"), + ManifestData(std::move(manifest_value).TakeDict(), "background page"), get_expected_error(keys::kBackgroundPage)); } { @@ -247,9 +250,9 @@ TEST_F(ExtensionManifestBackgroundTest, ManifestV3Restrictions) { })"; base::Value manifest_value = base::test::ParseJson(kManifestBackgroundScripts); - LoadAndExpectError( - ManifestData(std::move(manifest_value), "background scripts"), - get_expected_error(keys::kBackgroundScripts)); + LoadAndExpectError(ManifestData(std::move(manifest_value).TakeDict(), + "background scripts"), + get_expected_error(keys::kBackgroundScripts)); } { constexpr char kManifestBackgroundPersistent[] = @@ -264,9 +267,9 @@ TEST_F(ExtensionManifestBackgroundTest, ManifestV3Restrictions) { })"; base::Value manifest_value = base::test::ParseJson(kManifestBackgroundPersistent); - LoadAndExpectError( - ManifestData(std::move(manifest_value), "persistent background"), - get_expected_error(keys::kBackgroundPersistent)); + LoadAndExpectError(ManifestData(std::move(manifest_value).TakeDict(), + "persistent background"), + get_expected_error(keys::kBackgroundPersistent)); } { // An extension with no background key present should still be allowed. @@ -279,7 +282,7 @@ TEST_F(ExtensionManifestBackgroundTest, ManifestV3Restrictions) { base::Value manifest_value = base::test::ParseJson(kManifestBackgroundPersistent); LoadAndExpectSuccess( - ManifestData(std::move(manifest_value), "no background")); + ManifestData(std::move(manifest_value).TakeDict(), "no background")); } } @@ -294,7 +297,7 @@ TEST_F(ExtensionManifestBackgroundTest, ModuleServiceWorker) { })"; std::string manifest_str = base::StringPrintf(kManifestStub, background_value); - return base::test::ParseJson(manifest_str); + return base::test::ParseJson(manifest_str).TakeDict(); }; { diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_manifest_version_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_manifest_version_unittest.cc index bf8456df63151..75ebdfc3a9112 100644 --- a/chrome/common/extensions/manifest_tests/extension_manifests_manifest_version_unittest.cc +++ b/chrome/common/extensions/manifest_tests/extension_manifests_manifest_version_unittest.cc @@ -15,32 +15,32 @@ using extensions::Extension; namespace errors = extensions::manifest_errors; TEST_F(ChromeManifestTest, ManifestVersionError) { - base::Value mv_missing(base::Value::Type::DICTIONARY); - mv_missing.SetStringKey("name", "Miles"); - mv_missing.SetStringKey("version", "0.55"); + base::Value::Dict mv_missing; + mv_missing.Set("name", "Miles"); + mv_missing.Set("version", "0.55"); - base::Value mv0 = mv_missing.Clone(); - mv0.SetIntKey("manifest_version", 0); + base::Value::Dict mv0 = mv_missing.Clone(); + mv0.Set("manifest_version", 0); - base::Value mv1 = mv_missing.Clone(); - mv1.SetIntKey("manifest_version", 1); + base::Value::Dict mv1 = mv_missing.Clone(); + mv1.Set("manifest_version", 1); - base::Value mv2 = mv_missing.Clone(); - mv2.SetIntKey("manifest_version", 2); + base::Value::Dict mv2 = mv_missing.Clone(); + mv2.Set("manifest_version", 2); - base::Value mv3 = mv_missing.Clone(); - mv3.SetIntKey("manifest_version", 3); + base::Value::Dict mv3 = mv_missing.Clone(); + mv3.Set("manifest_version", 3); - base::Value mv4 = mv_missing.Clone(); - mv4.SetIntKey("manifest_version", 4); + base::Value::Dict mv4 = mv_missing.Clone(); + mv4.Set("manifest_version", 4); - base::Value mv_string = mv_missing.Clone(); - mv_string.SetStringKey("manifest_version", "2"); + base::Value::Dict mv_string = mv_missing.Clone(); + mv_string.Set("manifest_version", "2"); struct { const char* test_name; bool require_modern_manifest_version; - base::Value manifest; + base::Value::Dict manifest; std::string expected_error; } test_data[] = { {"require_modern_with_default", true, mv_missing.Clone(), diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_name_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_name_unittest.cc index 85946384be605..9fa77249d7f9b 100644 --- a/chrome/common/extensions/manifest_tests/extension_manifests_name_unittest.cc +++ b/chrome/common/extensions/manifest_tests/extension_manifests_name_unittest.cc @@ -22,7 +22,7 @@ class NameManifestTest : public ChromeManifestTest { base::Value manifest_value = base::test::ParseJson( base::StringPrintf(kManifestStub, name, manifest_version)); CHECK(manifest_value.is_dict()); - return ManifestData(std::move(manifest_value), "test"); + return ManifestData(std::move(manifest_value).TakeDict()); } }; diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_platformapp_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_platformapp_unittest.cc index 14ee14b427ede..f60e0a881f84c 100644 --- a/chrome/common/extensions/manifest_tests/extension_manifests_platformapp_unittest.cc +++ b/chrome/common/extensions/manifest_tests/extension_manifests_platformapp_unittest.cc @@ -109,9 +109,9 @@ TEST_F(PlatformAppsManifestTest, PlatformAppContentSecurityPolicy) { TEST_F(PlatformAppsManifestTest, CertainApisRequirePlatformApps) { // Put APIs here that should be restricted to platform apps, but that haven't // yet graduated from experimental. - const char* const kPlatformAppExperimentalApis[] = { - "dns", - "serial", + static constexpr const char* kPlatformAppExperimentalApis[] = { + "dns", + "serial", }; // TODO(miket): When the first platform-app API leaves experimental, write // similar code that tests without the experimental flag. @@ -120,23 +120,23 @@ TEST_F(PlatformAppsManifestTest, CertainApisRequirePlatformApps) { // testing. The requirements are that (1) it be a valid platform app, and (2) // it contain no permissions dictionary. std::string error; - base::Value platform_app_manifest = + absl::optional platform_app_manifest = LoadManifest("init_valid_platform_app.json", &error); + ASSERT_TRUE(platform_app_manifest); - std::vector> manifests; + std::vector manifests; // Create each manifest. for (const char* api_name : kPlatformAppExperimentalApis) { - base::Value permissions(base::Value::Type::LIST); - permissions.Append(base::Value("experimental")); - permissions.Append(base::Value(api_name)); - platform_app_manifest.SetKey("permissions", std::move(permissions)); - manifests.push_back( - std::make_unique(platform_app_manifest.Clone(), "")); + base::Value::List permissions; + permissions.Append("experimental"); + permissions.Append(api_name); + platform_app_manifest->Set("permissions", std::move(permissions)); + manifests.emplace_back(platform_app_manifest->Clone()); } // First try to load without any flags. This should warn for every API. - for (const std::unique_ptr& manifest : manifests) { + for (const auto& manifest : manifests) { LoadAndExpectWarning( - *manifest, + manifest, "'experimental' requires the 'experimental-extension-apis' " "command line switch to be enabled."); } @@ -144,8 +144,9 @@ TEST_F(PlatformAppsManifestTest, CertainApisRequirePlatformApps) { // Now try again with the experimental flag set. base::CommandLine::ForCurrentProcess()->AppendSwitch( switches::kEnableExperimentalExtensionApis); - for (const std::unique_ptr& manifest : manifests) - LoadAndExpectSuccess(*manifest); + for (const auto& manifest : manifests) { + LoadAndExpectSuccess(manifest); + } } } // namespace extensions diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_side_panel_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_side_panel_unittest.cc index 38c4a85616c44..95372e44993f6 100644 --- a/chrome/common/extensions/manifest_tests/extension_manifests_side_panel_unittest.cc +++ b/chrome/common/extensions/manifest_tests/extension_manifests_side_panel_unittest.cc @@ -26,8 +26,8 @@ class SidePanelManifestTest : public ChromeManifestTest { })"; base::Value manifest_value = base::test::ParseJson(base::StringPrintf( kManifestStub, manifest_version, side_panel.c_str())); - EXPECT_EQ(base::Value::Type::DICTIONARY, manifest_value.type()); - return ManifestData(std::move(manifest_value), "test"); + EXPECT_TRUE(manifest_value.is_dict()); + return ManifestData(std::move(manifest_value).TakeDict()); } }; diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_validapp_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_validapp_unittest.cc index 55f1ae3c19fde..78e17dc7e2a57 100644 --- a/chrome/common/extensions/manifest_tests/extension_manifests_validapp_unittest.cc +++ b/chrome/common/extensions/manifest_tests/extension_manifests_validapp_unittest.cc @@ -9,6 +9,7 @@ #include "chrome/common/extensions/manifest_handlers/app_launch_info.h" #include "chrome/common/extensions/manifest_tests/chrome_manifest_test.h" #include "testing/gtest/include/gtest/gtest.h" +#include "third_party/abseil-cpp/absl/types/optional.h" typedef ChromeManifestTest ValidAppManifestTest; @@ -27,10 +28,11 @@ TEST_F(ValidAppManifestTest, ValidApp) { TEST_F(ValidAppManifestTest, AllowUnrecognizedPermissions) { std::string error; - base::Value manifest = LoadManifest("valid_app.json", &error); - base::Value* permissions = - manifest.FindKeyOfType("permissions", base::Value::Type::LIST); + absl::optional manifest = + LoadManifest("valid_app.json", &error); + ASSERT_TRUE(manifest); + base::Value::List* permissions = manifest->FindList("permissions"); ASSERT_TRUE(permissions); permissions->Append("not-a-valid-permission"); - LoadAndExpectSuccess(ManifestData(std::move(manifest), "")); + LoadAndExpectSuccess(ManifestData(std::move(*manifest), "")); } diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_web_accessible_resources_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_web_accessible_resources_unittest.cc index aef9070d3f57b..8fa4eb16cdf04 100644 --- a/chrome/common/extensions/manifest_tests/extension_manifests_web_accessible_resources_unittest.cc +++ b/chrome/common/extensions/manifest_tests/extension_manifests_web_accessible_resources_unittest.cc @@ -26,8 +26,8 @@ class WebAccessibleResourcesManifestTest : public ChromeManifestTest { })"; base::Value manifest_value = base::test::ParseJson(base::StringPrintf( kManifestStub, manifest_version, web_accessible_resources.c_str())); - EXPECT_EQ(base::Value::Type::DICTIONARY, manifest_value.type()); - return ManifestData(std::move(manifest_value), "test"); + EXPECT_TRUE(manifest_value.is_dict()); + return ManifestData(std::move(manifest_value).TakeDict()); } }; @@ -324,8 +324,8 @@ TEST_F(WebAccessibleResourcesManifestTest, })"; base::Value manifest_value = base::test::ParseJson( base::StringPrintf(kManifestStub, extension_id.c_str())); - EXPECT_EQ(base::Value::Type::DICTIONARY, manifest_value.type()); - return ManifestData(std::move(manifest_value), "test"); + EXPECT_TRUE(manifest_value.is_dict()); + return ManifestData(std::move(manifest_value).TakeDict()); }; scoped_refptr extension_callee = LoadAndExpectSuccess(get_manifest_data()); @@ -479,8 +479,8 @@ TEST_F(WebAccessibleResourcesManifestTest, ShouldUseDynamicUrl) { "manifest_version": 3 })"; base::Value manifest_value = base::test::ParseJson(kManifestStub); - EXPECT_EQ(base::Value::Type::DICTIONARY, manifest_value.type()); - auto manifest_data = ManifestData(std::move(manifest_value), "test"); + ASSERT_TRUE(manifest_value.is_dict()); + ManifestData manifest_data(std::move(manifest_value).TakeDict()); scoped_refptr extension(LoadAndExpectSuccess(manifest_data)); EXPECT_EQ(false, WebAccessibleResourcesInfo::ShouldUseDynamicUrl( extension.get(), "resource.html")); diff --git a/chrome/common/extensions/manifest_tests/permissions_parser_unittest.cc b/chrome/common/extensions/manifest_tests/permissions_parser_unittest.cc index 5370359349f1e..6fa873bcbeca7 100644 --- a/chrome/common/extensions/manifest_tests/permissions_parser_unittest.cc +++ b/chrome/common/extensions/manifest_tests/permissions_parser_unittest.cc @@ -225,8 +225,8 @@ TEST_F(PermissionsParserTest, ChromeFavicon) { : manifest_keys::kPermissions; base::Value manifest_value = base::test::ParseJson(base::StringPrintf( kManifestStub, manifest_version, permissions_key, permission)); - EXPECT_EQ(base::Value::Type::DICTIONARY, manifest_value.type()); - return ManifestData(std::move(manifest_value), permission); + EXPECT_TRUE(manifest_value.is_dict()); + return ManifestData(std::move(manifest_value).TakeDict(), permission); }; static constexpr char kFaviconPattern[] = "chrome://favicon/*"; diff --git a/chrome/common/extensions/permissions/settings_override_permission_unittest.cc b/chrome/common/extensions/permissions/settings_override_permission_unittest.cc index f770deb67e463..ace3174bb0f44 100644 --- a/chrome/common/extensions/permissions/settings_override_permission_unittest.cc +++ b/chrome/common/extensions/permissions/settings_override_permission_unittest.cc @@ -70,7 +70,7 @@ class SettingsOverridePermissionTest : public ChromeManifestTest { ext_manifest.Set(manifest_keys::kSettingsOverride, std::move(settings_override)); - ManifestData manifest(std::move(ext_manifest), "test"); + ManifestData manifest(std::move(ext_manifest)); return LoadAndExpectSuccess(manifest); } diff --git a/extensions/common/api/declarative/declarative_manifest_unittest.cc b/extensions/common/api/declarative/declarative_manifest_unittest.cc index 029836a765f57..f97c825e87cfe 100644 --- a/extensions/common/api/declarative/declarative_manifest_unittest.cc +++ b/extensions/common/api/declarative/declarative_manifest_unittest.cc @@ -53,7 +53,8 @@ TEST_F(DeclarativeManifestTest, ConditionMissingType) { " }" " ]" "}"); - ManifestData manifest(std::move(manifest_data), "test"); + ASSERT_TRUE(manifest_data.is_dict()); + ManifestData manifest(std::move(manifest_data).TakeDict()); LoadAndExpectError(manifest, "'type' is required and must be a string"); } @@ -74,7 +75,8 @@ TEST_F(DeclarativeManifestTest, ConditionNotDictionary) { " }" " ]" "}"); - ManifestData manifest(std::move(manifest_data), "test"); + ASSERT_TRUE(manifest_data.is_dict()); + ManifestData manifest(std::move(manifest_data).TakeDict()); LoadAndExpectError(manifest, "expected dictionary, got boolean"); } @@ -96,7 +98,8 @@ TEST_F(DeclarativeManifestTest, ActionMissingType) { " }" " ]" "}"); - ManifestData manifest(std::move(manifest_data), "test"); + ASSERT_TRUE(manifest_data.is_dict()); + ManifestData manifest(std::move(manifest_data).TakeDict()); LoadAndExpectError(manifest, "'type' is required and must be a string"); } @@ -118,7 +121,8 @@ TEST_F(DeclarativeManifestTest, ActionNotDictionary) { " }" " ]" "}"); - ManifestData manifest(std::move(manifest_data), "test"); + ASSERT_TRUE(manifest_data.is_dict()); + ManifestData manifest(std::move(manifest_data).TakeDict()); LoadAndExpectError(manifest, "expected dictionary, got list"); } @@ -131,7 +135,8 @@ TEST_F(DeclarativeManifestTest, EventRulesNotList) { " \"manifest_version\": 2," " \"event_rules\": {}" "}"); - ManifestData manifest(std::move(manifest_data), "test"); + ASSERT_TRUE(manifest_data.is_dict()); + ManifestData manifest(std::move(manifest_data).TakeDict()); LoadAndExpectError(manifest, "'event_rules' expected list, got dictionary"); } @@ -144,7 +149,8 @@ TEST_F(DeclarativeManifestTest, EventRuleNotDictionary) { " \"manifest_version\": 2," " \"event_rules\": [0,1,2]" "}"); - ManifestData manifest(std::move(manifest_data), "test"); + ASSERT_TRUE(manifest_data.is_dict()); + ManifestData manifest(std::move(manifest_data).TakeDict()); LoadAndExpectError(manifest, "expected dictionary, got integer"); } @@ -167,7 +173,8 @@ TEST_F(DeclarativeManifestTest, EventMissingFromRule) { " }" " ]" "}"); - ManifestData manifest(std::move(manifest_data), "test"); + ASSERT_TRUE(manifest_data.is_dict()); + ManifestData manifest(std::move(manifest_data).TakeDict()); LoadAndExpectError(manifest, "'event' is required"); } @@ -184,7 +191,8 @@ TEST_F(DeclarativeManifestTest, RuleFailedToPopulate) { " }" " ]" "}"); - ManifestData manifest(std::move(manifest_data), "test"); + ASSERT_TRUE(manifest_data.is_dict()); + ManifestData manifest(std::move(manifest_data).TakeDict()); LoadAndExpectError(manifest, "rule failed to populate"); } diff --git a/extensions/common/manifest_handlers/action_handlers_handler_unittest.cc b/extensions/common/manifest_handlers/action_handlers_handler_unittest.cc index 0e198b1c06d70..230575feb1810 100644 --- a/extensions/common/manifest_handlers/action_handlers_handler_unittest.cc +++ b/extensions/common/manifest_handlers/action_handlers_handler_unittest.cc @@ -33,7 +33,7 @@ class ActionHandlersManifestTest : public ManifestTest { "manifest_version": 2, "action_handlers": )json" + action_handlers + "}"); - return ManifestData(std::move(manifest), "test"); + return ManifestData(std::move(manifest).TakeDict()); } // Returns all action handlers associated with |extension|. diff --git a/extensions/common/manifest_handlers/content_capabilities_manifest_unittest.cc b/extensions/common/manifest_handlers/content_capabilities_manifest_unittest.cc index 797428b3fbc8b..9908a88f7bf68 100644 --- a/extensions/common/manifest_handlers/content_capabilities_manifest_unittest.cc +++ b/extensions/common/manifest_handlers/content_capabilities_manifest_unittest.cc @@ -40,7 +40,7 @@ TEST_F(ContentCapabilitiesManifestTest, AllowSubdomainWildcards) { .AddJSON(kSubdomainWildcard) .BuildManifest(); scoped_refptr extension = - LoadAndExpectSuccess(ManifestData(std::move(manifest))); + LoadAndExpectSuccess(ManifestData(std::move(manifest).TakeDict())); const ContentCapabilitiesInfo& info = ContentCapabilitiesInfo::Get(extension.get()); // Make sure the wildcard subdomain is included in the pattern set. @@ -67,7 +67,7 @@ TEST_F(ContentCapabilitiesManifestTest, RejectedAllHosts) { base::Value manifest = ExtensionBuilder("all hosts").AddJSON(kAllHosts).BuildManifest(); scoped_refptr extension = LoadAndExpectWarning( - ManifestData(std::move(manifest)), + ManifestData(std::move(manifest).TakeDict()), manifest_errors::kInvalidContentCapabilitiesMatchOrigin); const ContentCapabilitiesInfo& info = ContentCapabilitiesInfo::Get( extension.get()); @@ -100,7 +100,7 @@ TEST_F(ContentCapabilitiesManifestTest, RejectedETLDWildcard) { ExtensionBuilder("etld wildcard").AddJSON(kEtldWildcard).BuildManifest(); std::string error; scoped_refptr extension = - LoadExtension(ManifestData(std::move(manifest)), &error); + LoadExtension(ManifestData(std::move(manifest).TakeDict()), &error); ASSERT_TRUE(extension); EXPECT_TRUE(error.empty()); // 3 bad patterns: *.co.uk, *.appspot.com, . @@ -138,7 +138,7 @@ TEST_F(ContentCapabilitiesManifestTest, InvalidPermission) { .AddJSON(kInvalidPermission) .BuildManifest(); scoped_refptr extension(LoadAndExpectWarning( - ManifestData(std::move(manifest)), + ManifestData(std::move(manifest).TakeDict()), manifest_errors::kInvalidContentCapabilitiesPermission)); const ContentCapabilitiesInfo& info = ContentCapabilitiesInfo::Get( extension.get()); @@ -150,24 +150,6 @@ TEST_F(ContentCapabilitiesManifestTest, InvalidPermission) { EXPECT_EQ(0u, info.permissions.count(APIPermissionID::kUsb)); } -TEST_F(ContentCapabilitiesManifestTest, InvalidValue) { - constexpr char kInvalidValue[] = - R"("content_capabilities": [{ - "matches": [ - "https://valid.example.com/" - ], - "permissions": [ - "clipboardRead", - "clipboardWrite", - "unlimitedStorage" - ] - }])"; - base::Value manifest = - ExtensionBuilder("invalid value").AddJSON(kInvalidValue).BuildManifest(); - LoadAndExpectError(ManifestData(std::move(manifest)), - "expected dictionary, got list"); -} - TEST_F(ContentCapabilitiesManifestTest, RejectNonHttpsUrlPatterns) { constexpr char kNonHttpsMatches[] = R"("content_capabilities": { @@ -183,7 +165,7 @@ TEST_F(ContentCapabilitiesManifestTest, RejectNonHttpsUrlPatterns) { base::Value manifest = ExtensionBuilder("non https matches") .AddJSON(kNonHttpsMatches) .BuildManifest(); - LoadAndExpectError(ManifestData(std::move(manifest)), + LoadAndExpectError(ManifestData(std::move(manifest).TakeDict()), manifest_errors::kInvalidContentCapabilitiesMatch); } @@ -202,7 +184,7 @@ TEST_F(ContentCapabilitiesManifestTest, Valid) { base::Value manifest = ExtensionBuilder("valid").AddJSON(kValid).BuildManifest(); scoped_refptr extension = - LoadAndExpectSuccess(ManifestData(std::move(manifest))); + LoadAndExpectSuccess(ManifestData(std::move(manifest).TakeDict())); const ContentCapabilitiesInfo& info = ContentCapabilitiesInfo::Get( extension.get()); EXPECT_EQ(1u, info.url_patterns.size()); diff --git a/extensions/common/manifest_handlers/extension_action_handler_unittest.cc b/extensions/common/manifest_handlers/extension_action_handler_unittest.cc index c710e49895fcb..a3296d20ea086 100644 --- a/extensions/common/manifest_handlers/extension_action_handler_unittest.cc +++ b/extensions/common/manifest_handlers/extension_action_handler_unittest.cc @@ -106,7 +106,7 @@ TEST_F(ExtensionActionHandlerManifestTest, NoActionSpecified_ManifestV2) { base::Value manifest_value = base::test::ParseJson(kManifest); ASSERT_TRUE(manifest_value.is_dict()); scoped_refptr extension = - LoadAndExpectSuccess(ManifestData(std::move(manifest_value), "test")); + LoadAndExpectSuccess(ManifestData(std::move(manifest_value).TakeDict())); ASSERT_TRUE(extension); const ActionInfo* action_info = @@ -125,7 +125,7 @@ TEST_F(ExtensionActionHandlerManifestTest, NoActionSpecified_ManifestV3) { base::Value manifest_value = base::test::ParseJson(kManifest); ASSERT_TRUE(manifest_value.is_dict()); scoped_refptr extension = - LoadAndExpectSuccess(ManifestData(std::move(manifest_value), "test")); + LoadAndExpectSuccess(ManifestData(std::move(manifest_value).TakeDict())); ASSERT_TRUE(extension); const ActionInfo* action_info = @@ -165,8 +165,7 @@ class ExtensionActionManifestTest base::Value manifest_value = base::test::ParseJson( base::StringPrintf(kManifestStub, action_key, action_spec)); EXPECT_TRUE(manifest_value.is_dict()); - EXPECT_FALSE(manifest_value.is_none()); - return ManifestData(std::move(manifest_value), "test"); + return ManifestData(std::move(manifest_value).TakeDict()); } scoped_refptr LoadExtensionWithDefaultPopup( diff --git a/extensions/common/manifest_handlers/file_handler_manifest_unittest.cc b/extensions/common/manifest_handlers/file_handler_manifest_unittest.cc index 84f1de933af90..074e6967a7e38 100644 --- a/extensions/common/manifest_handlers/file_handler_manifest_unittest.cc +++ b/extensions/common/manifest_handlers/file_handler_manifest_unittest.cc @@ -107,7 +107,7 @@ class FileHandlersManifestV3Test : public ManifestTest { base::Value manifest_value = base::test::ParseJson(base::StringPrintf(kManifestStub, manifest_part)); EXPECT_EQ(base::Value::Type::DICTIONARY, manifest_value.type()); - return ManifestData(std::move(manifest_value)); + return ManifestData(std::move(manifest_value).TakeDict()); } private: diff --git a/extensions/common/manifest_handlers/icons_handler_unittest.cc b/extensions/common/manifest_handlers/icons_handler_unittest.cc index d253eddc3bd90..a53f78116f7de 100644 --- a/extensions/common/manifest_handlers/icons_handler_unittest.cc +++ b/extensions/common/manifest_handlers/icons_handler_unittest.cc @@ -18,7 +18,7 @@ class ProductIconManifestTest : public ManifestTest { ProductIconManifestTest& operator=(const ProductIconManifestTest&) = delete; protected: - base::Value CreateManifest(const std::string& extra_icons) { + base::Value::Dict CreateManifest(const std::string& extra_icons) { constexpr const char kManifest[] = R"({ "name": "test", "version": "0.1", @@ -32,29 +32,29 @@ class ProductIconManifestTest : public ManifestTest { base::Value manifest = base::test::ParseJson( base::StringPrintf(kManifest, extra_icons.c_str())); EXPECT_TRUE(manifest.is_dict()); - return manifest; + return std::move(manifest).TakeDict(); } }; TEST_F(ProductIconManifestTest, Sizes) { // Too big. { - ManifestData manifest(CreateManifest(R"("100000": "icon3.png",)"), "test"); + ManifestData manifest(CreateManifest(R"("100000": "icon3.png",)")); LoadAndExpectError(manifest, "Invalid key in icons: \"100000\"."); } // Too small. { - ManifestData manifest(CreateManifest(R"("0": "icon3.png",)"), "test"); + ManifestData manifest(CreateManifest(R"("0": "icon3.png",)")); LoadAndExpectError(manifest, "Invalid key in icons: \"0\"."); } // NaN. { - ManifestData manifest(CreateManifest(R"("sixteen": "icon3.png",)"), "test"); + ManifestData manifest(CreateManifest(R"("sixteen": "icon3.png",)")); LoadAndExpectError(manifest, "Invalid key in icons: \"sixteen\"."); } // Just right. { - ManifestData manifest(CreateManifest(R"("512": "icon3.png",)"), "test"); + ManifestData manifest(CreateManifest(R"("512": "icon3.png",)")); scoped_refptr extension = LoadAndExpectSuccess(manifest); } diff --git a/extensions/common/manifest_handlers/mime_types_handler_unittest.cc b/extensions/common/manifest_handlers/mime_types_handler_unittest.cc index b78cc309d80a7..95d9088119e77 100644 --- a/extensions/common/manifest_handlers/mime_types_handler_unittest.cc +++ b/extensions/common/manifest_handlers/mime_types_handler_unittest.cc @@ -41,7 +41,8 @@ TEST_F(MimeTypesHandlerNotAllowedTest, Load) { "version": "0.1", "mime_types": ["text/plain", "application/octet-stream"], "mime_types_handler": "index.html" - })"))); + })") + .TakeDict())); ASSERT_TRUE(extension); EXPECT_FALSE(MimeTypesHandler::GetHandler(extension.get())); @@ -55,7 +56,8 @@ TEST_F(MimeTypesHandlerTest, Load) { "version": "0.1", "mime_types": ["text/plain", "application/octet-stream"], "mime_types_handler": "index.html" - })"))); + })") + .TakeDict())); ASSERT_TRUE(extension); MimeTypesHandler* handler = MimeTypesHandler::GetHandler(extension.get()); diff --git a/extensions/common/manifest_handlers/oauth2_manifest_unittest.cc b/extensions/common/manifest_handlers/oauth2_manifest_unittest.cc index da9ab9db9da7d..036c49f3205f8 100644 --- a/extensions/common/manifest_handlers/oauth2_manifest_unittest.cc +++ b/extensions/common/manifest_handlers/oauth2_manifest_unittest.cc @@ -4,6 +4,7 @@ #include +#include "base/strings/strcat.h" #include "base/test/values_test_util.h" #include "base/values.h" #include "extensions/common/api/oauth2.h" @@ -34,9 +35,8 @@ const char kExtensionKey[] = const char kAutoApproveNotAllowedWarning[] = "'oauth2.auto_approve' is not allowed for specified extension ID."; -std::vector GetOauth2KeyPath(base::StringPiece sub_key) { - return std::vector( - {api::oauth2::ManifestKeys::kOauth2, sub_key}); +std::string GetOauth2KeyPath(const char* sub_key) { + return base::StrCat({api::oauth2::ManifestKeys::kOauth2, ".", sub_key}); } } // namespace @@ -56,10 +56,10 @@ class OAuth2ManifestTest : public ManifestTest { CLIENT_ID_EMPTY }; - base::Value CreateManifest(AutoApproveValue auto_approve, - bool extension_id_allowlisted, - ClientIdValue client_id) { - base::Value manifest = base::test::ParseJson(R"({ + base::Value::Dict CreateManifest(AutoApproveValue auto_approve, + bool extension_id_allowlisted, + ClientIdValue client_id) { + base::Value manifest_value = base::test::ParseJson(R"({ "name": "test", "version": "0.1", "manifest_version": 2, @@ -67,69 +67,64 @@ class OAuth2ManifestTest : public ManifestTest { "scopes": [ "scope1" ], }, })"); - EXPECT_TRUE(manifest.is_dict()); + EXPECT_TRUE(manifest_value.is_dict()); + base::Value::Dict manifest = std::move(manifest_value).TakeDict(); switch (auto_approve) { case AUTO_APPROVE_NOT_SET: break; case AUTO_APPROVE_FALSE: - manifest.SetPath(GetOauth2KeyPath(OAuth2Info::kAutoApprove), - base::Value(false)); + manifest.SetByDottedPath(GetOauth2KeyPath(OAuth2Info::kAutoApprove), + false); break; case AUTO_APPROVE_TRUE: - manifest.SetPath(GetOauth2KeyPath(OAuth2Info::kAutoApprove), - base::Value(true)); + manifest.SetByDottedPath(GetOauth2KeyPath(OAuth2Info::kAutoApprove), + true); break; case AUTO_APPROVE_INVALID: - manifest.SetPath(GetOauth2KeyPath(OAuth2Info::kAutoApprove), - base::Value("incorrect value")); + manifest.SetByDottedPath(GetOauth2KeyPath(OAuth2Info::kAutoApprove), + "incorrect value"); break; } switch (client_id) { case CLIENT_ID_DEFAULT: - manifest.SetPath(GetOauth2KeyPath(OAuth2Info::kClientId), - base::Value("client1")); + manifest.SetByDottedPath(GetOauth2KeyPath(OAuth2Info::kClientId), + "client1"); break; case CLIENT_ID_NOT_SET: break; case CLIENT_ID_EMPTY: - manifest.SetPath(GetOauth2KeyPath(OAuth2Info::kClientId), - base::Value("")); + manifest.SetByDottedPath(GetOauth2KeyPath(OAuth2Info::kClientId), ""); } if (extension_id_allowlisted) { - manifest.SetPath(TokenizeDictionaryPath(keys::kKey), - base::Value(kExtensionKey)); + manifest.SetByDottedPath(keys::kKey, kExtensionKey); } return manifest; } }; TEST_F(OAuth2ManifestTest, OAuth2SectionParsing) { - base::Value base_manifest(base::Value::Type::DICTIONARY); - - base_manifest.SetPath(TokenizeDictionaryPath(keys::kName), - base::Value("test")); - base_manifest.SetPath(TokenizeDictionaryPath(keys::kVersion), - base::Value("0.1")); - base_manifest.SetPath(TokenizeDictionaryPath(keys::kManifestVersion), - base::Value(2)); - base_manifest.SetPath(GetOauth2KeyPath(OAuth2Info::kClientId), - base::Value("client1")); - base::Value scopes(base::Value::Type::LIST); - scopes.Append(base::Value("scope1")); - scopes.Append(base::Value("scope2")); - base_manifest.SetPath(GetOauth2KeyPath(OAuth2Info::kScopes), - std::move(scopes)); + base::Value::Dict base_manifest; + + base_manifest.Set(keys::kName, "test"); + base_manifest.Set(keys::kVersion, "0.1"); + base_manifest.Set(keys::kManifestVersion, 2); + base_manifest.SetByDottedPath(GetOauth2KeyPath(OAuth2Info::kClientId), + "client1"); + base::Value::List scopes; + scopes.Append("scope1"); + scopes.Append("scope2"); + base_manifest.SetByDottedPath(GetOauth2KeyPath(OAuth2Info::kScopes), + std::move(scopes)); // OAuth2 section should be parsed for an extension. { - base::Value ext_manifest(base::Value::Type::DICTIONARY); + base::Value::Dict ext_manifest; // Lack of "app" section representa an extension. So the base manifest // itself represents an extension. - ext_manifest.MergeDictionary(&base_manifest); - ext_manifest.SetPath(TokenizeDictionaryPath(keys::kKey), - base::Value(kExtensionKey)); - ext_manifest.SetPath(GetOauth2KeyPath(OAuth2Info::kAutoApprove), - base::Value(true)); + ext_manifest.Merge(base_manifest.Clone()); + ext_manifest.Set(keys::kKey, kExtensionKey); + ext_manifest.SetByDottedPath(GetOauth2KeyPath(OAuth2Info::kAutoApprove), + true); ManifestData manifest(std::move(ext_manifest), "test"); scoped_refptr extension = @@ -146,10 +141,9 @@ TEST_F(OAuth2ManifestTest, OAuth2SectionParsing) { // OAuth2 section should be parsed for a packaged app. { - base::Value app_manifest(base::Value::Type::DICTIONARY); - app_manifest.SetPath(TokenizeDictionaryPath(keys::kLaunchLocalPath), - base::Value("launch.html")); - app_manifest.MergeDictionary(&base_manifest); + base::Value::Dict app_manifest; + app_manifest.SetByDottedPath(keys::kLaunchLocalPath, "launch.html"); + app_manifest.Merge(base_manifest.Clone()); ManifestData manifest(std::move(app_manifest), "test"); scoped_refptr extension = @@ -165,10 +159,9 @@ TEST_F(OAuth2ManifestTest, OAuth2SectionParsing) { // OAuth2 section should NOT be parsed for a hosted app. { - base::Value app_manifest(base::Value::Type::DICTIONARY); - app_manifest.SetPath(TokenizeDictionaryPath(keys::kLaunchWebURL), - base::Value("http://www.google.com")); - app_manifest.MergeDictionary(&base_manifest); + base::Value::Dict app_manifest; + app_manifest.SetByDottedPath(keys::kLaunchWebURL, "http://www.google.com"); + app_manifest.Merge(base_manifest.Clone()); ManifestData manifest(std::move(app_manifest), "test"); scoped_refptr extension = @@ -188,7 +181,7 @@ TEST_F(OAuth2ManifestTest, OAuth2SectionParsing) { } TEST_F(OAuth2ManifestTest, AutoApproveNotSetExtensionNotOnAllowlist) { - base::Value ext_manifest = + base::Value::Dict ext_manifest = CreateManifest(AUTO_APPROVE_NOT_SET, false, CLIENT_ID_DEFAULT); ManifestData manifest(std::move(ext_manifest), "test"); scoped_refptr extension = @@ -198,7 +191,7 @@ TEST_F(OAuth2ManifestTest, AutoApproveNotSetExtensionNotOnAllowlist) { } TEST_F(OAuth2ManifestTest, AutoApproveFalseExtensionNotOnAllowlist) { - base::Value ext_manifest = + base::Value::Dict ext_manifest = CreateManifest(AUTO_APPROVE_FALSE, false, CLIENT_ID_DEFAULT); ManifestData manifest(std::move(ext_manifest), "test"); scoped_refptr extension = @@ -211,7 +204,7 @@ TEST_F(OAuth2ManifestTest, AutoApproveFalseExtensionNotOnAllowlist) { } TEST_F(OAuth2ManifestTest, AutoApproveTrueExtensionNotOnAllowlist) { - base::Value ext_manifest = + base::Value::Dict ext_manifest = CreateManifest(AUTO_APPROVE_TRUE, false, CLIENT_ID_DEFAULT); ManifestData manifest(std::move(ext_manifest), "test"); scoped_refptr extension = @@ -224,7 +217,7 @@ TEST_F(OAuth2ManifestTest, AutoApproveTrueExtensionNotOnAllowlist) { } TEST_F(OAuth2ManifestTest, AutoApproveInvalidExtensionNotOnAllowlist) { - base::Value ext_manifest = + base::Value::Dict ext_manifest = CreateManifest(AUTO_APPROVE_INVALID, false, CLIENT_ID_DEFAULT); ManifestData manifest(std::move(ext_manifest), "test"); scoped_refptr extension = @@ -237,7 +230,7 @@ TEST_F(OAuth2ManifestTest, AutoApproveInvalidExtensionNotOnAllowlist) { } TEST_F(OAuth2ManifestTest, AutoApproveNotSetExtensionOnAllowlist) { - base::Value ext_manifest = + base::Value::Dict ext_manifest = CreateManifest(AUTO_APPROVE_NOT_SET, true, CLIENT_ID_DEFAULT); ManifestData manifest(std::move(ext_manifest), "test"); scoped_refptr extension = @@ -247,7 +240,7 @@ TEST_F(OAuth2ManifestTest, AutoApproveNotSetExtensionOnAllowlist) { } TEST_F(OAuth2ManifestTest, AutoApproveFalseExtensionOnAllowlist) { - base::Value ext_manifest = + base::Value::Dict ext_manifest = CreateManifest(AUTO_APPROVE_FALSE, true, CLIENT_ID_DEFAULT); ManifestData manifest(std::move(ext_manifest), "test"); scoped_refptr extension = @@ -258,7 +251,7 @@ TEST_F(OAuth2ManifestTest, AutoApproveFalseExtensionOnAllowlist) { } TEST_F(OAuth2ManifestTest, AutoApproveTrueExtensionOnAllowlist) { - base::Value ext_manifest = + base::Value::Dict ext_manifest = CreateManifest(AUTO_APPROVE_TRUE, true, CLIENT_ID_DEFAULT); ManifestData manifest(std::move(ext_manifest), "test"); scoped_refptr extension = @@ -269,7 +262,7 @@ TEST_F(OAuth2ManifestTest, AutoApproveTrueExtensionOnAllowlist) { } TEST_F(OAuth2ManifestTest, AutoApproveInvalidExtensionOnAllowlist) { - base::Value ext_manifest = + base::Value::Dict ext_manifest = CreateManifest(AUTO_APPROVE_INVALID, true, CLIENT_ID_DEFAULT); ManifestData manifest(std::move(ext_manifest), "test"); std::string error; @@ -283,7 +276,7 @@ TEST_F(OAuth2ManifestTest, AutoApproveInvalidExtensionOnAllowlist) { TEST_F(OAuth2ManifestTest, InvalidClientId) { { - base::Value ext_manifest = + base::Value::Dict ext_manifest = CreateManifest(AUTO_APPROVE_NOT_SET, false, CLIENT_ID_NOT_SET); ManifestData manifest(std::move(ext_manifest), "test"); std::string error; @@ -291,7 +284,7 @@ TEST_F(OAuth2ManifestTest, InvalidClientId) { } { - base::Value ext_manifest = + base::Value::Dict ext_manifest = CreateManifest(AUTO_APPROVE_NOT_SET, false, CLIENT_ID_EMPTY); ManifestData manifest(std::move(ext_manifest), "test"); std::string error; @@ -302,7 +295,7 @@ TEST_F(OAuth2ManifestTest, InvalidClientId) { TEST_F(OAuth2ManifestTest, ComponentInvalidClientId) { // Component Apps without auto_approve must include a client ID. { - base::Value ext_manifest = + base::Value::Dict ext_manifest = CreateManifest(AUTO_APPROVE_NOT_SET, false, CLIENT_ID_NOT_SET); ManifestData manifest(std::move(ext_manifest), "test"); std::string error; @@ -311,7 +304,7 @@ TEST_F(OAuth2ManifestTest, ComponentInvalidClientId) { } { - base::Value ext_manifest = + base::Value::Dict ext_manifest = CreateManifest(AUTO_APPROVE_NOT_SET, false, CLIENT_ID_EMPTY); ManifestData manifest(std::move(ext_manifest), "test"); std::string error; @@ -322,7 +315,7 @@ TEST_F(OAuth2ManifestTest, ComponentInvalidClientId) { TEST_F(OAuth2ManifestTest, ComponentWithChromeClientId) { { - base::Value ext_manifest = + base::Value::Dict ext_manifest = CreateManifest(AUTO_APPROVE_TRUE, true, CLIENT_ID_NOT_SET); ManifestData manifest(std::move(ext_manifest), "test"); scoped_refptr extension = @@ -331,7 +324,7 @@ TEST_F(OAuth2ManifestTest, ComponentWithChromeClientId) { } { - base::Value ext_manifest = + base::Value::Dict ext_manifest = CreateManifest(AUTO_APPROVE_TRUE, true, CLIENT_ID_EMPTY); ManifestData manifest(std::move(ext_manifest), "test"); scoped_refptr extension = @@ -343,7 +336,7 @@ TEST_F(OAuth2ManifestTest, ComponentWithChromeClientId) { } TEST_F(OAuth2ManifestTest, ComponentWithStandardClientId) { - base::Value ext_manifest = + base::Value::Dict ext_manifest = CreateManifest(AUTO_APPROVE_TRUE, true, CLIENT_ID_DEFAULT); ManifestData manifest(std::move(ext_manifest), "test"); scoped_refptr extension = diff --git a/extensions/common/manifest_handlers/replacement_apps_unittest.cc b/extensions/common/manifest_handlers/replacement_apps_unittest.cc index ebf89ce189d0e..777f737872f85 100644 --- a/extensions/common/manifest_handlers/replacement_apps_unittest.cc +++ b/extensions/common/manifest_handlers/replacement_apps_unittest.cc @@ -42,7 +42,7 @@ class ReplacementAppsManifestTest : public ManifestTest { })"; base::Value manifest = base::test::ParseJson(base::StringPrintf( kManifest, replacement_web_app, replacement_android_app)); - return ManifestData(std::move(manifest), "test"); + return ManifestData(std::move(manifest).TakeDict()); } else if (replacement_web_app != nullptr) { // only web replacement app specified constexpr char kManifest[] = @@ -54,7 +54,7 @@ class ReplacementAppsManifestTest : public ManifestTest { })"; base::Value manifest = base::test::ParseJson( base::StringPrintf(kManifest, replacement_web_app)); - return ManifestData(std::move(manifest), "test"); + return ManifestData(std::move(manifest).TakeDict()); } else if (replacement_android_app != nullptr) { // only Android replacement app specified constexpr char kManifest[] = @@ -71,7 +71,7 @@ class ReplacementAppsManifestTest : public ManifestTest { })"; base::Value manifest = base::test::ParseJson( base::StringPrintf(kManifest, replacement_android_app)); - return ManifestData(std::move(manifest), "test"); + return ManifestData(std::move(manifest).TakeDict()); } base::Value manifest = base::test::ParseJson( @@ -80,7 +80,7 @@ class ReplacementAppsManifestTest : public ManifestTest { "version": "1", "manifest_version": 2 })"); - return ManifestData(std::move(manifest), "test"); + return ManifestData(std::move(manifest).TakeDict()); } private: diff --git a/extensions/common/manifest_test.cc b/extensions/common/manifest_test.cc index b0ea9186342cd..edbc22b9ff364 100644 --- a/extensions/common/manifest_test.cc +++ b/extensions/common/manifest_test.cc @@ -20,6 +20,7 @@ #include "extensions/common/extension_paths.h" #include "extensions/common/manifest_constants.h" #include "testing/gmock/include/gmock/gmock.h" +#include "third_party/abseil-cpp/absl/types/optional.h" #include "ui/base/l10n/l10n_util.h" using extensions::mojom::ManifestLocation; @@ -27,16 +28,15 @@ using extensions::mojom::ManifestLocation; namespace extensions { namespace { -std::string GetNameFromManifest(const base::Value& manifest) { - if (!manifest.is_dict()) - return std::string(); - const std::string* name = manifest.FindStringKey(manifest_keys::kName); +std::string GetNameFromManifest(const base::Value::Dict& manifest) { + const std::string* name = manifest.FindString(manifest_keys::kName); return name ? *name : std::string(); } // |manifest_path| is an absolute path to a manifest file. -base::Value LoadManifestFile(const base::FilePath& manifest_path, - std::string* error) { +absl::optional LoadManifestFile( + const base::FilePath& manifest_path, + std::string* error) { base::FilePath extension_path = manifest_path.DirName(); EXPECT_TRUE(base::PathExists(manifest_path)) << @@ -46,8 +46,9 @@ base::Value LoadManifestFile(const base::FilePath& manifest_path, std::unique_ptr manifest = deserializer.Deserialize(nullptr, error); - if (!manifest || !manifest->is_dict()) - return base::Value(); + if (!manifest || !manifest->is_dict()) { + return absl::nullopt; + } // Most unit tests don't need localization, and they'll fail if we try to // localize them, since their manifests don't have a default_locale key. @@ -60,7 +61,7 @@ base::Value LoadManifestFile(const base::FilePath& manifest_path, extension_l10n_util::GzippedMessagesPermission::kDisallow, error); } - return base::Value(std::move(*manifest)); + return std::move(*manifest).TakeDict(); } } // namespace @@ -76,32 +77,20 @@ ManifestTest::~ManifestTest() = default; ManifestTest::ManifestData::ManifestData(base::StringPiece name) : name_(name) {} -ManifestTest::ManifestData::ManifestData(base::Value manifest, - base::StringPiece name) - : name_(name), manifest_(std::move(manifest)) { - CHECK(manifest_.is_dict()) << "Manifest must be a dictionary. " << name_; -} - ManifestTest::ManifestData::ManifestData(base::Value::Dict manifest, base::StringPiece name) - : ManifestData(base::Value(std::move(manifest)), std::move(name)) {} + : name_(name), manifest_(std::move(manifest)) {} ManifestTest::ManifestData::ManifestData(base::Value::Dict manifest) - : ManifestData(base::Value(std::move(manifest))) {} - -ManifestTest::ManifestData::ManifestData(base::Value manifest) - : name_(GetNameFromManifest(manifest)), manifest_(std::move(manifest)) { - CHECK(manifest_.is_dict()) << "Manifest must be a dictionary."; - CHECK(!name_.empty()) << R"("name" must be specified in the manifest.)"; -} + : name_(GetNameFromManifest(manifest)), manifest_(std::move(manifest)) {} ManifestTest::ManifestData::ManifestData(ManifestData&& other) = default; ManifestTest::ManifestData::~ManifestData() = default; -const base::Value& ManifestTest::ManifestData::GetManifest( - const base::FilePath& test_data_dir, - std::string* error) const { - if (manifest_.is_none()) { +const absl::optional& +ManifestTest::ManifestData::GetManifest(const base::FilePath& test_data_dir, + std::string* error) const { + if (!manifest_) { base::FilePath manifest_path = test_data_dir.AppendASCII(name_); manifest_ = LoadManifestFile(manifest_path, error); } @@ -118,8 +107,9 @@ base::FilePath ManifestTest::GetTestDataDir() { return path.AppendASCII("manifest_tests"); } -base::Value ManifestTest::LoadManifest(char const* manifest_name, - std::string* error) { +absl::optional ManifestTest::LoadManifest( + char const* manifest_name, + std::string* error) { base::FilePath manifest_path = GetTestDataDir().AppendASCII(manifest_name); return LoadManifestFile(manifest_path, error); } @@ -130,11 +120,13 @@ scoped_refptr ManifestTest::LoadExtension( ManifestLocation location, int flags) { base::FilePath test_data_dir = GetTestDataDir(); - const base::Value& value = manifest.GetManifest(test_data_dir, error); - if (value.is_none()) + const absl::optional& dict = + manifest.GetManifest(test_data_dir, error); + if (!dict) { return nullptr; - return Extension::Create(test_data_dir.DirName(), location, value.GetDict(), - flags, GetTestExtensionID(), error); + } + return Extension::Create(test_data_dir.DirName(), location, *dict, flags, + GetTestExtensionID(), error); } scoped_refptr ManifestTest::LoadAndExpectSuccess( diff --git a/extensions/common/manifest_test.h b/extensions/common/manifest_test.h index 02c8c6cb549c8..00336c7ddde76 100644 --- a/extensions/common/manifest_test.h +++ b/extensions/common/manifest_test.h @@ -16,6 +16,7 @@ #include "extensions/common/manifest.h" #include "extensions/common/mojom/manifest.mojom-shared.h" #include "testing/gtest/include/gtest/gtest.h" +#include "third_party/abseil-cpp/absl/types/optional.h" namespace base { class FilePath; @@ -39,23 +40,20 @@ class ManifestTest : public testing::Test { class ManifestData { public: explicit ManifestData(base::StringPiece name); - // This is deprecated. Use the constructor that accepts base::Value::Dict. - explicit ManifestData(base::Value manifest); explicit ManifestData(base::Value::Dict manifest); - // This is deprecated. Use the constructor that accepts base::Value::Dict. - ManifestData(base::Value manifest, base::StringPiece name); ManifestData(base::Value::Dict manifest, base::StringPiece name); ManifestData(ManifestData&& other); ~ManifestData(); const std::string& name() const { return name_; } - const base::Value& GetManifest(const base::FilePath& manifest_path, - std::string* error) const; + const absl::optional& GetManifest( + const base::FilePath& manifest_path, + std::string* error) const; private: const std::string name_; - mutable base::Value manifest_; + mutable absl::optional manifest_; }; // Allows the test implementation to override a loaded test manifest's @@ -66,7 +64,8 @@ class ManifestTest : public testing::Test { // extensions/test/data/manifest_tests. virtual base::FilePath GetTestDataDir(); - base::Value LoadManifest(char const* manifest_name, std::string* error); + absl::optional LoadManifest(char const* manifest_name, + std::string* error); scoped_refptr LoadExtension( const ManifestData& manifest,