[CodeHealth] UIProxyConfigService base::Value refactoring

This CL provides refactoring in the implementation of
UIProxyConfigService, removing all the use of any deprecated interfaces,
or types from base::Value.

Bug: 1187001
Change-Id: Ie78eefc3406e33868cc42fd535abf11a36b510d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3810753
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Commit-Queue: Claudio DeSouza <cdesouza@igalia.com>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1033521}

chrome/browser/ash/net/network_pref_state_observer_unittest.cc

@@ -8,6 +8,7 @@
 
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
+#include "base/strings/string_util.h"
 #include "chrome/browser/ash/login/users/fake_chrome_user_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/test/base/testing_browser_process.h"
@@ -84,7 +85,7 @@ TEST_F(NetworkPrefStateObserverTest, LoginUser) {
       NetworkHandler::GetUiProxyConfigService();
   ASSERT_TRUE(device_ui_proxy_config_service);
   // There should be no proxy config available.
-  base::Value ui_proxy_config(base::Value::Type::DICTIONARY);
+  base::Value::Dict ui_proxy_config;
   EXPECT_FALSE(device_ui_proxy_config_service->MergeEnforcedProxyConfig(
       kNetworkId, &ui_proxy_config));
 
@@ -95,7 +96,7 @@ TEST_F(NetworkPrefStateObserverTest, LoginUser) {
       NetworkHandler::GetUiProxyConfigService();
   ASSERT_TRUE(profile_ui_proxy_config_service);
   ASSERT_NE(device_ui_proxy_config_service, profile_ui_proxy_config_service);
-  ui_proxy_config = base::Value(base::Value::Type::DICTIONARY);
+  ui_proxy_config = base::Value::Dict();
   EXPECT_FALSE(profile_ui_proxy_config_service->MergeEnforcedProxyConfig(
       kNetworkId, &ui_proxy_config));
 
@@ -108,12 +109,12 @@ TEST_F(NetworkPrefStateObserverTest, LoginUser) {
   base::RunLoop().RunUntilIdle();
 
   // Mode should now be MODE_PAC_SCRIPT.
-  ui_proxy_config = base::Value(base::Value::Type::DICTIONARY);
+  ui_proxy_config = base::Value::Dict();
   EXPECT_TRUE(
       NetworkHandler::GetUiProxyConfigService()->MergeEnforcedProxyConfig(
           kNetworkId, &ui_proxy_config));
-  base::Value* mode = ui_proxy_config.FindPath(
-      {::onc::network_config::kType, ::onc::kAugmentationActiveSetting});
+  base::Value* mode = ui_proxy_config.FindByDottedPath(base::JoinString(
+      {::onc::network_config::kType, ::onc::kAugmentationActiveSetting}, "."));
   ASSERT_TRUE(mode);
   EXPECT_EQ(base::Value(::onc::proxy::kPAC), *mode);
 }

chrome/browser/ash/net/system_proxy_manager.cc

@@ -536,7 +536,7 @@ void SystemProxyManager::OnProxyConfigChanged() {
 bool SystemProxyManager::IsManagedProxyConfigured() {
   DCHECK(NetworkHandler::IsInitialized());
   NetworkHandler* network_handler = NetworkHandler::Get();
-  base::Value proxy_settings(base::Value::Type::DICTIONARY);
+  base::Value::Dict proxy_settings;
 
   // |ui_proxy_config_service| may be missing in tests. If the device is offline
   // (no network connected) the |DefaultNetwork| is null.
@@ -548,7 +548,7 @@ bool SystemProxyManager::IsManagedProxyConfigured() {
         network_handler->network_state_handler()->DefaultNetwork()->guid(),
         &proxy_settings);
   }
-  if (proxy_settings.DictEmpty())
+  if (proxy_settings.empty())
     return false;  // no managed proxy set
 
   if (IsProxyConfiguredByUserViaExtension())

chrome/browser/ui/webui/management/management_ui_handler.cc

@@ -17,7 +17,7 @@
 #include "base/callback_helpers.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/user_metrics.h"
-#include "base/strings/strcat.h"
+#include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
@@ -725,7 +725,7 @@ void ManagementUIHandler::AddProxyServerPrivacyDisclosure(
     base::Value::Dict* response) const {
   bool showProxyDisclosure = false;
   ash::NetworkHandler* network_handler = ash::NetworkHandler::Get();
-  base::Value proxy_settings(base::Value::Type::DICTIONARY);
+  base::Value::Dict proxy_settings;
   // |ui_proxy_config_service| may be missing in tests. If the device is offline
   // (no network connected) the |DefaultNetwork| is null.
   if (ash::NetworkHandler::HasUiProxyConfigService() &&
@@ -736,15 +736,16 @@ void ManagementUIHandler::AddProxyServerPrivacyDisclosure(
         network_handler->network_state_handler()->DefaultNetwork()->guid(),
         &proxy_settings);
   }
-  if (!proxy_settings.DictEmpty()) {
+  if (!proxy_settings.empty()) {
     // Proxies can be specified by web server url, via a PAC script or via the
     // web proxy auto-discovery protocol. Chrome also supports the "direct"
     // mode, in which no proxy is used.
-    base::Value* proxy_specification_mode = proxy_settings.FindPath(
-        {::onc::network_config::kType, ::onc::kAugmentationActiveSetting});
-    showProxyDisclosure =
-        proxy_specification_mode &&
-        proxy_specification_mode->GetString() != ::onc::proxy::kDirect;
+    std::string* proxy_specification_mode =
+        proxy_settings.FindStringByDottedPath(base::JoinString(
+            {::onc::network_config::kType, ::onc::kAugmentationActiveSetting},
+            "."));
+    showProxyDisclosure = proxy_specification_mode &&
+                          *proxy_specification_mode != ::onc::proxy::kDirect;
   }
   response->Set("showProxyServerPrivacyDisclosure", showProxyDisclosure);
 }

chromeos/ash/components/network/managed_network_configuration_handler_impl.cc

@@ -327,17 +327,18 @@ void ManagedNetworkConfigurationHandlerImpl::SetManagedActiveProxyValues(
     base::Value* dictionary) {
   DCHECK(ui_proxy_config_service_);
   const std::string proxy_settings_key = ::onc::network_config::kProxySettings;
-  base::Value* proxy_settings = dictionary->FindKeyOfType(
-      proxy_settings_key, base::Value::Type::DICTIONARY);
+  base::Value::Dict* proxy_settings =
+      dictionary->GetDict().FindDict(proxy_settings_key);
 
   if (!proxy_settings) {
-    proxy_settings = dictionary->SetKey(
-        proxy_settings_key, base::Value(base::Value::Type::DICTIONARY));
+    proxy_settings = dictionary->GetDict()
+                         .Set(proxy_settings_key, base::Value::Dict())
+                         ->GetIfDict();
   }
   ui_proxy_config_service_->MergeEnforcedProxyConfig(guid, proxy_settings);
 
-  if (proxy_settings->DictEmpty())
-    dictionary->RemoveKey(proxy_settings_key);
+  if (proxy_settings->empty())
+    dictionary->GetDict().Remove(proxy_settings_key);
 }
 
 void ManagedNetworkConfigurationHandlerImpl::SetShillProperties(

chromeos/ash/components/network/proxy/ui_proxy_config_service.cc

@@ -10,7 +10,7 @@
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/logging.h"
-#include "base/values.h"
+#include "base/strings/string_util.h"
 #include "chromeos/ash/components/network/network_event_log.h"
 #include "chromeos/ash/components/network/network_state.h"
 #include "chromeos/ash/components/network/network_state_handler.h"
@@ -70,77 +70,79 @@ std::string EffectiveConfigStateToOncSourceString(
   return std::string();
 }
 
-base::Value CreateEffectiveValue(const std::string& source, base::Value value) {
-  base::Value dict(base::Value::Type::DICTIONARY);
-  dict.SetKey(::onc::kAugmentationEffectiveSetting, base::Value(source));
+template <typename T>
+base::Value::Dict CreateEffectiveValue(const std::string& source, T value) {
+  base::Value::Dict dict;
+  dict.Set(::onc::kAugmentationEffectiveSetting, source);
   // ActiveExtension is a special source type indicating that the Effective
   // value is the Active value and was set by an extension. It does not provide
   // a separate value.
   if (source != ::onc::kAugmentationActiveExtension) {
-    dict.SetKey(source, value.Clone());
+    dict.Set(source, value.Clone());
   }
-  dict.SetKey(::onc::kAugmentationActiveSetting, std::move(value));
-  dict.SetKey(::onc::kAugmentationUserEditable, base::Value(false));
+  dict.Set(::onc::kAugmentationActiveSetting, std::move(value));
+  dict.Set(::onc::kAugmentationUserEditable, false);
   return dict;
 }
 
-void SetManualProxy(base::Value* manual,
+void SetManualProxy(base::Value::Dict* manual,
                     const std::string& source,
                     const std::string& key,
                     const net::ProxyList& proxy_list) {
   if (proxy_list.IsEmpty()) {
-    manual->SetPath({key, ::onc::proxy::kHost},
-                    CreateEffectiveValue(source, base::Value("")));
-    manual->SetPath({key, ::onc::proxy::kPort},
-                    CreateEffectiveValue(source, base::Value(0)));
+    manual->SetByDottedPath(base::JoinString({key, ::onc::proxy::kHost}, "."),
+                            CreateEffectiveValue(source, base::Value("")));
+    manual->SetByDottedPath(base::JoinString({key, ::onc::proxy::kPort}, "."),
+                            CreateEffectiveValue(source, base::Value(0)));
     return;
   }
 
   const net::ProxyServer& proxy = proxy_list.Get();
-  manual->SetPath(
-      {key, ::onc::proxy::kHost},
+  manual->SetByDottedPath(
+      base::JoinString({key, ::onc::proxy::kHost}, "."),
       CreateEffectiveValue(source, base::Value(proxy.host_port_pair().host())));
-  manual->SetPath(
-      {key, ::onc::proxy::kPort},
+  manual->SetByDottedPath(
+      base::JoinString({key, ::onc::proxy::kPort}, "."),
       CreateEffectiveValue(source, base::Value(proxy.host_port_pair().port())));
 }
 
-base::Value OncValueWithMode(const std::string& source,
-                             const std::string& mode) {
-  base::Value result(base::Value::Type::DICTIONARY);
-  result.SetKey(::onc::network_config::kType,
-                CreateEffectiveValue(source, base::Value(mode)));
+base::Value::Dict OncValueWithMode(const std::string& source,
+                                   const std::string& mode) {
+  base::Value::Dict result;
+  result.Set(::onc::network_config::kType,
+             CreateEffectiveValue(source, base::Value(mode)));
   return result;
 }
 
-base::Value OncValueForManualProxyList(
+absl::optional<base::Value::Dict> OncValueForManualProxyList(
     const std::string& source,
     const net::ProxyList& for_http,
     const net::ProxyList& for_https,
     const net::ProxyList& fallback,
     const net::ProxyBypassRules& bypass_rules) {
   if (for_http.IsEmpty() && for_https.IsEmpty() && fallback.IsEmpty()) {
-    return base::Value();
+    return absl::nullopt;
   }
-  base::Value result = OncValueWithMode(source, ::onc::proxy::kManual);
+  base::Value::Dict result = OncValueWithMode(source, ::onc::proxy::kManual);
 
-  base::Value* manual = result.SetKey(
-      ::onc::proxy::kManual, base::Value(base::Value::Type::DICTIONARY));
+  base::Value::Dict* manual =
+      result.Set(::onc::proxy::kManual, base::Value::Dict())->GetIfDict();
   SetManualProxy(manual, source, ::onc::proxy::kHttp, for_http);
   SetManualProxy(manual, source, ::onc::proxy::kHttps, for_https);
   SetManualProxy(manual, source, ::onc::proxy::kSocks, fallback);
 
-  base::Value exclude_domains(base::Value::Type::LIST);
+  base::Value::List exclude_domains;
   for (const auto& rule : bypass_rules.rules())
     exclude_domains.Append(rule->ToString());
-  result.SetKey(::onc::proxy::kExcludeDomains,
-                CreateEffectiveValue(source, std::move(exclude_domains)));
+  result.Set(::onc::proxy::kExcludeDomains,
+             CreateEffectiveValue(source, std::move(exclude_domains)));
 
   return result;
 }
 
-base::Value OncValueForEmptyProxyRules(const net::ProxyConfig& net_config,
-                                       const std::string& source) {
+absl::optional<base::Value::Dict> OncValueForEmptyProxyRules(
+    const net::ProxyConfig& net_config,
+    const std::string& source) {
   if (!net_config.HasAutomaticSettings()) {
     return OncValueWithMode(source, ::onc::proxy::kDirect);
   }
@@ -150,18 +152,19 @@ base::Value OncValueForEmptyProxyRules(const net::ProxyConfig& net_config,
   }
 
   if (net_config.has_pac_url()) {
-    base::Value result = OncValueWithMode(source, ::onc::proxy::kPAC);
-    result.SetKey(
+    base::Value::Dict result = OncValueWithMode(source, ::onc::proxy::kPAC);
+    result.Set(
         ::onc::proxy::kPAC,
         CreateEffectiveValue(source, base::Value(net_config.pac_url().spec())));
     return result;
   }
 
-  return base::Value();
+  return absl::nullopt;
 }
 
-base::Value NetProxyConfigAsOncValue(const net::ProxyConfig& net_config,
-                                     const std::string& source) {
+absl::optional<base::Value::Dict> NetProxyConfigAsOncValue(
+    const net::ProxyConfig& net_config,
+    const std::string& source) {
   switch (net_config.proxy_rules().type) {
     case net::ProxyConfig::ProxyRules::Type::EMPTY:
       return OncValueForEmptyProxyRules(net_config, source);
@@ -178,7 +181,7 @@ base::Value NetProxyConfigAsOncValue(const net::ProxyConfig& net_config,
           net_config.proxy_rules().fallback_proxies,
           net_config.proxy_rules().bypass_rules);
   }
-  return base::Value();
+  return absl::nullopt;
 }
 
 }  // namespace
@@ -215,11 +218,11 @@ UIProxyConfigService::~UIProxyConfigService() = default;
 
 bool UIProxyConfigService::MergeEnforcedProxyConfig(
     const std::string& network_guid,
-    base::Value* proxy_settings) {
+    base::Value::Dict* proxy_settings) {
   current_ui_network_guid_ = network_guid;
   const NetworkState* network = nullptr;
   DCHECK(!network_guid.empty());
-  DCHECK(proxy_settings->is_dict());
+  DCHECK(proxy_settings);
   DCHECK(network_state_handler_);
 
   network = network_state_handler_->GetNetworkStateFromGuid(network_guid);
@@ -273,12 +276,12 @@ bool UIProxyConfigService::MergeEnforcedProxyConfig(
   if (source.empty())
     return false;
 
-  base::Value enforced_settings =
+  absl::optional<base::Value::Dict> enforced_settings =
       NetProxyConfigAsOncValue(effective_config.value(), source);
-  if (enforced_settings.is_none())
+  if (!enforced_settings)
     return false;
 
-  proxy_settings->MergeDictionary(&enforced_settings);
+  proxy_settings->Merge(std::move(*enforced_settings));
   return true;
 }
 
@@ -297,9 +300,6 @@ ProxyPrefs::ProxyMode UIProxyConfigService::ProxyModeForNetwork(
       proxy_config::GetProxyConfigForNetwork(nullptr, local_state_prefs_,
                                              *network, network_profile_handler_,
                                              &onc_source);
-  base::Value proxy_settings(base::Value::Type::DICTIONARY);
-  if (proxy_dict)
-    proxy_settings = proxy_dict->GetDictionary().Clone();
 
   PrefService* top_pref_service =
       profile_prefs_ ? profile_prefs_ : local_state_prefs_;

chromeos/ash/components/network/proxy/ui_proxy_config_service.h

@@ -8,15 +8,12 @@
 #include <string>
 
 #include "base/component_export.h"
+#include "base/values.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "components/proxy_config/proxy_prefs.h"
 
 class PrefService;
 
-namespace base {
-class Value;
-}
-
 namespace ash {
 
 class NetworkProfileHandler;
@@ -61,7 +58,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) UIProxyConfigService {
   //
   // Returns whether |proxy_settings| have been changed.
   bool MergeEnforcedProxyConfig(const std::string& network_guid,
-                                base::Value* proxy_settings);
+                                base::Value::Dict* proxy_settings);
 
   // Returns true if there is a default network and it has a proxy configuration
   // with mode == MODE_FIXED_SERVERS.