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

chrome/browser/net/pref_proxy_config_service.cc

Index: chrome/browser/net/pref_proxy_config_service.cc
diff --git a/chrome/browser/net/pref_proxy_config_service.cc b/chrome/browser/net/pref_proxy_config_service.cc
index 2b1010442ae083c5f9566c2fd0bccf7d75bdc83b..6590027b958ded44ff289a6ceaac981e36b6db9c 100644
--- a/chrome/browser/net/pref_proxy_config_service.cc
+++ b/chrome/browser/net/pref_proxy_config_service.cc
@@ -86,9 +86,10 @@ bool PrefProxyConfigTracker::ReadPrefConfig(net::ProxyConfig* config) {
   // Clear the configuration.
   *config = net::ProxyConfig();
 
+  ProxyPrefsDictionary proxy_dict(pref_service_->GetDictionary(prefs::kProxy));

[danno, 2011/02/04 10:32:13]

Perhaps there should be a specific constructor on the ProxyPrefsDictionary that
just takes a pref_service. This eliminates the needs to clients of the proxy
prefs dictionary to have to know which preference they need access.

[battre, 2011/02/07 14:13:47]

I think this change is a great idea if we can handle read and write access by
the ProxyPrefsDictionary. Unfortunately, I think that this would make stuff more
complicated.

- CommandLinePrefStore sets values directly into itself (i.e.
ProxyPrefsDictionary would need to write into a ValueMapPrefStore).
- We want to be able to store settings for incognito and regular pref services -
though the incognito pref service might not exist at the time of setting a
value. We would require a dependency to an ExtensionPrefs which would make
testing very awkward.
- AdvancedOptionsHandler checks whether the Preference is extension managed.
- We need to be able to subscribe to proxy pref changes.

I think it is better to leave this as it is.

+
   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 = ProxyPrefsDictionary::CreateSystem();
+  pref_service->RegisterDictionaryPref(prefs::kProxy, default_settings);
 }