Revert "Change update_client::ReadManifest() to return base::Value."

This reverts commit bc041ac.

Reason for revert: Compile errors, see https://ci.chromium.org/ui/p/chrome/builders/ci/win64-chrome/14513/overview

Original change's description:
> Change update_client::ReadManifest() to return base::Value.
>
> Stop returning base::DictionaryValue, which is deprecated. Then
> ReadManifest() can stop doing the weird std::unique_ptr release() into
> another std::unique_ptr. Then update ComponentInstaller::InstallHelper()
> to use base::Value in its out parameter as well. With base::Value, there
> is no more GetStringASCII() method, so use base::IsStringASCII() in the
> callers.
>
> Bug: 1187036
> Change-Id: I88b77d4c744fd4be23c5691ac31d5be8a35b411b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2891514
> Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
> Commit-Queue: Lei Zhang <thestig@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#882536}

Bug: 1187036
Change-Id: I8b07b993f584d2c598e084cdd5e34a2d0b98973a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2891201
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Kristi Park <kristipark@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Lei Zhang <thestig@chromium.org>
Commit-Queue: Kristi Park <kristipark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#882575}

chrome/updater/installer.cc

@@ -124,15 +124,13 @@ Installer::Result Installer::InstallHelper(
   base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
                                                 base::BlockingType::WILL_BLOCK);
 
-  base::Value local_manifest = update_client::ReadManifest(unpack_path);
-  if (!local_manifest.is_dict())
+  auto local_manifest = update_client::ReadManifest(unpack_path);
+  if (!local_manifest)
     return Result(update_client::InstallError::BAD_MANIFEST);
 
-  const std::string* version_ascii = local_manifest.FindStringKey("version");
-  if (!version_ascii || !base::IsStringASCII(*version_ascii))
-    return Result(update_client::InstallError::INVALID_VERSION);
-
-  const base::Version manifest_version(*version_ascii);
+  std::string version_ascii;
+  local_manifest->GetStringASCII("version", &version_ascii);
+  const base::Version manifest_version(version_ascii);
 
   VLOG(1) << "Installed version=" << pv_
           << ", installing version=" << manifest_version.GetString();

components/component_updater/component_installer.cc

@@ -18,7 +18,6 @@
 #include "base/path_service.h"
 #include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
-#include "base/strings/string_util.h"
 #include "base/task/post_task.h"
 #include "base/task/task_traits.h"
 #include "base/task/thread_pool.h"
@@ -104,19 +103,18 @@ void ComponentInstaller::OnUpdateError(int error) {
   LOG(ERROR) << "Component update error: " << error;
 }
 
-Result ComponentInstaller::InstallHelper(const base::FilePath& unpack_path,
-                                         base::Value* manifest,
-                                         base::Version* version,
-                                         base::FilePath* install_path) {
-  base::Value local_manifest = update_client::ReadManifest(unpack_path);
-  if (!local_manifest.is_dict())
+Result ComponentInstaller::InstallHelper(
+    const base::FilePath& unpack_path,
+    std::unique_ptr<base::DictionaryValue>* manifest,
+    base::Version* version,
+    base::FilePath* install_path) {
+  auto local_manifest = update_client::ReadManifest(unpack_path);
+  if (!local_manifest)
     return Result(InstallError::BAD_MANIFEST);
 
-  const std::string* version_ascii = local_manifest.FindStringKey("version");
-  if (!version_ascii || !base::IsStringASCII(*version_ascii))
-    return Result(InstallError::INVALID_VERSION);
-
-  const base::Version manifest_version(*version_ascii);
+  std::string version_ascii;
+  local_manifest->GetStringASCII("version", &version_ascii);
+  const base::Version manifest_version(version_ascii);
 
   VLOG(1) << "Install: version=" << manifest_version.GetString()
           << " current version=" << current_version_.GetString();
@@ -160,17 +158,14 @@ Result ComponentInstaller::InstallHelper(const base::FilePath& unpack_path,
   DCHECK(!base::PathExists(unpack_path));
   DCHECK(base::PathExists(local_install_path));
 
-  const base::DictionaryValue& local_manifest_dict =
-      base::Value::AsDictionaryValue(local_manifest);
-  const Result result = installer_policy_->OnCustomInstall(local_manifest_dict,
-                                                           local_install_path);
+  const Result result =
+      installer_policy_->OnCustomInstall(*local_manifest, local_install_path);
   if (result.error)
     return result;
 
-  if (!installer_policy_->VerifyInstallation(local_manifest_dict,
-                                             local_install_path)) {
+  if (!installer_policy_->VerifyInstallation(*local_manifest,
+                                             local_install_path))
     return Result(InstallError::INSTALL_VERIFICATION_FAILED);
-  }
 
   *manifest = std::move(local_manifest);
   *version = manifest_version;
@@ -185,7 +180,7 @@ void ComponentInstaller::Install(
     std::unique_ptr<InstallParams> /*install_params*/,
     ProgressCallback /*progress_callback*/,
     Callback callback) {
-  base::Value manifest;
+  std::unique_ptr<base::DictionaryValue> manifest;
   base::Version version;
   base::FilePath install_path;
   const Result result =
@@ -201,10 +196,8 @@ void ComponentInstaller::Install(
   current_install_dir_ = install_path;
 
   main_task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(&ComponentInstaller::ComponentReady, this,
-                     base::DictionaryValue::From(
-                         base::Value::ToUniquePtrValue(std::move(manifest)))));
+      FROM_HERE, base::BindOnce(&ComponentInstaller::ComponentReady, this,
+                                std::move(manifest)));
   main_task_runner_->PostTask(FROM_HERE,
                               base::BindOnce(std::move(callback), result));
 }
@@ -234,22 +227,20 @@ bool ComponentInstaller::FindPreinstallation(
     return false;
   }
 
-  base::Value manifest = update_client::ReadManifest(path);
-  if (!manifest.is_dict()) {
+  std::unique_ptr<base::DictionaryValue> manifest =
+      update_client::ReadManifest(path);
+  if (!manifest) {
     DVLOG(1) << "Manifest does not exist: " << path.MaybeAsASCII();
     return false;
   }
 
-  std::unique_ptr<base::DictionaryValue> manifest_dict =
-      base::DictionaryValue::From(
-          base::Value::ToUniquePtrValue(std::move(manifest)));
-  if (!installer_policy_->VerifyInstallation(*manifest_dict, path)) {
+  if (!installer_policy_->VerifyInstallation(*manifest, path)) {
     DVLOG(1) << "Installation verification failed: " << path.MaybeAsASCII();
     return false;
   }
 
   std::string version_lexical;
-  if (!manifest_dict->GetStringASCII("version", &version_lexical)) {
+  if (!manifest->GetStringASCII("version", &version_lexical)) {
     DVLOG(1) << "Failed to get component version from the manifest.";
     return false;
   }
@@ -266,7 +257,7 @@ bool ComponentInstaller::FindPreinstallation(
 
   registration_info->install_dir = path;
   registration_info->version = version;
-  registration_info->manifest = std::move(manifest_dict);
+  registration_info->manifest = std::move(manifest);
 
   return true;
 }
@@ -343,8 +334,7 @@ void ComponentInstaller::StartRegistration(
     }
 
     std::unique_ptr<base::DictionaryValue> manifest =
-        base::DictionaryValue::From(
-            base::Value::ToUniquePtrValue(update_client::ReadManifest(path)));
+        update_client::ReadManifest(path);
     if (!manifest || !installer_policy_->VerifyInstallation(*manifest, path)) {
       PLOG(ERROR) << "Failed to read manifest or verify installation for "
                   << installer_policy_->GetName() << " (" << path.MaybeAsASCII()

components/component_updater/component_installer.h

@@ -166,7 +166,7 @@ class ComponentInstaller final : public update_client::CrxInstaller {
                            scoped_refptr<RegistrationInfo> registration_info);
   update_client::CrxInstaller::Result InstallHelper(
       const base::FilePath& unpack_path,
-      base::Value* manifest,
+      std::unique_ptr<base::DictionaryValue>* manifest,
       base::Version* version,
       base::FilePath* install_path);
   void StartRegistration(scoped_refptr<RegistrationInfo> registration_info);

components/update_client/test_installer.cc

@@ -11,7 +11,6 @@
 #include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
-#include "base/strings/string_util.h"
 #include "base/task/post_task.h"
 #include "base/task/task_traits.h"
 #include "base/task/thread_pool.h"
@@ -97,12 +96,11 @@ void VersionedTestInstaller::Install(
     std::unique_ptr<InstallParams> /*install_params*/,
     ProgressCallback progress_callback,
     Callback callback) {
-  const base::Value manifest = update_client::ReadManifest(unpack_path);
-  const std::string* version_string = manifest.FindStringKey("version");
-  if (!version_string || !base::IsStringASCII(*version_string))
-    return;
+  const auto manifest = update_client::ReadManifest(unpack_path);
+  std::string version_string;
+  manifest->GetStringASCII("version", &version_string);
+  const base::Version version(version_string);
 
-  const base::Version version(*version_string);
   const base::FilePath path =
       install_directory_.AppendASCII(version.GetString());
   base::CreateDirectory(path.DirName());

components/update_client/utils.cc

@@ -9,8 +9,6 @@
 #include <algorithm>
 #include <cmath>
 #include <cstring>
-#include <memory>
-#include <utility>
 
 #include "base/callback.h"
 #include "base/files/file_path.h"
@@ -156,17 +154,21 @@ CrxInstaller::Result InstallFunctionWrapper(
 // TODO(cpu): add a specific attribute check to a component json that the
 // extension unpacker will reject, so that a component cannot be installed
 // as an extension.
-base::Value ReadManifest(const base::FilePath& unpack_path) {
+std::unique_ptr<base::DictionaryValue> ReadManifest(
+    const base::FilePath& unpack_path) {
   base::FilePath manifest =
       unpack_path.Append(FILE_PATH_LITERAL("manifest.json"));
   if (!base::PathExists(manifest))
-    return base::Value();
+    return nullptr;
   JSONFileValueDeserializer deserializer(manifest);
   std::string error;
   std::unique_ptr<base::Value> root = deserializer.Deserialize(nullptr, &error);
   if (!root)
-    return base::Value();
-  return base::Value::FromUniquePtrValue(std::move(root));
+    return nullptr;
+  if (!root->is_dict())
+    return nullptr;
+  return std::unique_ptr<base::DictionaryValue>(
+      static_cast<base::DictionaryValue*>(root.release()));
 }
 
 }  // namespace update_client

components/update_client/utils.h

@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_UPDATE_CLIENT_UTILS_H_
 #define COMPONENTS_UPDATE_CLIENT_UTILS_H_
 
+#include <memory>
 #include <string>
 #include <utility>
 #include <vector>
@@ -16,8 +17,8 @@
 class GURL;
 
 namespace base {
+class DictionaryValue;
 class FilePath;
-class Value;
 }
 
 namespace update_client {
@@ -71,9 +72,8 @@ CrxInstaller::Result InstallFunctionWrapper(
     base::OnceCallback<bool()> callback);
 
 // Deserializes the CRX manifest. The top level must be a dictionary.
-// Returns a base::Value object of type dictionary on success, or another type
-// on failure.
-base::Value ReadManifest(const base::FilePath& unpack_path);
+std::unique_ptr<base::DictionaryValue> ReadManifest(
+    const base::FilePath& unpack_path);
 
 // Converts a custom, specific installer error (and optionally extended error)
 // to an installer result.