Proxy settings automation hooks and related proxy settings fixes.

chrome/browser/chromeos/proxy_cros_settings_provider.cc

Index: chrome/browser/chromeos/proxy_cros_settings_provider.cc
diff --git a/chrome/browser/chromeos/proxy_cros_settings_provider.cc b/chrome/browser/chromeos/proxy_cros_settings_provider.cc
index ccb3e44edf1fab261b619e4edb337dd557fd34c9..fbaacbd550ff62f27fa9ff551cbe12ab9259381a 100644
--- a/chrome/browser/chromeos/proxy_cros_settings_provider.cc
+++ b/chrome/browser/chromeos/proxy_cros_settings_provider.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -57,50 +57,42 @@ void ProxyCrosSettingsProvider::DoSet(const std::string& path,
   } else if (path == kProxySingleHttp) {
     std::string val;
     if (in_value->GetAsString(&val)) {
-      std::string uri = val;
-      AppendPortIfValid(kProxySingleHttpPort, &uri);
-      config_service->UISetProxyConfigToSingleProxy(
-          net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
+      config_service->UISetProxyConfigToSingleProxy(CreateProxyServerFromHost(
+          val, config.single_proxy, net::ProxyServer::SCHEME_HTTP));

[xiyuan, 2011/04/04 21:24:20]

You need AppendPortIfValid to get cached user input because underlying proxy
service does not remember invalid input.
e.g.
if user enters port first without a valid uri, then port will be lost and revert
to default port.

[dtu, 2011/04/04 22:48:57]

This was true previously, when this function was creating a URI string and using
FromURI to parse a ProxyServer from that string. But now it uses the ProxyServer
constructor directly and will happily accept arbitrary URIs.

[xiyuan, 2011/04/04 23:31:18]

It would be great that the backend proxy service would remeber arbitrary values.

Please make sure you test this case:
1. Go to proxy settings and enter an invalid host name (simulate user typo);
2. Enter a valid port number;
3. Click anywhere on settings page to trigger a blur event so that the change is
saved;
4. Refresh proxy settings page and see if the port number is correct.

If that works, then I am okay.

[dtu, 2011/04/05 00:04:11]

I have tried it and it works =)

[xiyuan, 2011/04/05 16:32:49]

Cool. Thanks for trying this out.

Then we don't need a local cache for the user typed values at all. Please remove
cache_ and its references. Or at least put a TODO for this.

[dtu, 2011/04/05 18:48:49]

Huh, I thought I had already done that. Done now.

     }
   } else if (path == kProxySingleHttpPort) {
-    std::string val;
-    if (in_value->GetAsString(&val)) {
-      std::string uri;
-      FormServerUriIfValid(kProxySingleHttp, val, &uri);
-      config_service->UISetProxyConfigToSingleProxy(
-          net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
+    int val;
+    if (in_value->GetAsInteger(&val)) {
+      config_service->UISetProxyConfigToSingleProxy(CreateProxyServerFromPort(
+          val, config.single_proxy, net::ProxyServer::SCHEME_HTTP));

[xiyuan, 2011/04/04 21:24:20]

similar, you need FormServerUriIfValid.

We might want to get it a better name though. FormServerUriIfValid and
AppendPortIfValid is not intutive and does not tell you what they do.

[dtu, 2011/04/04 22:48:57]

See above.

     }
   } else if (path == kProxyHttpUrl) {
     std::string val;
     if (in_value->GetAsString(&val)) {
-      std::string uri = val;
-      AppendPortIfValid(kProxyHttpPort, &uri);
       config_service->UISetProxyConfigToProxyPerScheme("http",
-          net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
+          CreateProxyServerFromHost(
+              val, config.http_proxy, net::ProxyServer::SCHEME_HTTP));
     }
   } else if (path == kProxyHttpPort) {
-    std::string val;
-    if (in_value->GetAsString(&val)) {
-      std::string uri;
-      FormServerUriIfValid(kProxyHttpUrl, val, &uri);
+    int val;
+    if (in_value->GetAsInteger(&val)) {
       config_service->UISetProxyConfigToProxyPerScheme("http",
-         net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
+          CreateProxyServerFromPort(
+              val, config.http_proxy, net::ProxyServer::SCHEME_HTTP));
     }
   } else if (path == kProxyHttpsUrl) {
     std::string val;
     if (in_value->GetAsString(&val)) {
-      std::string uri = val;
-      AppendPortIfValid(kProxyHttpsPort, &uri);
       config_service->UISetProxyConfigToProxyPerScheme("https",
-          net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
+          CreateProxyServerFromHost(
+              val, config.https_proxy, net::ProxyServer::SCHEME_HTTPS));
     }
   } else if (path == kProxyHttpsPort) {
-    std::string val;
-    if (in_value->GetAsString(&val)) {
-      std::string uri;
-      FormServerUriIfValid(kProxyHttpsUrl, val, &uri);
+    int val;
+    if (in_value->GetAsInteger(&val)) {
       config_service->UISetProxyConfigToProxyPerScheme("https",
-          net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
+          CreateProxyServerFromPort(
+              val, config.https_proxy, net::ProxyServer::SCHEME_HTTPS));
     }
   } else if (path == kProxyType) {
     int val;
@@ -159,40 +151,35 @@ void ProxyCrosSettingsProvider::DoSet(const std::string& path,
   } else if (path == kProxyFtpUrl) {
     std::string val;
     if (in_value->GetAsString(&val)) {
-      std::string uri = val;
-      AppendPortIfValid(kProxyFtpPort, &uri);
       config_service->UISetProxyConfigToProxyPerScheme("ftp",
-          net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
+          CreateProxyServerFromHost(
+              val, config.ftp_proxy, net::ProxyServer::SCHEME_HTTP));
     }
   } else if (path == kProxyFtpPort) {
-    std::string val;
-    if (in_value->GetAsString(&val)) {
-      std::string uri;
-      FormServerUriIfValid(kProxyFtpUrl, val, &uri);
+    int val;
+    if (in_value->GetAsInteger(&val)) {
       config_service->UISetProxyConfigToProxyPerScheme("ftp",
-          net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
+          CreateProxyServerFromPort(
+              val, config.ftp_proxy, net::ProxyServer::SCHEME_HTTP));
     }
   } else if (path == kProxySocks) {
     std::string val;
     if (in_value->GetAsString(&val)) {
-      std::string uri = val;
-      AppendPortIfValid(kProxySocksPort, &uri);
       config_service->UISetProxyConfigToProxyPerScheme("socks",
-          net::ProxyServer::FromURI(uri,
-              StartsWithASCII(uri, "socks4://", false) ?
-                  net::ProxyServer::SCHEME_SOCKS4 :
-                  net::ProxyServer::SCHEME_SOCKS5));
+          CreateProxyServerFromHost(val, config.socks_proxy,
+                                    StartsWithASCII(val, "socks5://", false) ?
+                                        net::ProxyServer::SCHEME_SOCKS5 :
+                                        net::ProxyServer::SCHEME_SOCKS4));
     }
   } else if (path == kProxySocksPort) {
-    std::string val;
-    if (in_value->GetAsString(&val)) {
-      std::string uri;
-      FormServerUriIfValid(kProxySocks, val, &uri);
+    int val;
+    if (in_value->GetAsInteger(&val)) {
+      std::string host = config.socks_proxy.server.host_port_pair().host();
       config_service->UISetProxyConfigToProxyPerScheme("socks",
-          net::ProxyServer::FromURI(uri,
-              StartsWithASCII(uri, "socks4://", false) ?
-                  net::ProxyServer::SCHEME_SOCKS4 :
-                  net::ProxyServer::SCHEME_SOCKS5));
+          CreateProxyServerFromPort(val, config.socks_proxy,
+                                    StartsWithASCII(host, "socks5://", false) ?
+                                        net::ProxyServer::SCHEME_SOCKS5 :
+                                        net::ProxyServer::SCHEME_SOCKS4));
     }
   } else if (path == kProxyIgnoreList) {
     net::ProxyBypassRules bypass_rules;
@@ -294,22 +281,24 @@ chromeos::ProxyConfigServiceImpl*
   return g_browser_process->chromeos_proxy_config_service_impl();
 }
 
-void ProxyCrosSettingsProvider::AppendPortIfValid(
-    const char* port_cache_key,
-    std::string* server_uri) {
-  std::string port;
-  if (!server_uri->empty() && cache_.GetString(port_cache_key, &port) &&
-      !port.empty()) {
-    *server_uri += ":" + port;
-  }
+net::ProxyServer ProxyCrosSettingsProvider::CreateProxyServerFromHost(
+    const std::string& host,
+    const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy,
+    net::ProxyServer::Scheme scheme) const {
+  uint16 port = proxy.server.host_port_pair().port();
+  if (!port)
+    port = net::ProxyServer::GetDefaultPortForScheme(scheme);
+  net::HostPortPair host_port_pair(host, port);
+  return net::ProxyServer(scheme, host_port_pair);
 }
 
-void ProxyCrosSettingsProvider::FormServerUriIfValid(
-    const char* host_cache_key,
-    const std::string& port_num, std::string* server_uri) {
-  if (cache_.GetString(host_cache_key, server_uri) && !server_uri->empty() &&
-      !port_num.empty())
-    *server_uri += ":" + port_num;
+net::ProxyServer ProxyCrosSettingsProvider::CreateProxyServerFromPort(
+    uint16 port,
+    const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy,
+    net::ProxyServer::Scheme scheme) const {
+  std::string host = proxy.server.host_port_pair().host();
+  net::HostPortPair host_port_pair(host, port);
+  return net::ProxyServer(scheme, host_port_pair);
 }
 
 Value* ProxyCrosSettingsProvider::CreateServerHostValue(