Make proxy settings one atomic dictionary in the PrefStores such that modifications are atomic.

BUG=67779,70904
TEST=none

Review URL: http://codereview.chromium.org/6240013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@74284 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 20 insertions, 39 deletions

diff --git a/chrome/browser/net/pref_proxy_config_service.cc b/chrome/browser/net/pref_proxy_config_service.cc
index 2b10104..a31dc9f 100644
--- a/chrome/browser/net/pref_proxy_config_service.cc
+++ b/chrome/browser/net/pref_proxy_config_service.cc
@@ -8,7 +8,7 @@
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/pref_set_observer.h"
-#include "chrome/browser/prefs/proxy_prefs.h"
+#include "chrome/browser/prefs/proxy_config_dictionary.h"
 #include "chrome/common/notification_details.h"
 #include "chrome/common/notification_source.h"
 #include "chrome/common/notification_type.h"
@@ -86,9 +86,10 @@ bool PrefProxyConfigTracker::ReadPrefConfig(net::ProxyConfig* config) {
   // Clear the configuration.
   *config = net::ProxyConfig();
 
+  ProxyConfigDictionary proxy_dict(pref_service_->GetDictionary(prefs::kProxy));
+
   ProxyPrefs::ProxyMode mode;
-  int proxy_mode = pref_service_->GetInteger(prefs::kProxyMode);
-  if (!ProxyPrefs::IntToProxyMode(proxy_mode, &mode)) {
+  if (!proxy_dict.GetMode(&mode)) {
     // Fall back to system settings if the mode preference is invalid.
     return false;
   }
@@ -105,12 +106,12 @@ bool PrefProxyConfigTracker::ReadPrefConfig(net::ProxyConfig* config) {
       config->set_auto_detect(true);
       return true;
     case ProxyPrefs::MODE_PAC_SCRIPT: {
-      if (!pref_service_->HasPrefPath(prefs::kProxyPacUrl)) {
+      std::string proxy_pac;
+      if (!proxy_dict.GetPacUrl(&proxy_pac)) {
         LOG(ERROR) << "Proxy settings request PAC script but do not specify "
                    << "its URL. Falling back to direct connection.";
         return true;
       }
-      std::string proxy_pac = pref_service_->GetString(prefs::kProxyPacUrl);
       GURL proxy_pac_url(proxy_pac);
       if (!proxy_pac_url.is_valid()) {
         LOG(ERROR) << "Invalid proxy PAC url: " << proxy_pac;
@@ -120,18 +121,16 @@ bool PrefProxyConfigTracker::ReadPrefConfig(net::ProxyConfig* config) {
       return true;
     }
     case ProxyPrefs::MODE_FIXED_SERVERS: {
-      if (!pref_service_->HasPrefPath(prefs::kProxyServer)) {
+      std::string proxy_server;
+      if (!proxy_dict.GetProxyServer(&proxy_server)) {
         LOG(ERROR) << "Proxy settings request fixed proxy servers but do not "
                    << "specify their URLs. Falling back to direct connection.";
         return true;
       }
-      std::string proxy_server =
-          pref_service_->GetString(prefs::kProxyServer);
       config->proxy_rules().ParseFromString(proxy_server);
 
-      if (pref_service_->HasPrefPath(prefs::kProxyBypassList)) {
-        std::string proxy_bypass =
-            pref_service_->GetString(prefs::kProxyBypassList);
+      std::string proxy_bypass;
+      if (proxy_dict.GetBypassList(&proxy_bypass)) {
         config->proxy_rules().bypass_rules.ParseFromString(proxy_bypass);
       }
       return true;
@@ -229,8 +228,6 @@ void PrefProxyConfigService::RegisterObservers() {
 // static
 void PrefProxyConfigService::RegisterUserPrefs(
     PrefService* pref_service) {
-  pref_service->RegisterIntegerPref(prefs::kProxyMode, ProxyPrefs::MODE_SYSTEM);
-  pref_service->RegisterStringPref(prefs::kProxyServer, "");
-  pref_service->RegisterStringPref(prefs::kProxyPacUrl, "");
-  pref_service->RegisterStringPref(prefs::kProxyBypassList, "");
+  DictionaryValue* default_settings = ProxyConfigDictionary::CreateSystem();
+  pref_service->RegisterDictionaryPref(prefs::kProxy, default_settings);
 }
diff --git a/chrome/browser/net/pref_proxy_config_service_unittest.cc b/chrome/browser/net/pref_proxy_config_service_unittest.cc
index dad5c53..d25abaa 100644
--- a/chrome/browser/net/pref_proxy_config_service_unittest.cc
+++ b/chrome/browser/net/pref_proxy_config_service_unittest.cc
@@ -8,7 +8,7 @@
 #include "base/file_path.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
 #include "chrome/browser/prefs/pref_service_mock_builder.h"
-#include "chrome/browser/prefs/proxy_prefs.h"
+#include "chrome/browser/prefs/proxy_config_dictionary.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/testing_pref_service.h"
@@ -114,10 +114,8 @@ TEST_F(PrefProxyConfigServiceTest, BaseConfiguration) {
 
 TEST_F(PrefProxyConfigServiceTest, DynamicPrefOverrides) {
   pref_service_->SetManagedPref(
-      prefs::kProxyServer, Value::CreateStringValue("http://example.com:3128"));
-  pref_service_->SetManagedPref(
-      prefs::kProxyMode,
-      Value::CreateIntegerValue(ProxyPrefs::MODE_FIXED_SERVERS));
+      prefs::kProxy,
+      ProxyConfigDictionary::CreateFixedServers("http://example.com:3128", ""));
   loop_.RunAllPending();
 
   net::ProxyConfig actual_config;
@@ -129,9 +127,8 @@ TEST_F(PrefProxyConfigServiceTest, DynamicPrefOverrides) {
             net::ProxyServer::FromURI("http://example.com:3128",
                                       net::ProxyServer::SCHEME_HTTP));
 
-  pref_service_->SetManagedPref(
-      prefs::kProxyMode,
-      Value::CreateIntegerValue(ProxyPrefs::MODE_AUTO_DETECT));
+  pref_service_->SetManagedPref(prefs::kProxy,
+                                ProxyConfigDictionary::CreateAutoDetect());
   loop_.RunAllPending();
 
   proxy_config_service_->GetLatestProxyConfig(&actual_config);
@@ -164,17 +161,9 @@ TEST_F(PrefProxyConfigServiceTest, Observers) {
 
   EXPECT_CALL(observer,
               OnProxyConfigChanged(ProxyConfigMatches(pref_config))).Times(1);
-
-  pref_service_->SetManagedPref(prefs::kProxyPacUrl,
-                                Value::CreateStringValue(kFixedPacUrl));
-  // The above does not trigger a notification, because PrefProxyConfig still
-  // sees the mode as the default (ProxyPrefs::SYSTEM), so that it doesn't claim
-  // to have proxy config.
-  // TODO(battre): Remove this comment when http://crbug.com/65732 is
-  // resolved.
   pref_service_->SetManagedPref(
-      prefs::kProxyMode,
-      Value::CreateIntegerValue(ProxyPrefs::MODE_PAC_SCRIPT));
+      prefs::kProxy,
+      ProxyConfigDictionary::CreatePacScript(kFixedPacUrl));
   loop_.RunAllPending();
   Mock::VerifyAndClearExpectations(&observer);
 
@@ -190,12 +179,7 @@ TEST_F(PrefProxyConfigServiceTest, Observers) {
   // Clear the override should switch back to the fixed configuration.
   EXPECT_CALL(observer,
               OnProxyConfigChanged(ProxyConfigMatches(config3))).Times(1);
-  pref_service_->RemoveManagedPref(prefs::kProxyMode);
-  // The above switches the mode to the default (ProxyPrefs::SYSTEM), so the
-  // next removal won't bother PrefProxyConfigService.
-  // TODO(battre): Remove this comment when http://crbug.com/65732 is
-  // completed.
-  pref_service_->RemoveManagedPref(prefs::kProxyPacUrl);
+  pref_service_->RemoveManagedPref(prefs::kProxy);
   loop_.RunAllPending();
   Mock::VerifyAndClearExpectations(&observer);