A bunch of proxy DOMUI changes.

- Make sure invalid manual proxy changes can be changed to valid;
- Disable single proxy when manual proxy is not checked;
- Clearing manual settings take effect;

Kuan fixed the backend and I fixed the DOMUI.

BUG=chromium-os:8703, chromium-os:8683, chromium-os:8710
TEST=Verify per bug report.

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

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

6 files changed, 125 insertions, 125 deletions

diff --git a/chrome/browser/browser_resources.grd b/chrome/browser/browser_resources.grd
index 82c0731..da04f3a 100644
--- a/chrome/browser/browser_resources.grd
+++ b/chrome/browser/browser_resources.grd
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!-- This comment is only here because changes to resources are not picked up
-without changes to the corresponding grd file. et -->
+without changes to the corresponding grd file. etaa -->
 <grit latest_public_release="0" current_release="1">
   <outputs>
     <output filename="grit/browser_resources.h" type="rc_header">
diff --git a/chrome/browser/chromeos/cros_settings_provider_proxy.cc b/chrome/browser/chromeos/cros_settings_provider_proxy.cc
index 5e88cb4..5f113ee 100644
--- a/chrome/browser/chromeos/cros_settings_provider_proxy.cc
+++ b/chrome/browser/chromeos/cros_settings_provider_proxy.cc
@@ -11,6 +11,21 @@
 
 namespace chromeos {
 
+static const char kProxyPacUrl[]         = "cros.proxy.pacurl";
+static const char kProxySingleHttp[]     = "cros.proxy.singlehttp";
+static const char kProxySingleHttpPort[] = "cros.proxy.singlehttpport";
+static const char kProxyHttpUrl[]        = "cros.proxy.httpurl";
+static const char kProxyHttpPort[]       = "cros.proxy.httpport";
+static const char kProxyHttpsUrl[]       = "cros.proxy.httpsurl";
+static const char kProxyHttpsPort[]      = "cros.proxy.httpsport";
+static const char kProxyType[]           = "cros.proxy.type";
+static const char kProxySingle[]         = "cros.proxy.single";
+static const char kProxyFtpUrl[]         = "cros.proxy.ftpurl";
+static const char kProxyFtpPort[]        = "cros.proxy.ftpport";
+static const char kProxySocks[]          = "cros.proxy.socks";
+static const char kProxySocksPort[]      = "cros.proxy.socksport";
+static const char kProxyIgnoreList[]     = "cros.proxy.ignorelist";
+
 //------------------ CrosSettingsProviderProxy: public methods -----------------
 
 CrosSettingsProviderProxy::CrosSettingsProviderProxy() { }
@@ -20,50 +35,69 @@ void CrosSettingsProviderProxy::DoSet(const std::string& path,
   if (!in_value) {
     return;
   }
+
+  // Keep whatever user inputs so that we could use it later.
+  SetCache(path, in_value);
+
   chromeos::ProxyConfigServiceImpl* config_service = GetConfigService();
   chromeos::ProxyConfigServiceImpl::ProxyConfig config;
   config_service->UIGetProxyConfig(&config);
 
-  if (path == "cros.proxy.pacurl") {
+  if (path == kProxyPacUrl) {
     std::string val;
     if (in_value->GetAsString(&val)) {
       GURL url(val);
       config_service->UISetProxyConfigToPACScript(url);
     }
-  } else if (path == "cros.proxy.singlehttp") {
+  } else if (path == kProxySingleHttp) {
     std::string val;
     if (in_value->GetAsString(&val)) {
       std::string uri = val;
-      AppendPortIfValid(config.single_proxy, &uri);
+      AppendPortIfValid(kProxySingleHttpPort, &uri);
       config_service->UISetProxyConfigToSingleProxy(
           net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
     }
-  } else if (path == "cros.proxy.singlehttpport") {
+  } else if (path == kProxySingleHttpPort) {
     std::string val;
     if (in_value->GetAsString(&val)) {
       std::string uri;
-      if (FormServerUriIfValid(config.single_proxy, val, &uri)) {
-        config_service->UISetProxyConfigToSingleProxy(
-            net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
-      }
+      FormServerUriIfValid(kProxySingleHttp, val, &uri);
+      config_service->UISetProxyConfigToSingleProxy(
+          net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
     }
-  } else if (path == "cros.proxy.httpurl") {
+  } else if (path == kProxyHttpUrl) {
     std::string val;
     if (in_value->GetAsString(&val)) {
       std::string uri = val;
-      AppendPortIfValid(config.http_proxy, &uri);
+      AppendPortIfValid(kProxyHttpPort, &uri);
       config_service->UISetProxyConfigToProxyPerScheme("http",
           net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
     }
-  } else if (path == "cros.proxy.httpsurl") {
+  } else if (path == kProxyHttpPort) {
+    std::string val;
+    if (in_value->GetAsString(&val)) {
+      std::string uri;
+      FormServerUriIfValid(kProxyHttpUrl, val, &uri);
+      config_service->UISetProxyConfigToProxyPerScheme("http",
+         net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
+    }
+  } else if (path == kProxyHttpsUrl) {
     std::string val;
     if (in_value->GetAsString(&val)) {
       std::string uri = val;
-      AppendPortIfValid(config.https_proxy, &uri);
+      AppendPortIfValid(kProxyHttpsPort, &uri);
       config_service->UISetProxyConfigToProxyPerScheme("https",
           net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTPS));
     }
-  } else if (path == "cros.proxy.type") {
+  } else if (path == kProxyHttpsPort) {
+    std::string val;
+    if (in_value->GetAsString(&val)) {
+      std::string uri;
+      FormServerUriIfValid(kProxyHttpsUrl, val, &uri);
+      config_service->UISetProxyConfigToProxyPerScheme("https",
+          net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTPS));
+    }
+  } else if (path == kProxyType) {
     int val;
     if (in_value->GetAsInteger(&val)) {
       if (val == 3) {
@@ -107,7 +141,7 @@ void CrosSettingsProviderProxy::DoSet(const std::string& path,
         config_service->UISetProxyConfigToDirect();
       }
     }
-  } else if (path == "cros.proxy.single") {
+  } else if (path == kProxySingle) {
     bool val;
     if (in_value->GetAsBoolean(&val)) {
       if (val)
@@ -117,59 +151,39 @@ void CrosSettingsProviderProxy::DoSet(const std::string& path,
         config_service->UISetProxyConfigToProxyPerScheme("http",
             config.http_proxy.server);
     }
-  } else if (path == "cros.proxy.ftpurl") {
+  } else if (path == kProxyFtpUrl) {
     std::string val;
     if (in_value->GetAsString(&val)) {
       std::string uri = val;
-      AppendPortIfValid(config.ftp_proxy, &uri);
+      AppendPortIfValid(kProxyFtpPort, &uri);
       config_service->UISetProxyConfigToProxyPerScheme("ftp",
           net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
     }
-  } else if (path == "cros.proxy.socks") {
-    std::string val;
-    if (in_value->GetAsString(&val)) {
-      std::string uri = val;
-      AppendPortIfValid(config.socks_proxy, &uri);
-      config_service->UISetProxyConfigToProxyPerScheme("socks",
-         net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_SOCKS4));
-    }
-  } else if (path == "cros.proxy.httpport") {
+  } else if (path == kProxyFtpPort) {
     std::string val;
     if (in_value->GetAsString(&val)) {
       std::string uri;
-      if (FormServerUriIfValid(config.http_proxy, val, &uri)) {
-        config_service->UISetProxyConfigToProxyPerScheme("http",
-           net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
-      }
-    }
-  } else if (path == "cros.proxy.httpsport") {
-    std::string val;
-    if (in_value->GetAsString(&val)) {
-      std::string uri;
-      if (FormServerUriIfValid(config.https_proxy, val, &uri)) {
-        config_service->UISetProxyConfigToProxyPerScheme("https",
-            net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTPS));
-      }
+      FormServerUriIfValid(kProxyFtpUrl, val, &uri);
+      config_service->UISetProxyConfigToProxyPerScheme("ftp",
+          net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
     }
-  } else if (path == "cros.proxy.ftpport") {
+  } else if (path == kProxySocks) {
     std::string val;
     if (in_value->GetAsString(&val)) {
-      std::string uri;
-      if (FormServerUriIfValid(config.ftp_proxy, val, &uri)) {
-        config_service->UISetProxyConfigToProxyPerScheme("ftp",
-            net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_HTTP));
-      }
+      std::string uri = val;
+      AppendPortIfValid(kProxySocksPort, &uri);
+      config_service->UISetProxyConfigToProxyPerScheme("socks",
+         net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_SOCKS4));
     }
-  } else if (path == "cros.proxy.socksport") {
+  } else if (path == kProxySocksPort) {
     std::string val;
     if (in_value->GetAsString(&val)) {
       std::string uri;
-      if (FormServerUriIfValid(config.socks_proxy, val, &uri)) {
-        config_service->UISetProxyConfigToProxyPerScheme("socks",
-            net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_SOCKS5));
-      }
+      FormServerUriIfValid(kProxySocks, val, &uri);
+      config_service->UISetProxyConfigToProxyPerScheme("socks",
+          net::ProxyServer::FromURI(uri, net::ProxyServer::SCHEME_SOCKS4));
     }
-  } else if (path == "cros.proxy.ignorelist") {
+  } else if (path == kProxyIgnoreList) {
     net::ProxyBypassRules bypass_rules;
     if (in_value->GetType() == Value::TYPE_LIST) {
       const ListValue* list_value = static_cast<const ListValue*>(in_value);
@@ -193,20 +207,20 @@ bool CrosSettingsProviderProxy::Get(const std::string& path,
   chromeos::ProxyConfigServiceImpl::ProxyConfig config;
   config_service->UIGetProxyConfig(&config);
 
-  if (path == "cros.proxy.pacurl") {
+  if (path == kProxyPacUrl) {
     if (config.automatic_proxy.pac_url.is_valid()) {
       data = Value::CreateStringValue(config.automatic_proxy.pac_url.spec());
       found = true;
     }
-  } else if (path == "cros.proxy.singlehttp") {
+  } else if (path == kProxySingleHttp) {
     found = (data = CreateServerHostValue(config.single_proxy));
-  } else if (path == "cros.proxy.singlehttpport") {
+  } else if (path == kProxySingleHttpPort) {
     found = (data = CreateServerPortValue(config.single_proxy));
-  } else if (path == "cros.proxy.httpurl") {
+  } else if (path == kProxyHttpUrl) {
     found = (data = CreateServerHostValue(config.http_proxy));
-  } else if (path == "cros.proxy.httpsurl") {
+  } else if (path == kProxyHttpsUrl) {
     found = (data = CreateServerHostValue(config.https_proxy));
-  } else if (path == "cros.proxy.type") {
+  } else if (path == kProxyType) {
     if (config.mode ==
         chromeos::ProxyConfigServiceImpl::ProxyConfig::MODE_AUTO_DETECT ||
         config.mode ==
@@ -221,23 +235,23 @@ bool CrosSettingsProviderProxy::Get(const std::string& path,
       data = Value::CreateIntegerValue(1);
     }
     found = true;
-  } else if (path == "cros.proxy.single") {
+  } else if (path == kProxySingle) {
     data = Value::CreateBooleanValue(config.mode ==
         chromeos::ProxyConfigServiceImpl::ProxyConfig::MODE_SINGLE_PROXY);
     found = true;
-  } else if (path == "cros.proxy.ftpurl") {
+  } else if (path == kProxyFtpUrl) {
     found = (data = CreateServerHostValue(config.ftp_proxy));
-  } else if (path == "cros.proxy.socks") {
+  } else if (path == kProxySocks) {
     found = (data = CreateServerHostValue(config.socks_proxy));
-  } else if (path == "cros.proxy.httpport") {
+  } else if (path == kProxyHttpPort) {
     found = (data = CreateServerPortValue(config.http_proxy));
-  } else if (path == "cros.proxy.httpsport") {
+  } else if (path == kProxyHttpsPort) {
     found = (data = CreateServerPortValue(config.https_proxy));
-  } else if (path == "cros.proxy.ftpport") {
+  } else if (path == kProxyFtpPort) {
     found = (data = CreateServerPortValue(config.ftp_proxy));
-  } else if (path == "cros.proxy.socksport") {
+  } else if (path == kProxySocksPort) {
     found = (data = CreateServerPortValue(config.socks_proxy));
-  } else if (path == "cros.proxy.ignorelist") {
+  } else if (path == kProxyIgnoreList) {
     ListValue* list =  new ListValue();
     net::ProxyBypassRules::RuleList bypass_rules = config.bypass_rules.rules();
     for (size_t x = 0; x < bypass_rules.size(); x++) {
@@ -275,19 +289,21 @@ chromeos::ProxyConfigServiceImpl*
 }
 
 void CrosSettingsProviderProxy::AppendPortIfValid(
-    const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy,
+    const char* port_cache_key,
     std::string* server_uri) {
-  if (proxy.server.is_valid())
-    *server_uri += ":" + proxy.server.host_port_pair().port();
+  std::string port;
+  if (!server_uri->empty() && cache_.GetString(port_cache_key, &port) &&
+      !port.empty()) {
+    *server_uri += ":" + port;
+  }
 }
 
-bool CrosSettingsProviderProxy::FormServerUriIfValid(
-    const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy,
+void CrosSettingsProviderProxy::FormServerUriIfValid(
+    const char* host_cache_key,
     const std::string& port_num, std::string* server_uri) {
-  if (!proxy.server.is_valid())
-    return false;
-  *server_uri = proxy.server.host_port_pair().host() + ":" + port_num;
-  return true;
+  if (cache_.GetString(host_cache_key, server_uri) && !server_uri->empty() &&
+      !port_num.empty())
+    *server_uri += ":" + port_num;
 }
 
 Value* CrosSettingsProviderProxy::CreateServerHostValue(
@@ -304,4 +320,9 @@ Value* CrosSettingsProviderProxy::CreateServerPortValue(
          NULL;
 }
 
+void CrosSettingsProviderProxy::SetCache(const std::string& key,
+    const Value* value) {
+  cache_.Set(key, value->DeepCopy());
+}
+
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/cros_settings_provider_proxy.h b/chrome/browser/chromeos/cros_settings_provider_proxy.h
index afb24ad..1b1672bc 100644
--- a/chrome/browser/chromeos/cros_settings_provider_proxy.h
+++ b/chrome/browser/chromeos/cros_settings_provider_proxy.h
@@ -26,12 +26,9 @@ class CrosSettingsProviderProxy : public CrosSettingsProvider {
 
   chromeos::ProxyConfigServiceImpl* GetConfigService() const;
 
-  void AppendPortIfValid(
-      const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy,
-      std::string* server_uri);
+  void AppendPortIfValid(const char* port_cache_key, std::string* server_uri);
 
-  bool FormServerUriIfValid(
-      const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy,
+  void FormServerUriIfValid(const char* host_cache_key,
       const std::string& port_num, std::string* server_uri);
 
   Value* CreateServerHostValue(
@@ -40,6 +37,11 @@ class CrosSettingsProviderProxy : public CrosSettingsProvider {
   Value* CreateServerPortValue(
       const ProxyConfigServiceImpl::ProxyConfig::ManualProxy& proxy) const;
 
+  void SetCache(const std::string& key, const Value* value);
+
+  // A cache to keep whatever user typed.
+  DictionaryValue cache_;
+
   DISALLOW_COPY_AND_ASSIGN(CrosSettingsProviderProxy);
 };
 
diff --git a/chrome/browser/chromeos/proxy_config_service_impl.cc b/chrome/browser/chromeos/proxy_config_service_impl.cc
index 539afad..73693fd 100644
--- a/chrome/browser/chromeos/proxy_config_service_impl.cc
+++ b/chrome/browser/chromeos/proxy_config_service_impl.cc
@@ -409,12 +409,8 @@ bool ProxyConfigServiceImpl::UISetProxyConfigToPACScript(const GURL& pac_url) {
   CheckCurrentlyOnUIThread();
   reference_config_.mode = ProxyConfig::MODE_PAC_SCRIPT;
   reference_config_.automatic_proxy.pac_url = pac_url;
-  if (pac_url.is_valid()) {
-    OnUISetProxyConfig(true);
-    return true;
-  }
-  VLOG(1) << "Cannot set proxy: invalid pac url";
-  return false;
+  OnUISetProxyConfig(true);
+  return true;
 }
 
 bool ProxyConfigServiceImpl::UISetProxyConfigToSingleProxy(
@@ -423,12 +419,8 @@ bool ProxyConfigServiceImpl::UISetProxyConfigToSingleProxy(
   CheckCurrentlyOnUIThread();
   reference_config_.mode = ProxyConfig::MODE_SINGLE_PROXY;
   reference_config_.single_proxy.server = server;
-  if (server.is_valid()) {
-    OnUISetProxyConfig(true);
-    return true;
-  }
-  VLOG(1) << "Cannot set proxy: invalid single server";
-  return false;
+  OnUISetProxyConfig(true);
+  return true;
 }
 
 bool ProxyConfigServiceImpl::UISetProxyConfigToProxyPerScheme(
@@ -450,12 +442,8 @@ bool ProxyConfigServiceImpl::UISetProxyConfigToProxyPerScheme(
   }
   reference_config_.mode = ProxyConfig::MODE_PROXY_PER_SCHEME;
   proxy->server = server;
-  if (server.is_valid()) {
-    OnUISetProxyConfig(true);
-    return true;
-  }
-  VLOG(1) << "Cannot set proxy: invalid " << scheme << " server";
-  return false;
+  OnUISetProxyConfig(true);
+  return true;
 }
 
 bool ProxyConfigServiceImpl::UISetProxyConfigBypassRules(
diff --git a/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc b/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc
index 5e5f0f3..46fc3ff 100644
--- a/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc
+++ b/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc
@@ -388,55 +388,42 @@ TEST_F(ProxyConfigServiceImplTest, ModifyFromUI) {
     const Input& input = tests[i].input;
     switch (input.mode) {
       case MK_MODE(DIRECT) :
-        EXPECT_EQ(tests[i].is_valid,
-                  config_service()->UISetProxyConfigToDirect());
+        config_service()->UISetProxyConfigToDirect();
         break;
       case MK_MODE(AUTO_DETECT) :
-        EXPECT_EQ(tests[i].is_valid,
-                  config_service()->UISetProxyConfigToAutoDetect());
+        config_service()->UISetProxyConfigToAutoDetect();
         break;
       case MK_MODE(PAC_SCRIPT) :
-        EXPECT_EQ(tests[i].is_valid,
-                  config_service()->UISetProxyConfigToPACScript(
-                      GURL(input.pac_url)));
+        config_service()->UISetProxyConfigToPACScript(GURL(input.pac_url));
         break;
       case MK_MODE(SINGLE_PROXY) :
-        EXPECT_EQ(tests[i].is_valid,
-                  config_service()->UISetProxyConfigToSingleProxy(
-                      net::ProxyServer::FromURI(input.single_uri,
-                                                MK_SCHM(HTTP))));
+        config_service()->UISetProxyConfigToSingleProxy(
+            net::ProxyServer::FromURI(input.single_uri, MK_SCHM(HTTP)));
         if (input.bypass_rules) {
           bypass_rules.ParseFromStringUsingSuffixMatching(input.bypass_rules);
-          EXPECT_EQ(tests[i].is_valid,
-                    config_service()->UISetProxyConfigBypassRules(
-                        bypass_rules));
+          config_service()->UISetProxyConfigBypassRules(bypass_rules);
         }
         break;
       case MK_MODE(PROXY_PER_SCHEME) :
         if (input.http_uri) {
-          EXPECT_EQ(tests[i].is_valid,
-              config_service()->UISetProxyConfigToProxyPerScheme("http",
-                  net::ProxyServer::FromURI(input.http_uri, MK_SCHM(HTTP))));
+          config_service()->UISetProxyConfigToProxyPerScheme("http",
+                  net::ProxyServer::FromURI(input.http_uri, MK_SCHM(HTTP)));
         }
         if (input.https_uri) {
-          EXPECT_EQ(tests[i].is_valid,
-              config_service()->UISetProxyConfigToProxyPerScheme("https",
-                  net::ProxyServer::FromURI(input.https_uri, MK_SCHM(HTTPS))));
+          config_service()->UISetProxyConfigToProxyPerScheme("https",
+              net::ProxyServer::FromURI(input.https_uri, MK_SCHM(HTTPS)));
         }
         if (input.ftp_uri) {
-          EXPECT_EQ(tests[i].is_valid,
-              config_service()->UISetProxyConfigToProxyPerScheme("ftp",
-                  net::ProxyServer::FromURI(input.ftp_uri, MK_SCHM(HTTP))));
+          config_service()->UISetProxyConfigToProxyPerScheme("ftp",
+              net::ProxyServer::FromURI(input.ftp_uri, MK_SCHM(HTTP)));
         }
         if (input.socks_uri) {
-          EXPECT_EQ(tests[i].is_valid,
-              config_service()->UISetProxyConfigToProxyPerScheme("socks",
-                  net::ProxyServer::FromURI(input.socks_uri, MK_SCHM(SOCKS4))));
+          config_service()->UISetProxyConfigToProxyPerScheme("socks",
+              net::ProxyServer::FromURI(input.socks_uri, MK_SCHM(SOCKS4)));
         }
         if (input.bypass_rules) {
           bypass_rules.ParseFromStringUsingSuffixMatching(input.bypass_rules);
-          EXPECT_EQ(tests[i].is_valid,
-              config_service()->UISetProxyConfigBypassRules(bypass_rules));
+          config_service()->UISetProxyConfigBypassRules(bypass_rules);
         }
         break;
     }
diff --git a/chrome/browser/resources/options/chromeos_proxy_options.js b/chrome/browser/resources/options/chromeos_proxy_options.js
index d2fcef7..b1e7f16 100644
--- a/chrome/browser/resources/options/chromeos_proxy_options.js
+++ b/chrome/browser/resources/options/chromeos_proxy_options.js
@@ -91,6 +91,7 @@ cr.define('options', function() {
      * @param {Event} e Click event.
      */
     disableManual_: function(e) {
+      $('proxyAllProtocols').disabled = true;
       $('proxyHostName').disabled = true;
       $('proxyHostPort').disabled = true;
       $('proxyHostSingleName').disabled = true;
@@ -114,6 +115,7 @@ cr.define('options', function() {
      * @param {Event} e Click event.
      */
     enableManual_: function(e) {
+      $('proxyAllProtocols').disabled = false;
       $('proxyHostName').disabled = false;
       $('proxyHostPort').disabled = false;
       $('proxyHostSingleName').disabled = false;