[Code Health] Migrate ProxyConfigDictionary to base::Value::Dict

This cl migrates ProxyConfigDictionary (see
components/proxy_config/proxy_config_dictionary.h) to use
base::Value::Dict, most of the changes are directly propagated from
there. Besides that it expands TestingPrefServiceBase (see
components/prefs/testing_pref_service.h) interface with setters taking
base::Value::Dict and base::Value::List which allows to avoid manual
conversion to the base::Value on the call side in many tests.

LOW_COVERAGE_REASON=Code Health type migration, nothing new to test

Bug: 1187061, 1187001
Change-Id: I0dbca1a78b3c0a4f313f364f3d2fc35378ccf273
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3922202
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Andrey Davydov <andreydav@google.com>
Cr-Commit-Position: refs/heads/main@{#1053030}

chrome/browser/ash/arc/intent_helper/arc_settings_service.cc

@@ -102,7 +102,7 @@ bool GetHttpProxyServer(const ProxyConfigDictionary* proxy_config_dict,
 }
 
 bool IsProxyAutoDetectionConfigured(const base::Value& proxy_config_dict) {
-  ProxyConfigDictionary dict(proxy_config_dict.Clone());
+  ProxyConfigDictionary dict(proxy_config_dict.GetDict().Clone());
   ProxyPrefs::ProxyMode mode;
   dict.GetMode(&mode);
   return mode == ProxyPrefs::MODE_AUTO_DETECT;

chrome/browser/ash/crosapi/network_settings_service_ash.cc

@@ -126,9 +126,10 @@ void NetworkSettingsServiceAsh::SetExtensionProxy(
   pref_service->Set(ash::prefs::kLacrosProxyControllingExtension,
                     std::move(proxy_extension));
 
-  pref_service->Set(
-      proxy_config::prefs::kProxy,
-      CrosapiProxyToProxyConfig(std::move(proxy_config)).GetDictionary());
+  pref_service->SetDict(proxy_config::prefs::kProxy,
+                        CrosapiProxyToProxyConfig(std::move(proxy_config))
+                            .GetDictionary()
+                            .Clone());
 }
 
 void NetworkSettingsServiceAsh::ClearExtensionProxy() {

chrome/browser/ash/crosapi/network_settings_service_ash_browsertest.cc

@@ -210,7 +210,8 @@ class NetworkSettingsServiceAshExtensionTest
  protected:
   // This method simulates sending an extension controlled proxy config from
   // Lacros to Ash.
-  void SendExtensionProxyConfig(base::Value proxy_dict, bool can_be_disabled) {
+  void SendExtensionProxyConfig(base::Value::Dict proxy_dict,
+                                bool can_be_disabled) {
     ProxyConfigDictionary proxy_config_dict(std::move(proxy_dict));
     auto proxy_config =
         crosapi::ProxyConfigToCrosapiProxy(&proxy_config_dict,
@@ -248,8 +249,8 @@ IN_PROC_BROWSER_TEST_F(NetworkSettingsServiceAshExtensionTest,
   ASSERT_TRUE(extension_proxy_pref);
 
   // Emulate receiving an initial proxy config from lacros-chrome.
-  base::Value proxy_config = ProxyConfigDictionary::CreatePacScript(
-      kPacUrl, /*is_pac_mandatory=*/true);
+  base::Value::Dict proxy_config =
+      ProxyConfigDictionary::CreatePacScript(kPacUrl, /*pac_mandatory=*/true);
   SendExtensionProxyConfig(proxy_config.Clone(),
                            /*can_be_disabled=*/true);
   EXPECT_EQ(*(proxy_pref->GetValue()), proxy_config);
@@ -286,8 +287,8 @@ IN_PROC_BROWSER_TEST_F(NetworkSettingsServiceAshExtensionTest,
 // via policy and then verifies that the direct proxy is applied.
 IN_PROC_BROWSER_TEST_F(NetworkSettingsServiceAshExtensionTest,
                        UserPolicyHasPrecedence) {
-  base::Value pac_proxy = ProxyConfigDictionary::CreatePacScript(
-      kPacUrl, /*is_pac_mandatory=*/true);
+  base::Value::Dict pac_proxy =
+      ProxyConfigDictionary::CreatePacScript(kPacUrl, /*pac_mandatory=*/true);
   SendExtensionProxyConfig(pac_proxy.Clone(),
                            /*can_be_disabled=*/true);
   // Set proxy by policy.
@@ -323,7 +324,7 @@ IN_PROC_BROWSER_TEST_F(NetworkSettingsServiceAshExtensionTest,
 
   ProxyConfigDictionary proxy_config_dict(
       ProxyConfigDictionary::CreatePacScript(kPacUrl,
-                                             /*is_pac_mandatory=*/true));
+                                             /*pac_mandatory=*/true));
   auto proxy_config = crosapi::ProxyConfigToCrosapiProxy(&proxy_config_dict,
                                                          /*wpad_url=*/GURL(""));
   proxy_config->extension = crosapi::mojom::ExtensionControllingProxy::New();
@@ -350,8 +351,8 @@ IN_PROC_BROWSER_TEST_F(NetworkSettingsServiceAshExtensionTest,
   {
     base::RunLoop run_loop;
     observer_->SetQuitClosure(run_loop.QuitClosure());
-    base::Value pac_proxy = ProxyConfigDictionary::CreatePacScript(
-        kPacUrl, /*is_pac_mandatory=*/true);
+    base::Value::Dict pac_proxy =
+        ProxyConfigDictionary::CreatePacScript(kPacUrl, /*pac_mandatory=*/true);
     SendExtensionProxyConfig(pac_proxy.Clone(),
                              /*can_be_disabled=*/true);
     // Wait for the `observer` to get the proxy
@@ -379,8 +380,8 @@ IN_PROC_BROWSER_TEST_F(NetworkSettingsServiceAshExtensionTest,
   {
     base::RunLoop run_loop;
     observer_->SetQuitClosure(run_loop.QuitClosure());
-    base::Value pac_proxy = ProxyConfigDictionary::CreatePacScript(
-        kPacUrl, /*is_pac_mandatory=*/true);
+    base::Value::Dict pac_proxy =
+        ProxyConfigDictionary::CreatePacScript(kPacUrl, /*pac_mandatory=*/true);
     SendExtensionProxyConfig(pac_proxy.Clone(),
                              /*can_be_disabled=*/true);
     // Wait for the `observer` to get the proxy

chrome/browser/ash/crosapi/network_settings_translation_unittest.cc

@@ -14,12 +14,13 @@ namespace {
 
 constexpr char kPacUrl[] = "http://pac.pac/";
 
-base::Value GetPacProxyConfig(const std::string& pac_url, bool pac_mandatory) {
+base::Value::Dict GetPacProxyConfig(const std::string& pac_url,
+                                    bool pac_mandatory) {
   return ProxyConfigDictionary::CreatePacScript(pac_url, pac_mandatory);
 }
 
-base::Value GetManualProxyConfig(const std::string& proxy_servers,
-                                 const std::string& bypass_list) {
+base::Value::Dict GetManualProxyConfig(const std::string& proxy_servers,
+                                       const std::string& bypass_list) {
   return ProxyConfigDictionary::CreateFixedServers(proxy_servers, bypass_list);
 }
 

chrome/browser/ash/crosapi/translate_crosapi_to_proxy_config.cc

@@ -68,7 +68,7 @@ std::string FormatProxyUri(const char* request_scheme,
 
 // See //net/docs/proxy.md and net::ProxyConfig::ProxyRules::ParseFromString()
 // for translation rules.
-base::Value TranslateManualProxySettings(
+base::Value::Dict TranslateManualProxySettings(
     crosapi::mojom::ProxySettingsManualPtr proxy_settings) {
   std::vector<std::string> proxy_server_specs;
   for (auto const& proxy : proxy_settings->http_proxies) {
@@ -89,15 +89,15 @@ base::Value TranslateManualProxySettings(
       base::JoinString(proxy_settings->exclude_domains, ";"));
 }
 
-base::Value TranslatePacProxySettings(
+base::Value::Dict TranslatePacProxySettings(
     crosapi::mojom::ProxySettingsPacPtr proxy_settings) {
   if (!proxy_settings->pac_url.is_valid())
     return ProxyConfigDictionary::CreateDirect();
   return ProxyConfigDictionary::CreatePacScript(proxy_settings->pac_url.spec(),
                                                 proxy_settings->pac_mandatory);
 }
 
-base::Value TranslateWpadProxySettings(
+base::Value::Dict TranslateWpadProxySettings(
     crosapi::mojom::ProxySettingsWpadPtr proxy_settings) {
   return ProxyConfigDictionary::CreateAutoDetect();
 }

chrome/browser/ash/login/saml/saml_lockscreen_browsertest.cc

@@ -684,10 +684,9 @@ class ProxyAuthLockscreenWebUiTest : public LockscreenWebUiTest {
   // Configure settings which are neccesarry for `NetworkStateInformer` to
   // report `NetworkStateInformer::PROXY_AUTH_REQUIRED` in the tests.
   void ConfigureNetworkBehindProxy() {
-    base::Value proxy_config = ProxyConfigDictionary::CreateFixedServers(
-        proxy_server_.host_port_pair().ToString(), "");
-
-    ProxyConfigDictionary proxy_config_dict(std::move(proxy_config));
+    ProxyConfigDictionary proxy_config_dict(
+        ProxyConfigDictionary::CreateFixedServers(
+            proxy_server_.host_port_pair().ToString(), ""));
     const NetworkState* network =
         network_state_test_helper_->network_state_handler()->DefaultNetwork();
     ASSERT_TRUE(network);

chrome/browser/ash/net/system_proxy_manager_browsertest.cc

@@ -420,7 +420,7 @@ class SystemProxyManagerPolicyCredentialsBrowserTest
   }
 
   void SetProxyConfigForNetworkService(const std::string& service_path,
-                                       base::Value proxy_config) {
+                                       base::Value::Dict proxy_config) {
     ProxyConfigDictionary proxy_config_dict(std::move(proxy_config));
     DCHECK(NetworkHandler::IsInitialized());
     const NetworkState* network =
@@ -582,10 +582,9 @@ IN_PROC_BROWSER_TEST_F(SystemProxyManagerPolicyCredentialsBrowserTest,
 IN_PROC_BROWSER_TEST_F(SystemProxyManagerPolicyCredentialsBrowserTest,
                        UserSetProxy) {
   SetPolicyCredentials(kUsername, kPassword);
-  base::Value proxy_config(base::Value::Type::DICTIONARY);
-  proxy_config.SetKey("mode",
-                      base::Value(ProxyPrefs::kFixedServersProxyModeName));
-  proxy_config.SetKey("server", base::Value("proxy:8080"));
+  base::Value::Dict proxy_config;
+  proxy_config.Set("mode", base::Value(ProxyPrefs::kFixedServersProxyModeName));
+  proxy_config.Set("server", base::Value("proxy:8080"));
   SetProxyConfigForNetworkService(kDefaultServicePath, std::move(proxy_config));
   RunUntilIdle();
   int set_auth_details_call_count = 0;

chrome/browser/ash/proxy_config_service_impl_unittest.cc

@@ -329,7 +329,7 @@ class ProxyConfigServiceImplTest : public testing::Test {
     proxy_config_service_.reset();
   }
 
-  base::Value InitConfigWithTestInput(const Input& input) {
+  base::Value::Dict InitConfigWithTestInput(const Input& input) {
     switch (input.mode) {
       case Mode::kDirect:
         return ProxyConfigDictionary::CreateDirect();
@@ -343,10 +343,10 @@ class ProxyConfigServiceImplTest : public testing::Test {
                                                          input.bypass_rules);
     }
     NOTREACHED();
-    return base::Value();
+    return base::Value::Dict();
   }
 
-  void SetUserConfigInShill(const base::Value* pref_proxy_config_dict) {
+  void SetUserConfigInShill(const base::Value::Dict* pref_proxy_config_dict) {
     std::string proxy_config;
     if (pref_proxy_config_dict)
       base::JSONWriter::Write(*pref_proxy_config_dict, &proxy_config);
@@ -392,7 +392,7 @@ TEST_F(ProxyConfigServiceImplTest, NetworkProxy) {
     SCOPED_TRACE(base::StringPrintf("Test[%" PRIuS "] %s", i,
                                     tests[i].description.c_str()));
 
-    base::Value test_config = InitConfigWithTestInput(tests[i].input);
+    base::Value::Dict test_config = InitConfigWithTestInput(tests[i].input);
     SetUserConfigInShill(&test_config);
 
     net::ProxyConfigWithAnnotation config;
@@ -445,10 +445,9 @@ TEST_F(ProxyConfigServiceImplTest, DynamicPrefsOverride) {
         recommended_params.description.c_str(),
         network_params.description.c_str()));
 
-    base::Value managed_config = InitConfigWithTestInput(managed_params.input);
-    base::Value recommended_config =
-        InitConfigWithTestInput(recommended_params.input);
-    base::Value network_config = InitConfigWithTestInput(network_params.input);
+    base::Value managed_config(InitConfigWithTestInput(managed_params.input));
+    base::Value recommended_config(
+        InitConfigWithTestInput(recommended_params.input));
 
     // Managed proxy pref should take effect over recommended proxy and
     // non-existent network proxy.
@@ -477,6 +476,8 @@ TEST_F(ProxyConfigServiceImplTest, DynamicPrefsOverride) {
     EXPECT_TRUE(recommended_params.proxy_rules.Matches(
         actual_config.value().proxy_rules()));
 
+    base::Value::Dict network_config =
+        InitConfigWithTestInput(network_params.input);
     // Network proxy should take take effect over recommended proxy pref.
     SetUserConfigInShill(&network_config);
     SyncGetLatestProxyConfig(&actual_config);

chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc

@@ -2249,10 +2249,10 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest,
   // that the PAC isn't already loaded by the time the extension starts
   // affecting requests.
   PrefService* pref_service = browser()->profile()->GetPrefs();
-  pref_service->Set(proxy_config::prefs::kProxy,
-                    ProxyConfigDictionary::CreatePacScript(
-                        embedded_test_server()->GetURL("/self.pac").spec(),
-                        true /* pac_mandatory */));
+  pref_service->SetDict(proxy_config::prefs::kProxy,
+                        ProxyConfigDictionary::CreatePacScript(
+                            embedded_test_server()->GetURL("/self.pac").spec(),
+                            true /* pac_mandatory */));
   // Flush the proxy configuration change over the Mojo pipe to avoid any races.
   ProfileNetworkContextServiceFactory::GetForContext(browser()->profile())
       ->FlushProxyConfigMonitorForTesting();

chrome/browser/extensions/api/proxy/proxy_api.cc

@@ -146,7 +146,7 @@ std::unique_ptr<base::Value> ProxyPrefTransformer::BrowserToExtensionPref(
 
   // This is a dictionary wrapper that exposes the proxy configuration stored in
   // the browser preferences.
-  ProxyConfigDictionary config(browser_pref->Clone());
+  ProxyConfigDictionary config(browser_pref->GetDict().Clone());
 
   ProxyPrefs::ProxyMode mode;
   if (!config.GetMode(&mode)) {

chrome/browser/extensions/api/proxy/proxy_api_helpers_unittest.cc

@@ -228,40 +228,40 @@ TEST(ExtensionProxyApiHelpers, GetBypassListFromExtensionPref) {
 
 TEST(ExtensionProxyApiHelpers, CreateProxyConfigDict) {
   std::string error;
-  base::Value exp_direct = ProxyConfigDictionary::CreateDirect();
+  base::Value::Dict exp_direct = ProxyConfigDictionary::CreateDirect();
   std::unique_ptr<base::Value> out_direct(CreateProxyConfigDict(
       ProxyPrefs::MODE_DIRECT, false, std::string(), std::string(),
       std::string(), std::string(), &error));
   EXPECT_EQ(exp_direct, *out_direct);
 
-  base::Value exp_auto = ProxyConfigDictionary::CreateAutoDetect();
+  base::Value::Dict exp_auto = ProxyConfigDictionary::CreateAutoDetect();
   std::unique_ptr<base::Value> out_auto(CreateProxyConfigDict(
       ProxyPrefs::MODE_AUTO_DETECT, false, std::string(), std::string(),
       std::string(), std::string(), &error));
   EXPECT_EQ(exp_auto, *out_auto);
 
-  base::Value exp_pac_url =
+  base::Value::Dict exp_pac_url =
       ProxyConfigDictionary::CreatePacScript(kSamplePacScriptUrl, false);
   std::unique_ptr<base::Value> out_pac_url(CreateProxyConfigDict(
       ProxyPrefs::MODE_PAC_SCRIPT, false, kSamplePacScriptUrl, std::string(),
       std::string(), std::string(), &error));
   EXPECT_EQ(exp_pac_url, *out_pac_url);
 
-  base::Value exp_pac_data =
+  base::Value::Dict exp_pac_data =
       ProxyConfigDictionary::CreatePacScript(kSamplePacScriptAsDataUrl, false);
   std::unique_ptr<base::Value> out_pac_data(CreateProxyConfigDict(
       ProxyPrefs::MODE_PAC_SCRIPT, false, std::string(), kSamplePacScript,
       std::string(), std::string(), &error));
   EXPECT_EQ(exp_pac_data, *out_pac_data);
 
-  base::Value exp_fixed =
+  base::Value::Dict exp_fixed =
       ProxyConfigDictionary::CreateFixedServers("foo:80", "localhost");
   std::unique_ptr<base::Value> out_fixed(CreateProxyConfigDict(
       ProxyPrefs::MODE_FIXED_SERVERS, false, std::string(), std::string(),
       "foo:80", "localhost", &error));
   EXPECT_EQ(exp_fixed, *out_fixed);
 
-  base::Value exp_system = ProxyConfigDictionary::CreateSystem();
+  base::Value::Dict exp_system = ProxyConfigDictionary::CreateSystem();
   std::unique_ptr<base::Value> out_system(CreateProxyConfigDict(
       ProxyPrefs::MODE_SYSTEM, false, std::string(), std::string(),
       std::string(), std::string(), &error));

chrome/browser/extensions/api/proxy/proxy_apitest.cc

@@ -49,7 +49,7 @@ class ProxySettingsApiTest : public ExtensionApiTest {
     // TODO(https://crbug.com/1348219) This should call
     // `PrefService::GetDict`.
     ProxyConfigDictionary dict(
-        pref_service->GetValue(proxy_config::prefs::kProxy).Clone());
+        pref_service->GetDict(proxy_config::prefs::kProxy).Clone());
 
     ProxyPrefs::ProxyMode mode;
     ASSERT_TRUE(dict.GetMode(&mode));

chrome/browser/extensions/api/web_request/web_request_apitest.cc

@@ -1676,10 +1676,10 @@ IN_PROC_BROWSER_TEST_P(ExtensionWebRequestApiTestWithContextType,
   // that the PAC isn't already loaded by the time the extension starts
   // affecting requests.
   PrefService* pref_service = browser()->profile()->GetPrefs();
-  pref_service->Set(proxy_config::prefs::kProxy,
-                    ProxyConfigDictionary::CreatePacScript(
-                        embedded_test_server()->GetURL("/self.pac").spec(),
-                        true /* pac_mandatory */));
+  pref_service->SetDict(proxy_config::prefs::kProxy,
+                        ProxyConfigDictionary::CreatePacScript(
+                            embedded_test_server()->GetURL("/self.pac").spec(),
+                            true /* pac_mandatory */));
   // Flush the proxy configuration change over the Mojo pipe to avoid any races.
   ProfileNetworkContextServiceFactory::GetForContext(browser()->profile())
       ->FlushProxyConfigMonitorForTesting();
@@ -5021,10 +5021,10 @@ class ProxyCORSWebRequestApiTest
     ASSERT_TRUE(proxy_cors_server_.Start());
 
     PrefService* pref_service = browser()->profile()->GetPrefs();
-    pref_service->Set(proxy_config::prefs::kProxy,
-                      ProxyConfigDictionary::CreateFixedServers(
-                          proxy_cors_server_.host_port_pair().ToString(),
-                          "accounts.google.com"));
+    pref_service->SetDict(proxy_config::prefs::kProxy,
+                          ProxyConfigDictionary::CreateFixedServers(
+                              proxy_cors_server_.host_port_pair().ToString(),
+                              "accounts.google.com"));
 
     // Flush the proxy configuration change to avoid any races.
     ProfileNetworkContextServiceFactory::GetForContext(browser()->profile())

chrome/browser/net/load_timing_browsertest.cc

@@ -155,7 +155,7 @@ IN_PROC_BROWSER_TEST_F(LoadTimingBrowserTest, DISABLED_HTTPS) {
 IN_PROC_BROWSER_TEST_F(LoadTimingBrowserTest, Proxy) {
   ASSERT_TRUE(embedded_test_server()->Start());
 
-  browser()->profile()->GetPrefs()->Set(
+  browser()->profile()->GetPrefs()->SetDict(
       proxy_config::prefs::kProxy,
       ProxyConfigDictionary::CreateFixedServers(
           embedded_test_server()->host_port_pair().ToString(), std::string()));

chrome/browser/net/network_context_configuration_browsertest.cc

@@ -462,9 +462,9 @@ class NetworkContextConfigurationBrowserTest
   // Sets the proxy preference on a PrefService based on the NetworkContextType,
   // and waits for it to be applied.
   void SetProxyPref(const net::HostPortPair& host_port_pair) {
-    GetPrefService()->Set(proxy_config::prefs::kProxy,
-                          ProxyConfigDictionary::CreateFixedServers(
-                              host_port_pair.ToString(), std::string()));
+    GetPrefService()->SetDict(proxy_config::prefs::kProxy,
+                              ProxyConfigDictionary::CreateFixedServers(
+                                  host_port_pair.ToString(), std::string()));
 
     // Wait for the new ProxyConfig to be passed over the pipe. Needed because
     // Mojo doesn't guarantee ordering of events on different Mojo pipes, and

chrome/browser/net/private_network_access_browsertest.cc

@@ -727,7 +727,7 @@ IN_PROC_BROWSER_TEST_F(PrivateNetworkAccessWithFeatureEnabledBrowserTest,
   EXPECT_TRUE(
       content::NavigateToURL(web_contents(), PublicNonSecureURL(*server)));
 
-  browser()->profile()->GetPrefs()->Set(
+  browser()->profile()->GetPrefs()->SetDict(
       proxy_config::prefs::kProxy,
       ProxyConfigDictionary::CreateFixedServers(
           server->host_port_pair().ToString(), ""));

chrome/browser/policy/test/proxy_policies_browsertest.cc

@@ -32,9 +32,9 @@ void VerifyProxyPrefs(PrefService* prefs,
                       absl::optional<bool> expected_proxy_pac_mandatory,
                       const std::string& expected_proxy_bypass_list,
                       const ProxyPrefs::ProxyMode& expected_proxy_mode) {
-  const base::Value& value = prefs->GetValue(proxy_config::prefs::kProxy);
-  ASSERT_TRUE(value.is_dict());
-  ProxyConfigDictionary dict(value.Clone());
+  const base::Value::Dict& pref_dict =
+      prefs->GetDict(proxy_config::prefs::kProxy);
+  ProxyConfigDictionary dict(pref_dict.Clone());
   std::string s;
   bool b;
   if (expected_proxy_server.empty()) {