Change ProxyRules to handle ProxyLists rather than just single ProxyServer instances.

net/proxy/proxy_config.cc

Index: net/proxy/proxy_config.cc
diff --git a/net/proxy/proxy_config.cc b/net/proxy/proxy_config.cc
index 022e73288e5342449b92ca67e4501d34dec43bd4..f65d8c084b1ac90869aba1fc9609621a8c23411d 100644
--- a/net/proxy/proxy_config.cc
+++ b/net/proxy/proxy_config.cc
@@ -16,17 +16,17 @@ namespace {
 
 // If |proxy| is valid, sets it in |dict| under the key |name|.

[eroman, 2013/02/26 01:13:50]

"is valid" needs to be updated.

[marq_use_my_chromium_address, 2013/02/27 23:08:19]

Done.

 void AddProxyToValue(const char* name,

[eroman, 2013/02/26 01:13:50]

This should be renamed to AddProxyListToValue()

[marq_use_my_chromium_address, 2013/02/27 23:08:19]

Done.

-                     const ProxyServer& proxy,
+                     const ProxyList& proxy,
                      base::DictionaryValue* dict) {
-  if (proxy.is_valid())
-    dict->SetString(name, proxy.ToURI());
+  if (!proxy.IsEmpty())
+    dict->SetString(name, proxy.ToPacString());
 }
 
 }  // namespace
 
 ProxyConfig::ProxyRules::ProxyRules()
     : reverse_bypass(false),
-      type(TYPE_NO_RULES) {
+      type(TYPE_NO_RULES){

[eroman, 2013/02/26 01:13:50]

Why remove the space? That is counter to the style AFAIK.

[marq_use_my_chromium_address, 2013/02/27 23:08:19]

Done.

 }
 
 ProxyConfig::ProxyRules::~ProxyRules() {
@@ -48,13 +48,13 @@ void ProxyConfig::ProxyRules::Apply(const GURL& url, ProxyInfo* result) const {
 
   switch (type) {
     case ProxyRules::TYPE_SINGLE_PROXY: {
-      result->UseProxyServer(single_proxy);
+      result->UsePacString(single_proxy.ToPacString());

[eroman, 2013/02/26 01:13:50]

Rather than round-tripping through the PAC string format, this should just set
the values directly.

Perhaps add a method like:
  result->UseProxyList(single_proxy);

[marq_use_my_chromium_address, 2013/02/27 23:08:19]

Done.

       return;
     }
     case ProxyRules::TYPE_PROXY_PER_SCHEME: {
-      const ProxyServer* entry = MapUrlSchemeToProxy(url.scheme());
+      const ProxyList* entry = MapUrlSchemeToProxy(url.scheme());
       if (entry) {
-        result->UseProxyServer(*entry);
+        result->UsePacString(entry->ToPacString());

[eroman, 2013/02/26 01:13:50]

See comment above.

[marq_use_my_chromium_address, 2013/02/27 23:08:19]

Done.

       } else {
         // We failed to find a matching proxy server for the current URL
         // scheme. Default to direct.
@@ -73,11 +73,11 @@ void ProxyConfig::ProxyRules::Apply(const GURL& url, ProxyInfo* result) const {
 void ProxyConfig::ProxyRules::ParseFromString(const std::string& proxy_rules) {
   // Reset.
   type = TYPE_NO_RULES;
-  single_proxy = ProxyServer();
-  proxy_for_http = ProxyServer();
-  proxy_for_https = ProxyServer();
-  proxy_for_ftp = ProxyServer();
-  fallback_proxy = ProxyServer();
+  single_proxy = ProxyList();
+  proxy_for_http = ProxyList();
+  proxy_for_https = ProxyList();
+  proxy_for_ftp = ProxyList();
+  fallback_proxy = ProxyList();
 
   base::StringTokenizer proxy_server_list(proxy_rules, ";");
   while (proxy_server_list.GetNext()) {
@@ -93,8 +93,8 @@ void ProxyConfig::ProxyRules::ParseFromString(const std::string& proxy_rules) {
       if (!proxy_server_for_scheme.GetNext()) {
         if (type == TYPE_PROXY_PER_SCHEME)
           continue;  // Unexpected.
-        single_proxy = ProxyServer::FromURI(url_scheme,
-                                            ProxyServer::SCHEME_HTTP);
+        single_proxy.SetSingleProxyServer(
+            ProxyServer::FromURI(url_scheme, ProxyServer::SCHEME_HTTP));
         type = TYPE_SINGLE_PROXY;
         return;
       }
@@ -104,7 +104,7 @@ void ProxyConfig::ProxyRules::ParseFromString(const std::string& proxy_rules) {
 
       // Add it to the per-scheme mappings (if supported scheme).
       type = TYPE_PROXY_PER_SCHEME;
-      ProxyServer* entry = MapUrlSchemeToProxyNoFallback(url_scheme);
+      ProxyList* entry = MapUrlSchemeToProxyNoFallback(url_scheme);
       ProxyServer::Scheme default_scheme = ProxyServer::SCHEME_HTTP;
 
       // socks=XXX is inconsistent with the other formats, since "socks"
@@ -113,40 +113,45 @@ void ProxyConfig::ProxyRules::ParseFromString(const std::string& proxy_rules) {
       if (url_scheme == "socks") {
         DCHECK(!entry);
         entry = &fallback_proxy;
+        // Note that here 'socks' is understood to be SOCKS4, even though
+        // 'socks' maps to SOCKS5 in ProxyServer::GetSchemeFromURIInternal.

[eroman, 2013/02/26 01:13:50]

Yeah, icky carry-over from the windows interpretation of this string (which is
where the format comes from). It is convenient having a cross-platform
implementation for the purpose of command line flags, but maybe it doesn't make
sense and we should invent our own for that.

[marq_use_my_chromium_address, 2013/02/27 23:08:19]

Not my call; I just added the comment because it's non-obvious.

         default_scheme = ProxyServer::SCHEME_SOCKS4;
       }
 
       if (entry) {
-        *entry = ProxyServer::FromURI(proxy_server_for_scheme.token(),
-                                      default_scheme);
+        ProxyServer new_proxy = ProxyServer::FromURI(
+            proxy_server_for_scheme.token(), default_scheme);
+        // Don't have DIRECT as the first proxy in a list.

[eroman, 2013/02/26 01:13:50]

Why disallow this?

[marq_use_my_chromium_address, 2013/02/27 23:08:19]

I had assumed that this would create a wasteful situation where a ProxyInfo
would not return true from IsDirect, but would nonetheless function that way. On
closer reading of the code, that actually isn't the case, so I've removed this
condition.

+        if (!new_proxy.is_direct() || !entry->IsEmpty()) {
+          entry->AddProxyServer(new_proxy);
+        }
       }
     }
   }
 }
 
-const ProxyServer* ProxyConfig::ProxyRules::MapUrlSchemeToProxy(
+const ProxyList* ProxyConfig::ProxyRules::MapUrlSchemeToProxy(
     const std::string& url_scheme) const {
-  const ProxyServer* proxy_server =
+  const ProxyList* proxy_server_list =
       const_cast<ProxyRules*>(this)->MapUrlSchemeToProxyNoFallback(url_scheme);
-  if (proxy_server && proxy_server->is_valid())
-    return proxy_server;
-  if (fallback_proxy.is_valid())
+  if (proxy_server_list && !proxy_server_list->IsEmpty())
+    return proxy_server_list;
+  if (!fallback_proxy.IsEmpty())
     return &fallback_proxy;
   return NULL;  // No mapping for this scheme. Use direct.
 }
 
 bool ProxyConfig::ProxyRules::Equals(const ProxyRules& other) const {
   return type == other.type &&
-         single_proxy == other.single_proxy &&
-         proxy_for_http == other.proxy_for_http &&
-         proxy_for_https == other.proxy_for_https &&
-         proxy_for_ftp == other.proxy_for_ftp &&
-         fallback_proxy == other.fallback_proxy &&
+         single_proxy.Equals(other.single_proxy) &&
+         proxy_for_http.Equals(other.proxy_for_http) &&
+         proxy_for_https.Equals(other.proxy_for_https) &&
+         proxy_for_ftp.Equals(other.proxy_for_ftp) &&
          bypass_rules.Equals(other.bypass_rules) &&
          reverse_bypass == other.reverse_bypass;
 }
 
-ProxyServer* ProxyConfig::ProxyRules::MapUrlSchemeToProxyNoFallback(
+ProxyList* ProxyConfig::ProxyRules::MapUrlSchemeToProxyNoFallback(
     const std::string& scheme) {
   DCHECK_EQ(TYPE_PROXY_PER_SCHEME, type);
   if (scheme == "http")