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

chrome/browser/extensions/extension_proxy_api.cc

Index: chrome/browser/extensions/extension_proxy_api.cc
diff --git a/chrome/browser/extensions/extension_proxy_api.cc b/chrome/browser/extensions/extension_proxy_api.cc
index 621262c4ec67d432a58036007fefc75856a7bd66..70aa1c8974186bf56a1a9cfbbe34a65326cc018e 100644
--- a/chrome/browser/extensions/extension_proxy_api.cc
+++ b/chrome/browser/extensions/extension_proxy_api.cc
@@ -72,18 +72,72 @@ bool UseCustomProxySettingsFunction::RunImpl() {
 
   std::string proxy_mode;
   proxy_config->GetString("mode", &proxy_mode);
+  ProxyPrefs::ProxyMode mode_enum;
+  if (!ProxyPrefs::StringToProxyMode(proxy_mode, &mode_enum)) {
+    LOG(ERROR) << "Invalid mode for proxy settings: " << proxy_mode << ". "
+               << "Setting custom proxy settings failed.";
+    return false;
+  }
 
   DictionaryValue* pac_dict = NULL;
   proxy_config->GetDictionary("pacScript", &pac_dict);
+  std::string pac_url;
+  if (pac_dict && !pac_dict->GetString("url", &pac_url)) {
+    LOG(ERROR) << "'pacScript' requires a 'url' field. "
+               << "Setting custom proxy settings failed.";
+    return false;
+  }
 
   DictionaryValue* proxy_rules = NULL;
   proxy_config->GetDictionary("rules", &proxy_rules);
+  std::string proxy_rules_string;
+  if (proxy_rules && !GetProxyRules(proxy_rules, &proxy_rules_string)) {
+    LOG(ERROR) << "Invalid 'rules' specified. "
+               << "Setting custom proxy settings failed.";
+    return false;
+  }
 
-  // TODO(battre,gfeher): Make sure all the preferences get always
-  // overwritten.
-  return ApplyMode(proxy_mode, incognito) &&
-      ApplyPacScript(pac_dict, incognito) &&
-      ApplyProxyRules(proxy_rules, incognito);
+  // not supported, yet.
+  std::string bypass_list;
+
+  DictionaryValue* result_proxy_config = NULL;
+  switch (mode_enum) {
+    case ProxyPrefs::MODE_DIRECT:
+      result_proxy_config = ProxyPrefsDictionary::CreateDirect();
+      break;
+    case ProxyPrefs::MODE_AUTO_DETECT:
+      result_proxy_config = ProxyPrefsDictionary::CreateAutoDetect();
+      break;
+    case ProxyPrefs::MODE_PAC_SCRIPT: {
+      if (!pac_dict) {
+        LOG(ERROR) << "Proxy mode 'pac_script' requires a 'pacScript' field. "
+                   << "Setting custom proxy settings failed.";
+        return false;
+      }
+      result_proxy_config = ProxyPrefsDictionary::CreatePacScript(pac_url);
+      break;
+    }
+    case ProxyPrefs::MODE_FIXED_SERVERS: {
+      if (!proxy_rules) {
+        LOG(ERROR) << "Proxy mode 'fixed_servers' requires a 'rules' field. "
+                   << "Setting custom proxy settings failed.";
+        return false;
+      }
+      result_proxy_config = ProxyPrefsDictionary::CreateFixedServers(
+          proxy_rules_string, bypass_list);
+      break;
+    }
+    case ProxyPrefs::MODE_SYSTEM:
+      result_proxy_config = ProxyPrefsDictionary::CreateSystem();
+      break;
+    case ProxyPrefs::kModeCount:
+      NOTREACHED();
+  }
+  if (!result_proxy_config)
+    return false;
+
+  ApplyPreference(prefs::kProxy, result_proxy_config, incognito);
+  return true;
 }
 
 bool UseCustomProxySettingsFunction::GetProxyServer(
@@ -94,45 +148,11 @@ bool UseCustomProxySettingsFunction::GetProxyServer(
   return true;
 }
 
-bool UseCustomProxySettingsFunction::ApplyMode(const std::string& mode,
-                                               bool incognito) {
-  // We take control of the mode preference even if none was specified, so that
-  // all proxy preferences are controlled by the same extension (if not by a
-  // higher-priority source).
-  bool result = true;
-  ProxyPrefs::ProxyMode mode_enum;
-  if (!ProxyPrefs::StringToProxyMode(mode, &mode_enum)) {
-    mode_enum = ProxyPrefs::MODE_SYSTEM;
-    LOG(WARNING) << "Invalid mode for proxy settings: " << mode;
-    result = false;
-  }
-  ApplyPreference(
-      prefs::kProxyMode, Value::CreateIntegerValue(mode_enum), incognito);
-  return result;
-}
-
-bool UseCustomProxySettingsFunction::ApplyPacScript(DictionaryValue* pac_dict,
-                                                    bool incognito) {
-  std::string pac_url;
-  if (pac_dict)
-    pac_dict->GetString("url", &pac_url);
-
-  // We take control of the PAC preference even if none was specified, so that
-  // all proxy preferences are controlled by the same extension (if not by a
-  // higher-priority source).
-  ApplyPreference(
-      prefs::kProxyPacUrl, Value::CreateStringValue(pac_url), incognito);
-  return true;
-}
-
-bool UseCustomProxySettingsFunction::ApplyProxyRules(
+bool UseCustomProxySettingsFunction::GetProxyRules(
     DictionaryValue* proxy_rules,
-    bool incognito) {
-  if (!proxy_rules) {
-    ApplyPreference(
-        prefs::kProxyServer, Value::CreateStringValue(""), incognito);
-    return true;
-  }
+    std::string* out) {
+  if (!proxy_rules)
+    return false;
 
   // Local data into which the parameters will be parsed. has_proxy describes
   // whether a setting was found for the scheme; proxy_dict holds the
@@ -186,8 +206,7 @@ bool UseCustomProxySettingsFunction::ApplyProxyRules(
     }
   }
 
-  ApplyPreference(
-      prefs::kProxyServer, Value::CreateStringValue(proxy_pref), incognito);
+  *out = proxy_pref;
   return true;
 }
 
@@ -195,8 +214,6 @@ bool RemoveCustomProxySettingsFunction::RunImpl() {
   bool incognito = false;
   args_->GetBoolean(0, &incognito);
 
-  ApplyPreference(prefs::kProxyMode, NULL, incognito);
-  ApplyPreference(prefs::kProxyPacUrl, NULL, incognito);
-  ApplyPreference(prefs::kProxyServer, NULL, incognito);
+  ApplyPreference(prefs::kProxy, NULL, incognito);
   return true;
 }