Fix handling of setting a single proxy in Proxy Settings API

Previously, setting a singleProxy entry in the Proxy Settings API meant that we set http, https, and ftp proxies individually. I.e. the Proxy settings was "http=http://foo;https=https://foo;ftp=ftp://foo". With this change it becomes "foo".

BUG=67779
TEST=none

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

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

3 files changed, 27 insertions, 31 deletions

diff --git a/chrome/browser/extensions/extension_proxy_api.cc b/chrome/browser/extensions/extension_proxy_api.cc
index a8852cc..c905cab 100644
--- a/chrome/browser/extensions/extension_proxy_api.cc
+++ b/chrome/browser/extensions/extension_proxy_api.cc
@@ -49,6 +49,7 @@ COMPILE_ASSERT(arraysize(field_name) == SCHEME_MAX + 1,
                field_name_array_is_wrong_size);
 COMPILE_ASSERT(arraysize(scheme_name) == SCHEME_MAX + 1,
                scheme_name_array_is_wrong_size);
+COMPILE_ASSERT(SCHEME_ALL == 0, singleProxy_must_be_first_option);
 
 void ProxySettingsFunction::ApplyPreference(const char* pref_path,
                                             Value* pref_value,
@@ -176,23 +177,34 @@ bool UseCustomProxySettingsFunction::GetProxyRules(
     }
   }
 
-  // A single proxy supersedes individual HTTP, HTTPS, and FTP proxies.
+  // Handle case that only singleProxy is specified.
   if (has_proxy[SCHEME_ALL]) {
-    proxy_server[SCHEME_HTTP] = proxy_server[SCHEME_ALL];
-    proxy_server[SCHEME_HTTPS] = proxy_server[SCHEME_ALL];
-    proxy_server[SCHEME_FTP] = proxy_server[SCHEME_ALL];
-    has_proxy[SCHEME_HTTP] = true;
-    has_proxy[SCHEME_HTTPS] = true;
-    has_proxy[SCHEME_FTP] = true;
-    has_proxy[SCHEME_ALL] = false;
+    for (size_t i = 1; i <= SCHEME_MAX; ++i) {
+      if (has_proxy[i]) {
+        LOG(ERROR) << "Proxy rule for " << field_name[SCHEME_ALL] << " and "
+                   << field_name[i] << " cannot be set at the same time.";
+        return false;
+      }
+    }
+    if (!proxy_server[SCHEME_ALL].scheme.empty())
+      LOG(WARNING) << "Ignoring scheme attribute from proxy server.";
+    // Build the proxy preference string.
+    std::string proxy_pref;
+    proxy_pref.append(proxy_server[SCHEME_ALL].host);
+    if (proxy_server[SCHEME_ALL].port != ProxyServer::INVALID_PORT) {
+      proxy_pref.append(":");
+      proxy_pref.append(base::StringPrintf("%d",
+                                           proxy_server[SCHEME_ALL].port));
+    }
+    *out = proxy_pref;
+    return true;
   }
 
-  // TODO(pamg): Ensure that if a value is empty, that means "don't use a proxy
-  // for this scheme".
+  // Handle case the anything but singleProxy is specified.
 
   // Build the proxy preference string.
   std::string proxy_pref;
-  for (size_t i = 0; i <= SCHEME_MAX; ++i) {
+  for (size_t i = 1; i <= SCHEME_MAX; ++i) {
     if (has_proxy[i]) {
       // http=foopy:4010;ftp=socks://foopy2:80
       if (!proxy_pref.empty())
diff --git a/chrome/browser/extensions/extension_proxy_apitest.cc b/chrome/browser/extensions/extension_proxy_apitest.cc
index 024595f..44856dd 100644
--- a/chrome/browser/extensions/extension_proxy_apitest.cc
+++ b/chrome/browser/extensions/extension_proxy_apitest.cc
@@ -118,12 +118,9 @@ IN_PROC_BROWSER_TEST_F(ProxySettingsApiTest, ProxyFixedSingle) {
 
   PrefService* pref_service = browser()->profile()->GetPrefs();
   ValidateSettings(ProxyPrefs::MODE_FIXED_SERVERS,
-                   "http=http://127.0.0.1:100;"
-                       "https=http://127.0.0.1:100;"
-                       "ftp=http://127.0.0.1:100;"
-                       "socks=http://9.9.9.9",
-                   kNoPac,
-                   pref_service);
+                 "127.0.0.1:100",
+                 kNoPac,
+                 pref_service);
 }
 
 // Tests setting to use the system's proxy settings.
diff --git a/chrome/test/data/extensions/api_test/proxy/single/test.js b/chrome/test/data/extensions/api_test/proxy/single/test.js
index 9cc8502..5f19b5e 100644
--- a/chrome/test/data/extensions/api_test/proxy/single/test.js
+++ b/chrome/test/data/extensions/api_test/proxy/single/test.js
@@ -8,25 +8,12 @@
 chrome.test.runTests([
   function setSingleProxy() {
     var oneProxy = {
-      scheme: "http",
       host: "127.0.0.1",
       port: 100
     };
 
-    // Single proxy should override HTTP proxy.
-    var httpProxy = {
-      host: "8.8.8.8"
-    };
-
-    // Single proxy should not override SOCKS proxy.
-    var socksProxy = {
-      host: "9.9.9.9"
-    };
-
     var rules = {
-      singleProxy: oneProxy,
-      proxyForHttp: httpProxy,
-      socksProxy: socksProxy,
+      singleProxy: oneProxy
     };
 
     var config = { rules: rules, mode: "fixed_servers" };