Whenever proxy configurations contain socks and http/https/ftp proxies, socks...

net/proxy/proxy_config.cc

Index: net/proxy/proxy_config.cc
===================================================================
--- net/proxy/proxy_config.cc	(revision 20189)
+++ net/proxy/proxy_config.cc	(working copy)
@@ -36,6 +36,7 @@
   proxy_for_http = ProxyServer();
   proxy_for_https = ProxyServer();
   proxy_for_ftp = ProxyServer();
+  socks_proxy = ProxyServer();
 
   StringTokenizer proxy_server_list(proxy_rules, ";");
   while (proxy_server_list.GetNext()) {
@@ -51,36 +52,40 @@
       if (!proxy_server_for_scheme.GetNext()) {
         if (type == TYPE_PROXY_PER_SCHEME)
           continue;  // Unexpected.
-        single_proxy = ProxyServer::FromURI(url_scheme);
+        single_proxy = ProxyServer::FromURI(url_scheme,
+                                            ProxyServer::SCHEME_HTTP);
         type = TYPE_SINGLE_PROXY;
         return;
       }
 
-      // If the proxy settings has only socks and others blank,
-      // make that the default for all the proxies
-      // This gets hit only on windows when using IE settings.
-      if (url_scheme == "socks") {
-        std::string proxy_server_string = "socks://";
-        proxy_server_string.append(proxy_server_for_scheme.token());
-        single_proxy = ProxyServer::FromURI(proxy_server_string);
-        type = TYPE_SINGLE_PROXY;
-        return;
-      }
-
       // Trim whitespace off the url scheme.
       TrimWhitespaceASCII(url_scheme, TRIM_ALL, &url_scheme);
 
       // Add it to the per-scheme mappings (if supported scheme).
       type = TYPE_PROXY_PER_SCHEME;
-      if (const ProxyServer* entry = MapSchemeToProxy(url_scheme))
-        *const_cast<ProxyServer*>(entry) =
-            ProxyServer::FromURI(proxy_server_for_scheme.token());
+      if (ProxyServer* entry = MapSchemeToProxy(url_scheme)) {
+        std::string proxy_server_token = proxy_server_for_scheme.token();
+        ProxyServer::Scheme scheme = (entry == &socks_proxy) ?
+            ProxyServer::SCHEME_SOCKS4 : ProxyServer::SCHEME_HTTP;
+        *entry = ProxyServer::FromURI(proxy_server_token, scheme);
+      }
     }
   }
 }
 
-const ProxyServer* ProxyConfig::ProxyRules::MapSchemeToProxy(
-    const std::string& scheme) const {
+const ProxyServer* ProxyConfig::ProxyRules::MapUrlSchemeToProxy(
+    const std::string& url_scheme) const {
+  const ProxyServer* proxy_server =
+      const_cast<ProxyRules*>(this)->MapSchemeToProxy(url_scheme);
+  if (proxy_server != NULL && proxy_server->is_valid())

[eroman, 2009/07/10 21:24:46]

nit: leave off the "!= NULL" .

[Arindam, 2009/07/11 00:53:35]

We need this, in case the url_scheme is invalid, then MapSchemeToProxy returns
NULL. This is required for this case :
http://codereview.chromium.org/149191/diff/1079/76#oldcode184

[eroman, 2009/07/13 23:41:31]

What I mean was purely a stylistic comment:

Rewrite 
  if (proxy_server != NULL && proxy_server->is_valid())
AS:
  if (proxy_server && proxy_server->is_valid())

(We prefer the former style in the network code.)

+    return proxy_server;
+  if (socks_proxy.is_valid())
+    return &socks_proxy;
+  return NULL;  // No mapping for this scheme. Use direct.
+}
+
+ProxyServer* ProxyConfig::ProxyRules::MapSchemeToProxy(
+    const std::string& scheme) {
   DCHECK(type == TYPE_PROXY_PER_SCHEME);
   if (scheme == "http")
     return &proxy_for_http;
@@ -88,6 +93,8 @@
     return &proxy_for_https;
   if (scheme == "ftp")
     return &proxy_for_ftp;
+  if (scheme == "socks")
+    return &socks_proxy;
   return NULL;  // No mapping for this scheme.
 }
 
@@ -194,6 +201,7 @@
              << "    proxy_for_http: " << rules.proxy_for_http << "\n"
              << "    proxy_for_https: " << rules.proxy_for_https << "\n"
              << "    proxy_for_ftp: " << rules.proxy_for_ftp << "\n"
+             << "    socks_proxy: " << rules.socks_proxy << "\n"
              << "  }";
 }