Revert "[CodeHealth] Settings WebUI base::Value refactoring - Part 1"

This reverts commit f0e6d42.

Reason for revert: This cl could break lacros compliation

Original change's description:
> [CodeHealth] Settings WebUI base::Value refactoring - Part 1
>
> This CL corrects the use of base value for the webui handlers of a
> couple of files under webui/settings.
>
> Bug: 1187001
> Change-Id: I4dc51df9ae48773d6e1fee292f7e7ba1dd9e83ba
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3810934
> Reviewed-by: Aga Wronska <agawronska@chromium.org>
> Commit-Queue: Claudio DeSouza <cdesouza@igalia.com>
> Reviewed-by: Giovanni Ortuno Urquidi <ortuno@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1033140}

Bug: 1187001
Change-Id: Id6b1acdd1d70372e25d00ba2b3eea93bfd624339
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3821562
Reviewed-by: Thanh Nguyen <thanhdng@chromium.org>
Owners-Override: Thanh Nguyen <thanhdng@chromium.org>
Commit-Queue: Thanh Nguyen <thanhdng@chromium.org>
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1033254}

chrome/browser/ui/webui/family_link_user_internals/family_link_user_internals_message_handler.cc

@@ -9,7 +9,6 @@
 
 #include "base/bind.h"
 #include "base/memory/ref_counted.h"
-#include "base/strings/string_piece.h"
 #include "base/values.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_key.h"
@@ -37,38 +36,38 @@ namespace {
 // consisting of a title and a list of fields. Returns a pointer to the new
 // section's contents, for use with |AddSectionEntry| below. Note that
 // |parent_list|, not the caller, owns the newly added section.
-base::Value::List* AddSection(base::Value::List* parent_list,
-                              base::StringPiece title) {
-  base::Value::Dict section;
-  base::Value::List section_contents;
-  section.Set("title", title);
+base::ListValue* AddSection(base::ListValue* parent_list,
+                            const std::string& title) {
+  std::unique_ptr<base::DictionaryValue> section(new base::DictionaryValue);
+  std::unique_ptr<base::ListValue> section_contents(new base::ListValue);
+  section->SetStringKey("title", title);
   // Grab a raw pointer to the result before |Pass()|ing it on.
-  base::Value::List* result =
-      section.Set("data", std::move(section_contents))->GetIfList();
-  parent_list->Append(std::move(section));
+  base::ListValue* result =
+      section->SetList("data", std::move(section_contents));
+  parent_list->Append(base::Value::FromUniquePtrValue(std::move(section)));
   return result;
 }
 
 // Adds a bool entry to a section (created with |AddSection| above).
-void AddSectionEntry(base::Value::List* section_list,
-                     base::StringPiece name,
+void AddSectionEntry(base::ListValue* section_list,
+                     const std::string& name,
                      bool value) {
   base::Value::Dict entry;
   entry.Set("stat_name", name);
   entry.Set("stat_value", value);
   entry.Set("is_valid", true);
-  section_list->Append(std::move(entry));
+  section_list->GetList().Append(std::move(entry));
 }
 
 // Adds a string entry to a section (created with |AddSection| above).
-void AddSectionEntry(base::Value::List* section_list,
-                     base::StringPiece name,
-                     base::StringPiece value) {
+void AddSectionEntry(base::ListValue* section_list,
+                     const std::string& name,
+                     const std::string& value) {
   base::Value::Dict entry;
   entry.Set("stat_name", name);
   entry.Set("stat_value", value);
   entry.Set("is_valid", true);
-  section_list->Append(std::move(entry));
+  section_list->GetList().Append(std::move(entry));
 }
 
 std::string FilteringBehaviorToString(
@@ -203,21 +202,21 @@ void FamilyLinkUserInternalsMessageHandler::HandleTryURL(
 }
 
 void FamilyLinkUserInternalsMessageHandler::SendBasicInfo() {
-  base::Value::List section_list;
+  base::ListValue section_list;
 
-  base::Value::List* section_general = AddSection(&section_list, "General");
+  base::ListValue* section_general = AddSection(&section_list, "General");
   AddSectionEntry(section_general, "Child detection enabled",
                   ChildAccountService::IsChildAccountDetectionEnabled());
 
   Profile* profile = Profile::FromWebUI(web_ui());
 
-  base::Value::List* section_profile = AddSection(&section_list, "Profile");
+  base::ListValue* section_profile = AddSection(&section_list, "Profile");
   AddSectionEntry(section_profile, "Account", profile->GetProfileUserName());
   AddSectionEntry(section_profile, "Child", profile->IsChild());
 
   SupervisedUserURLFilter* filter = GetSupervisedUserService()->GetURLFilter();
 
-  base::Value::List* section_filter = AddSection(&section_list, "Filter");
+  base::ListValue* section_filter = AddSection(&section_list, "Filter");
   AddSectionEntry(section_filter, "Denylist active", filter->HasDenylist());
   AddSectionEntry(section_filter, "Online checks active",
                   filter->HasAsyncURLChecker());
@@ -232,7 +231,7 @@ void FamilyLinkUserInternalsMessageHandler::SendBasicInfo() {
     for (const auto& account :
          identity_manager
              ->GetExtendedAccountInfoForAccountsWithRefreshToken()) {
-      base::Value::List* section_user = AddSection(
+      base::ListValue* section_user = AddSection(
           &section_list, "User Information for " + account.full_name);
       AddSectionEntry(section_user, "Account id",
                       account.account_id.ToString());
@@ -247,8 +246,8 @@ void FamilyLinkUserInternalsMessageHandler::SendBasicInfo() {
     }
   }
 
-  base::Value::Dict result;
-  result.Set("sections", std::move(section_list));
+  base::DictionaryValue result;
+  result.SetKey("sections", std::move(section_list));
   FireWebUIListener("basic-info-received", result);
 
   // Trigger retrieval of the user settings
@@ -272,10 +271,11 @@ void FamilyLinkUserInternalsMessageHandler::OnTryURLResult(
     SupervisedUserURLFilter::FilteringBehavior behavior,
     supervised_user_error_page::FilteringBehaviorReason reason,
     bool uncertain) {
-  base::Value::Dict result;
-  result.Set("allowResult", FilteringBehaviorToString(behavior, uncertain));
-  result.Set("manual", reason == supervised_user_error_page::MANUAL &&
-                           behavior == SupervisedUserURLFilter::ALLOW);
+  base::DictionaryValue result;
+  result.SetStringKey("allowResult",
+                      FilteringBehaviorToString(behavior, uncertain));
+  result.SetBoolKey("manual", reason == supervised_user_error_page::MANUAL &&
+                                  behavior == SupervisedUserURLFilter::ALLOW);
   ResolveJavascriptCallback(base::Value(callback_id), result);
 }
 
@@ -287,9 +287,9 @@ void FamilyLinkUserInternalsMessageHandler::OnURLChecked(
     supervised_user_error_page::FilteringBehaviorReason reason,
     bool uncertain) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  base::Value::Dict result;
-  result.Set("url", url.possibly_invalid_spec());
-  result.Set("result", FilteringBehaviorToString(behavior, uncertain));
-  result.Set("reason", FilteringBehaviorReasonToString(reason));
+  base::DictionaryValue result;
+  result.SetStringKey("url", url.possibly_invalid_spec());
+  result.SetStringKey("result", FilteringBehaviorToString(behavior, uncertain));
+  result.SetStringKey("reason", FilteringBehaviorReasonToString(reason));
   FireWebUIListener("filtering-result-received", result);
 }

chrome/browser/ui/webui/settings/about_handler.cc

@@ -22,6 +22,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/thread_pool.h"
 #include "base/time/default_clock.h"
+#include "base/values.h"
 #include "build/branding_buildflags.h"
 #include "build/build_config.h"
 #include "build/chromeos_buildflags.h"
@@ -200,12 +201,16 @@ std::string ReadRegulatoryLabelText(const base::FilePath& label_dir_path) {
   return std::string();
 }
 
-base::Value::Dict GetVersionInfo() {
-  base::Value::Dict version_info;
-  version_info.Set("osVersion", chromeos::version_loader::GetVersion(
-                                    chromeos::version_loader::VERSION_FULL));
-  version_info.Set("arcVersion", chromeos::version_loader::GetARCVersion());
-  version_info.Set("osFirmware", chromeos::version_loader::GetFirmware());
+std::unique_ptr<base::DictionaryValue> GetVersionInfo() {
+  std::unique_ptr<base::DictionaryValue> version_info(
+      new base::DictionaryValue);
+  version_info->SetStringKey("osVersion",
+                             chromeos::version_loader::GetVersion(
+                                 chromeos::version_loader::VERSION_FULL));
+  version_info->SetStringKey("arcVersion",
+                             chromeos::version_loader::GetARCVersion());
+  version_info->SetStringKey("osFirmware",
+                             chromeos::version_loader::GetFirmware());
   return version_info;
 }
 
@@ -537,9 +542,10 @@ void AboutHandler::HandleGetVersionInfo(const base::Value::List& args) {
                      weak_factory_.GetWeakPtr(), callback_id));
 }
 
-void AboutHandler::OnGetVersionInfoReady(std::string callback_id,
-                                         base::Value::Dict version_info) {
-  ResolveJavascriptCallback(base::Value(callback_id), version_info);
+void AboutHandler::OnGetVersionInfoReady(
+    std::string callback_id,
+    std::unique_ptr<base::DictionaryValue> version_info) {
+  ResolveJavascriptCallback(base::Value(callback_id), *version_info);
 }
 
 void AboutHandler::HandleGetFirmwareUpdateCount(const base::Value::List& args) {
@@ -591,18 +597,19 @@ void AboutHandler::OnGetCurrentChannel(std::string callback_id,
 void AboutHandler::OnGetTargetChannel(std::string callback_id,
                                       const std::string& current_channel,
                                       const std::string& target_channel) {
-  base::Value::Dict channel_info;
-  channel_info.Set("currentChannel", current_channel);
-  channel_info.Set("targetChannel", target_channel);
+  std::unique_ptr<base::DictionaryValue> channel_info(
+      new base::DictionaryValue);
+  channel_info->SetStringKey("currentChannel", current_channel);
+  channel_info->SetStringKey("targetChannel", target_channel);
 
   // For the LTS pilot simply check whether the device policy is set and ignore
   // its value.
   std::string value;
   bool is_lts =
       ash::CrosSettings::Get()->GetString(ash::kReleaseLtsTag, &value);
-  channel_info.Set("isLts", is_lts);
+  channel_info->SetBoolKey("isLts", is_lts);
 
-  ResolveJavascriptCallback(base::Value(callback_id), channel_info);
+  ResolveJavascriptCallback(base::Value(callback_id), *channel_info);
 }
 
 void AboutHandler::HandleApplyDeferredUpdate(const base::Value::List& args) {
@@ -643,9 +650,9 @@ void AboutHandler::HandleRefreshTPMFirmwareUpdateStatus(
 
 void AboutHandler::RefreshTPMFirmwareUpdateStatus(
     const std::set<ash::tpm_firmware_update::Mode>& modes) {
-  base::Value::Dict event;
-  event.Set("updateAvailable", !modes.empty());
-  FireWebUIListener("tpm-firmware-update-status-changed", event);
+  std::unique_ptr<base::DictionaryValue> event(new base::DictionaryValue);
+  event->SetBoolKey("updateAvailable", !modes.empty());
+  FireWebUIListener("tpm-firmware-update-status-changed", *event);
 }
 
 void AboutHandler::HandleGetEndOfLifeInfo(const base::Value::List& args) {
@@ -659,14 +666,14 @@ void AboutHandler::HandleGetEndOfLifeInfo(const base::Value::List& args) {
 void AboutHandler::OnGetEndOfLifeInfo(
     std::string callback_id,
     ash::UpdateEngineClient::EolInfo eol_info) {
-  base::Value::Dict response;
+  base::Value response(base::Value::Type::DICTIONARY);
   if (!eol_info.eol_date.is_null()) {
     bool has_eol_passed = eol_info.eol_date <= clock_->Now();
-    response.Set("hasEndOfLife", has_eol_passed);
+    response.SetBoolKey("hasEndOfLife", has_eol_passed);
     int eol_string_id =
         has_eol_passed ? IDS_SETTINGS_ABOUT_PAGE_END_OF_LIFE_MESSAGE_PAST
                        : IDS_SETTINGS_ABOUT_PAGE_END_OF_LIFE_MESSAGE_FUTURE;
-    response.Set(
+    response.SetStringKey(
         "aboutPageEndOfLifeMessage",
         l10n_util::GetStringFUTF16(
             eol_string_id,
@@ -675,8 +682,8 @@ void AboutHandler::OnGetEndOfLifeInfo(
             base::ASCIIToUTF16(has_eol_passed ? chrome::kEolNotificationURL
                                               : chrome::kAutoUpdatePolicyURL)));
   } else {
-    response.Set("hasEndOfLife", false);
-    response.Set("aboutPageEndOfLifeMessage", "");
+    response.SetBoolKey("hasEndOfLife", false);
+    response.SetStringKey("aboutPageEndOfLifeMessage", "");
   }
   ResolveJavascriptCallback(base::Value(callback_id), response);
 }
@@ -748,29 +755,29 @@ void AboutHandler::SetUpdateStatus(VersionUpdater::Status status,
   // Only UPDATING state should have progress set.
   DCHECK(status == VersionUpdater::UPDATING || progress == 0);
 
-  base::Value::Dict event;
-  event.Set("status", UpdateStatusToString(status));
-  event.Set("message", message);
-  event.Set("progress", progress);
-  event.Set("rollback", rollback);
-  event.Set("powerwash", powerwash);
-  event.Set("version", version);
+  std::unique_ptr<base::DictionaryValue> event(new base::DictionaryValue);
+  event->SetStringKey("status", UpdateStatusToString(status));
+  event->SetStringKey("message", message);
+  event->SetIntKey("progress", progress);
+  event->SetBoolKey("rollback", rollback);
+  event->SetBoolKey("powerwash", powerwash);
+  event->SetStringKey("version", version);
   // DictionaryValue does not support int64_t, so convert to string.
-  event.Set("size", base::NumberToString(size));
+  event->SetStringKey("size", base::NumberToString(size));
 #if BUILDFLAG(IS_CHROMEOS_ASH)
   if (status == VersionUpdater::FAILED_OFFLINE ||
       status == VersionUpdater::FAILED_CONNECTION_TYPE_DISALLOWED) {
     std::u16string types_msg = GetAllowedConnectionTypesMessage();
     if (!types_msg.empty())
-      event.Set("connectionTypes", types_msg);
+      event->SetStringKey("connectionTypes", types_msg);
     else
-      event.Set("connectionTypes", base::Value());
+      event->Set("connectionTypes", std::make_unique<base::Value>());
   } else {
-    event.Set("connectionTypes", base::Value());
+    event->Set("connectionTypes", std::make_unique<base::Value>());
   }
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 
-  FireWebUIListener("update-status-changed", event);
+  FireWebUIListener("update-status-changed", *event);
 }
 
 #if BUILDFLAG(IS_MAC)
@@ -790,12 +797,12 @@ void AboutHandler::SetPromotionState(VersionUpdater::PromotionState state) {
   else if (state == VersionUpdater::PROMOTED)
     text = l10n_util::GetStringUTF16(IDS_ABOUT_CHROME_AUTOUPDATE_ALL_IS_ON);
 
-  base::Value::Dict promo_state;
-  promo_state.Set("hidden", hidden);
-  promo_state.Set("disabled", disabled);
-  promo_state.Set("actionable", actionable);
+  base::DictionaryValue promo_state;
+  promo_state.SetBoolKey("hidden", hidden);
+  promo_state.SetBoolKey("disabled", disabled);
+  promo_state.SetBoolKey("actionable", actionable);
   if (!text.empty())
-    promo_state.Set("text", text);
+    promo_state.SetStringKey("text", text);
 
   FireWebUIListener("promotion-state-changed", promo_state);
 }
@@ -821,17 +828,19 @@ void AboutHandler::OnRegulatoryLabelTextRead(
     std::string callback_id,
     const base::FilePath& label_dir_path,
     const std::string& text) {
-  base::Value::Dict regulatory_info;
+  std::unique_ptr<base::DictionaryValue> regulatory_info(
+      new base::DictionaryValue);
   // Remove unnecessary whitespace.
-  regulatory_info.Set("text", base::CollapseWhitespaceASCII(text, true));
+  regulatory_info->SetStringKey("text",
+                                base::CollapseWhitespaceASCII(text, true));
 
   std::string image_path =
       label_dir_path.AppendASCII(kRegulatoryLabelImageFilename).MaybeAsASCII();
   std::string url =
       std::string("chrome://") + chrome::kChromeOSAssetHost + "/" + image_path;
-  regulatory_info.Set("url", url);
+  regulatory_info->SetStringKey("url", url);
 
-  ResolveJavascriptCallback(base::Value(callback_id), regulatory_info);
+  ResolveJavascriptCallback(base::Value(callback_id), *regulatory_info);
 }
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 

chrome/browser/ui/webui/settings/about_handler.h

@@ -10,7 +10,6 @@
 
 #include "base/memory/raw_ptr.h"
 #include "base/memory/weak_ptr.h"
-#include "base/values.h"
 #include "build/build_config.h"
 #include "build/chromeos_buildflags.h"
 #include "chrome/browser/ui/webui/help/version_updater.h"
@@ -25,6 +24,7 @@
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 
 namespace base {
+class DictionaryValue;
 class FilePath;
 class Clock;
 }  // namespace base
@@ -102,8 +102,9 @@ class AboutHandler : public settings::SettingsPageUIHandler,
 
   // Retrieves OS, ARC and firmware versions.
   void HandleGetVersionInfo(const base::Value::List& args);
-  void OnGetVersionInfoReady(std::string callback_id,
-                             base::Value::Dict version_info);
+  void OnGetVersionInfoReady(
+      std::string callback_id,
+      std::unique_ptr<base::DictionaryValue> version_info);
 
   // Retrieves the number of firmware updates available.
   void HandleGetFirmwareUpdateCount(const base::Value::List& args);

chrome/browser/ui/webui/settings/chrome_cleanup_handler_win.cc

@@ -61,13 +61,14 @@ base::Value::List GetStringSetAsListStorage(
   return value;
 }
 
-base::Value::Dict GetScannerResultsAsDictionary(
+base::DictionaryValue GetScannerResultsAsDictionary(
     const safe_browsing::ChromeCleanerScannerResults& scanner_results,
     Profile* profile) {
-  base::Value::Dict value;
-  value.Set("files", GetFilesAsListStorage(scanner_results.files_to_delete()));
-  value.Set("registryKeys",
-            GetStringSetAsListStorage(scanner_results.registry_keys()));
+  base::DictionaryValue value;
+  value.GetDict().Set("files",
+                      GetFilesAsListStorage(scanner_results.files_to_delete()));
+  value.GetDict().Set("registryKeys", GetStringSetAsListStorage(
+                                          scanner_results.registry_keys()));
   return value;
 }