Remove deprecated ManifestData constructors.

Update callers that were using those deprecated ctors, by passing
base::Value directly from them.

This CL also removes some ParseJsonDeprecated usage in
//extensions.

Bug: 1062547, 932873
Change-Id: Ib7609d63b8530845e3458b43d4f73c5448028456
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2096107
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751261}

chrome/common/extensions/api/extension_action/browser_action_manifest_unittest.cc

@@ -5,6 +5,7 @@
 #include <memory>
 #include <utility>
 
+#include "base/test/values_test_util.h"
 #include "chrome/common/extensions/api/extension_action/action_info.h"
 #include "chrome/common/extensions/manifest_tests/chrome_manifest_test.h"
 #include "extensions/common/constants.h"
@@ -112,22 +113,19 @@ TEST_F(BrowserActionManifestTest,
 
 TEST_F(BrowserActionManifestTest,
        BrowserActionManifestIcons_InvalidDefaultIcon) {
-  std::unique_ptr<base::DictionaryValue> manifest_value =
-      DictionaryBuilder()
-          .Set("name", "Invalid default icon")
-          .Set("version", "1.0.0")
-          .Set("manifest_version", 2)
-          .Set("browser_action",
-               DictionaryBuilder()
-                   .Set("default_icon",
-                        DictionaryBuilder()
-                            .Set("19", std::string())  // Invalid value.
-                            .Set("24", "icon24.png")
-                            .Set("38", "icon38.png")
-                            .Build())
-                   .Build())
-          .Build();
-
+  base::Value manifest_value = base::test::ParseJson(R"(
+      {
+        "name": "Invalid default icon",
+        "version": "1.0.0",
+        "manifest_version": 2,
+        "browser_action": {
+          "default_icon": {
+            "19": "",  // Invalid value.
+            "24": "icon24.png",
+            "38": "icon38.png"
+          }
+        }
+      })");
   base::string16 error = ErrorUtils::FormatErrorMessageUTF16(
       errors::kInvalidIconPath, "19");
   LoadAndExpectError(

chrome/common/extensions/manifest_tests/extension_manifests_background_unittest.cc

@@ -39,7 +39,7 @@ TEST_F(ExtensionManifestBackgroundTest, BackgroundScripts) {
   ASSERT_TRUE(manifest.is_dict());
 
   scoped_refptr<Extension> extension(
-      LoadAndExpectSuccess(ManifestData(&manifest, "")));
+      LoadAndExpectSuccess(ManifestData(manifest.Clone(), "")));
   ASSERT_TRUE(extension.get());
   const std::vector<std::string>& background_scripts =
       BackgroundInfo::GetBackgroundScripts(extension.get());
@@ -53,7 +53,7 @@ TEST_F(ExtensionManifestBackgroundTest, BackgroundScripts) {
       BackgroundInfo::GetBackgroundURL(extension.get()).path());
 
   manifest.SetPath({"background", "page"}, base::Value("monkey.html"));
-  LoadAndExpectError(ManifestData(&manifest, ""),
+  LoadAndExpectError(ManifestData(std::move(manifest), ""),
                      errors::kInvalidBackgroundCombination);
 }
 
@@ -86,14 +86,14 @@ TEST_F(ExtensionManifestBackgroundTest, BackgroundPageWebRequest) {
   manifest.SetPath({"background", "persistent"}, base::Value(false));
   manifest.SetKey(keys::kManifestVersion, base::Value(2));
   scoped_refptr<Extension> extension(
-      LoadAndExpectSuccess(ManifestData(&manifest, "")));
+      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(&manifest, ""),
+  LoadAndExpectError(ManifestData(std::move(manifest), ""),
                      errors::kWebRequestConflictsWithLazyBackground);
 }
 

chrome/common/extensions/manifest_tests/extension_manifests_manifest_version_unittest.cc

@@ -14,28 +14,28 @@ using extensions::Extension;
 namespace errors = extensions::manifest_errors;
 
 TEST_F(ChromeManifestTest, ManifestVersionError) {
-  std::unique_ptr<base::DictionaryValue> manifest1(new base::DictionaryValue());
-  manifest1->SetString("name", "Miles");
-  manifest1->SetString("version", "0.55");
+  base::Value manifest1(base::Value::Type::DICTIONARY);
+  manifest1.SetStringKey("name", "Miles");
+  manifest1.SetStringKey("version", "0.55");
 
-  std::unique_ptr<base::DictionaryValue> manifest2(manifest1->DeepCopy());
-  manifest2->SetInteger("manifest_version", 1);
+  base::Value manifest2 = manifest1.Clone();
+  manifest2.SetIntKey("manifest_version", 1);
 
-  std::unique_ptr<base::DictionaryValue> manifest3(manifest1->DeepCopy());
-  manifest3->SetInteger("manifest_version", 2);
+  base::Value manifest3 = manifest1.Clone();
+  manifest3.SetIntKey("manifest_version", 2);
 
   struct {
     const char* test_name;
     bool require_modern_manifest_version;
-    base::DictionaryValue* manifest;
+    base::Value manifest;
     bool expect_error;
   } test_data[] = {
-      {"require_modern_with_default", true, manifest1.get(), true},
-      {"require_modern_with_v1", true, manifest2.get(), true},
-      {"require_modern_with_v2", true, manifest3.get(), false},
-      {"dont_require_modern_with_default", false, manifest1.get(), true},
-      {"dont_require_modern_with_v1", false, manifest2.get(), true},
-      {"dont_require_modern_with_v2", false, manifest3.get(), false},
+      {"require_modern_with_default", true, manifest1.Clone(), true},
+      {"require_modern_with_v1", true, manifest2.Clone(), true},
+      {"require_modern_with_v2", true, manifest3.Clone(), false},
+      {"dont_require_modern_with_default", false, manifest1.Clone(), true},
+      {"dont_require_modern_with_v1", false, manifest2.Clone(), true},
+      {"dont_require_modern_with_v2", false, manifest3.Clone(), false},
   };
 
   for (auto& entry : test_data) {
@@ -43,12 +43,14 @@ TEST_F(ChromeManifestTest, ManifestVersionError) {
     if (entry.require_modern_manifest_version)
       create_flags |= Extension::REQUIRE_MODERN_MANIFEST_VERSION;
     if (entry.expect_error) {
-      LoadAndExpectError(ManifestData(entry.manifest, entry.test_name),
-                         errors::kInvalidManifestVersionOld,
-                         extensions::Manifest::UNPACKED, create_flags);
+      LoadAndExpectError(
+          ManifestData(std::move(entry.manifest), entry.test_name),
+          errors::kInvalidManifestVersionOld, extensions::Manifest::UNPACKED,
+          create_flags);
     } else {
-      LoadAndExpectSuccess(ManifestData(entry.manifest, entry.test_name),
-                           extensions::Manifest::UNPACKED, create_flags);
+      LoadAndExpectSuccess(
+          ManifestData(std::move(entry.manifest), entry.test_name),
+          extensions::Manifest::UNPACKED, create_flags);
     }
   }
 }

chrome/common/extensions/manifest_tests/extension_manifests_platformapp_unittest.cc

@@ -129,8 +129,7 @@ TEST_F(PlatformAppsManifestTest, CertainApisRequirePlatformApps) {
     permissions.Append(base::Value("experimental"));
     permissions.Append(base::Value(api_name));
     manifest.SetKey("permissions", std::move(permissions));
-    manifests.push_back(
-        std::make_unique<ManifestData>(manifest.CreateDeepCopy(), ""));
+    manifests.push_back(std::make_unique<ManifestData>(manifest.Clone(), ""));
   }
   // First try to load without any flags. This should warn for every API.
   for (const std::unique_ptr<ManifestData>& manifest : manifests) {

chrome/common/extensions/permissions/settings_override_permission_unittest.cc

@@ -71,7 +71,7 @@ class SettingsOverridePermissionTest : public ChromeManifestTest {
     ext_manifest.Set(manifest_keys::kSettingsOverride,
                      std::move(settings_override));
 
-    ManifestData manifest(&ext_manifest, "test");
+    ManifestData manifest(std::move(ext_manifest), "test");
     return LoadAndExpectSuccess(manifest);
   }
 

extensions/common/api/declarative/declarative_manifest_unittest.cc

@@ -4,6 +4,7 @@
 
 #include <utility>
 
+#include "base/test/values_test_util.h"
 #include "extensions/browser/api_test_utils.h"
 #include "extensions/common/api/declarative/declarative_manifest_data.h"
 #include "extensions/common/manifest_test.h"
@@ -35,7 +36,7 @@ TEST_F(DeclarativeManifestTest, Valid) {
 
 TEST_F(DeclarativeManifestTest, ConditionMissingType) {
   // Create extension
-  std::unique_ptr<base::DictionaryValue> manifest_data = ParseDictionary(
+  base::Value manifest_data = base::test::ParseJson(
       "{"
       "  \"name\": \"Test\","
       "  \"version\": \"1\","
@@ -58,7 +59,7 @@ TEST_F(DeclarativeManifestTest, ConditionMissingType) {
 
 TEST_F(DeclarativeManifestTest, ConditionNotDictionary) {
   // Create extension
-  std::unique_ptr<base::DictionaryValue> manifest_data = ParseDictionary(
+  base::Value manifest_data = base::test::ParseJson(
       "{"
       "  \"name\": \"Test\","
       "  \"version\": \"1\","
@@ -79,7 +80,7 @@ TEST_F(DeclarativeManifestTest, ConditionNotDictionary) {
 
 TEST_F(DeclarativeManifestTest, ActionMissingType) {
   // Create extension
-  std::unique_ptr<base::DictionaryValue> manifest_data = ParseDictionary(
+  base::Value manifest_data = base::test::ParseJson(
       "{"
       "  \"name\": \"Test\","
       "  \"version\": \"1\","
@@ -101,7 +102,7 @@ TEST_F(DeclarativeManifestTest, ActionMissingType) {
 
 TEST_F(DeclarativeManifestTest, ActionNotDictionary) {
   // Create extension
-  std::unique_ptr<base::DictionaryValue> manifest_data = ParseDictionary(
+  base::Value manifest_data = base::test::ParseJson(
       "{"
       "  \"name\": \"Test\","
       "  \"version\": \"1\","
@@ -123,7 +124,7 @@ TEST_F(DeclarativeManifestTest, ActionNotDictionary) {
 
 TEST_F(DeclarativeManifestTest, EventRulesNotList) {
   // Create extension
-  std::unique_ptr<base::DictionaryValue> manifest_data = ParseDictionary(
+  base::Value manifest_data = base::test::ParseJson(
       "{"
       "  \"name\": \"Test\","
       "  \"version\": \"1\","
@@ -136,7 +137,7 @@ TEST_F(DeclarativeManifestTest, EventRulesNotList) {
 
 TEST_F(DeclarativeManifestTest, EventRuleNotDictionary) {
   // Create extension
-  std::unique_ptr<base::DictionaryValue> manifest_data = ParseDictionary(
+  base::Value manifest_data = base::test::ParseJson(
       "{"
       "  \"name\": \"Test\","
       "  \"version\": \"1\","
@@ -149,7 +150,7 @@ TEST_F(DeclarativeManifestTest, EventRuleNotDictionary) {
 
 TEST_F(DeclarativeManifestTest, EventMissingFromRule) {
   // Create extension
-  std::unique_ptr<base::DictionaryValue> manifest_data = ParseDictionary(
+  base::Value manifest_data = base::test::ParseJson(
       "{"
       "  \"name\": \"Test\","
       "  \"version\": \"1\","
@@ -172,7 +173,7 @@ TEST_F(DeclarativeManifestTest, EventMissingFromRule) {
 
 TEST_F(DeclarativeManifestTest, RuleFailedToPopulate) {
   // Create extension
-  std::unique_ptr<base::DictionaryValue> manifest_data = ParseDictionary(
+  base::Value manifest_data = base::test::ParseJson(
       "{"
       "  \"name\": \"Test\","
       "  \"version\": \"1\","

extensions/common/manifest_handlers/action_handlers_handler_unittest.cc

@@ -33,8 +33,7 @@ class ActionHandlersManifestTest : public ManifestTest {
                                     "manifest_version": 2,
                                     "action_handlers": )json" +
                                                  action_handlers + "}");
-    return ManifestData(base::Value::ToUniquePtrValue(std::move(manifest)),
-                        "test");
+    return ManifestData(std::move(manifest), "test");
   }
 
   // Returns all action handlers associated with |extension|.

extensions/common/manifest_handlers/icons_handler_unittest.cc

@@ -2,8 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/test/values_test_util.h"
 #include "extensions/common/manifest_handlers/icons_handler.h"
+#include "base/strings/stringprintf.h"
+#include "base/test/values_test_util.h"
 #include "extensions/common/manifest_test.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -11,22 +12,23 @@ namespace extensions {
 
 class ProductIconManifestTest : public ManifestTest {
  public:
-  ProductIconManifestTest() {}
+  ProductIconManifestTest() = default;
 
  protected:
-  std::unique_ptr<base::Value> CreateManifest(const std::string& extra_icons) {
-    std::unique_ptr<base::Value> manifest = base::test::ParseJsonDeprecated(
-        "{ \n"
-        "  \"name\": \"test\", \n"
-        "  \"version\": \"0.1\", \n"
-        "  \"manifest_version\": 2, \n"
-        "  \"icons\": { \n" +
-        extra_icons +
-        "    \"16\": \"icon1.png\", \n"
-        "    \"32\": \"icon2.png\" \n"
-        "  } \n"
-        "} \n");
-    EXPECT_TRUE(manifest);
+  base::Value CreateManifest(const std::string& extra_icons) {
+    constexpr const char kManifest[] = R"({
+      "name": "test",
+      "version": "0.1",
+      "manifest_version": 2,
+      "icons": {
+        %s
+        "16": "icon1.png",
+        "32": "icon2.png"
+      }
+    })";
+    base::Value manifest = base::test::ParseJson(
+        base::StringPrintf(kManifest, extra_icons.c_str()));
+    EXPECT_TRUE(manifest.is_dict());
     return manifest;
   }
 
@@ -37,30 +39,22 @@ class ProductIconManifestTest : public ManifestTest {
 TEST_F(ProductIconManifestTest, Sizes) {
   // Too big.
   {
-    std::unique_ptr<base::Value> ext_manifest =
-        CreateManifest("\"100000\": \"icon3.png\", \n");
-    ManifestData manifest(std::move(ext_manifest), "test");
+    ManifestData manifest(CreateManifest(R"("100000": "icon3.png",)"), "test");
     LoadAndExpectError(manifest, "Invalid key in icons: \"100000\".");
   }
   // Too small.
   {
-    std::unique_ptr<base::Value> ext_manifest =
-        CreateManifest("\"0\": \"icon3.png\", \n");
-    ManifestData manifest(std::move(ext_manifest), "test");
+    ManifestData manifest(CreateManifest(R"("0": "icon3.png",)"), "test");
     LoadAndExpectError(manifest, "Invalid key in icons: \"0\".");
   }
   // NaN.
   {
-    std::unique_ptr<base::Value> ext_manifest =
-        CreateManifest("\"sixteen\": \"icon3.png\", \n");
-    ManifestData manifest(std::move(ext_manifest), "test");
+    ManifestData manifest(CreateManifest(R"("sixteen": "icon3.png",)"), "test");
     LoadAndExpectError(manifest, "Invalid key in icons: \"sixteen\".");
   }
   // Just right.
   {
-    std::unique_ptr<base::Value> ext_manifest =
-        CreateManifest("\"512\": \"icon3.png\", \n");
-    ManifestData manifest(std::move(ext_manifest), "test");
+    ManifestData manifest(CreateManifest(R"("512": "icon3.png",)"), "test");
     scoped_refptr<extensions::Extension> extension =
         LoadAndExpectSuccess(manifest);
   }

extensions/common/manifest_handlers/replacement_apps_unittest.cc

@@ -42,8 +42,7 @@ class ReplacementAppsManifestTest : public ManifestTest {
            })";
       base::Value manifest = base::test::ParseJson(base::StringPrintf(
           kManifest, replacement_web_app, replacement_android_app));
-      return ManifestData(base::Value::ToUniquePtrValue(std::move(manifest)),
-                          "test");
+      return ManifestData(std::move(manifest), "test");
     } else if (replacement_web_app != nullptr) {
       // only web replacement app specified
       constexpr char kManifest[] =
@@ -55,8 +54,7 @@ class ReplacementAppsManifestTest : public ManifestTest {
            })";
       base::Value manifest = base::test::ParseJson(
           base::StringPrintf(kManifest, replacement_web_app));
-      return ManifestData(base::Value::ToUniquePtrValue(std::move(manifest)),
-                          "test");
+      return ManifestData(std::move(manifest), "test");
     } else if (replacement_android_app != nullptr) {
       // only Android replacement app specified
       constexpr char kManifest[] =
@@ -73,8 +71,7 @@ class ReplacementAppsManifestTest : public ManifestTest {
             })";
       base::Value manifest = base::test::ParseJson(
           base::StringPrintf(kManifest, replacement_android_app));
-      return ManifestData(base::Value::ToUniquePtrValue(std::move(manifest)),
-                          "test");
+      return ManifestData(std::move(manifest), "test");
     }
 
     base::Value manifest = base::test::ParseJson(
@@ -83,8 +80,7 @@ class ReplacementAppsManifestTest : public ManifestTest {
              "version": "1",
              "manifest_version": 2
            })");
-    return ManifestData(base::Value::ToUniquePtrValue(std::move(manifest)),
-                        "test");
+    return ManifestData(std::move(manifest), "test");
   }
 
  private:

extensions/common/manifest_test.cc

@@ -75,14 +75,6 @@ ManifestTest::ManifestData::ManifestData(base::Value manifest,
 
 ManifestTest::ManifestData::ManifestData(ManifestData&& other) = default;
 
-ManifestTest::ManifestData::ManifestData(base::Value* manifest,
-                                         const char* name)
-    : ManifestData(manifest->Clone(), name) {}
-
-ManifestTest::ManifestData::ManifestData(std::unique_ptr<base::Value> manifest,
-                                         const char* name)
-    : ManifestData(base::Value(std::move(*manifest)), name) {}
-
 ManifestTest::ManifestData::~ManifestData() {
 }
 

extensions/common/manifest_test.h

@@ -39,11 +39,6 @@ class ManifestTest : public testing::Test {
 
     ManifestData(ManifestData&& other);
 
-    // DEPRECATED. Use one of the above constructors.
-    ManifestData(base::Value* manifest, const char* name);
-    explicit ManifestData(std::unique_ptr<base::Value> manifest,
-                          const char* name);
-
     ~ManifestData();
 
     const std::string& name() const { return name_; }