[Network Settings] Remove FTP proxy from manual proxy config UI

This CL removes FTP proxy fields from manual proxy configuration UI.
FTP over HTTP proxy is no longer supported.

Bug: 1032168
Change-Id: I524cc2f34af8c21bdfa04d389b65fefa29732066
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1984715Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Azeem Arshad <azeemarshad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728101}

chromeos/network/proxy/ui_proxy_config_service.cc

@@ -115,11 +115,9 @@ base::Value OncValueForManualProxyList(
     const std::string& source,
     const net::ProxyList& for_http,
     const net::ProxyList& for_https,
-    const net::ProxyList& for_ftp,
     const net::ProxyList& fallback,
     const net::ProxyBypassRules& bypass_rules) {
-  if (for_http.IsEmpty() && for_https.IsEmpty() && for_ftp.IsEmpty() &&
-      fallback.IsEmpty()) {
+  if (for_http.IsEmpty() && for_https.IsEmpty() && fallback.IsEmpty()) {
     return base::Value();
   }
   base::Value result = OncValueWithMode(source, ::onc::proxy::kManual);
@@ -128,7 +126,6 @@ base::Value OncValueForManualProxyList(
       ::onc::proxy::kManual, base::Value(base::Value::Type::DICTIONARY));
   SetManualProxy(manual, source, ::onc::proxy::kHttp, for_http);
   SetManualProxy(manual, source, ::onc::proxy::kHttps, for_https);
-  SetManualProxy(manual, source, ::onc::proxy::kFtp, for_ftp);
   SetManualProxy(manual, source, ::onc::proxy::kSocks, fallback);
 
   base::Value exclude_domains(base::Value::Type::LIST);
@@ -171,13 +168,11 @@ base::Value NetProxyConfigAsOncValue(const net::ProxyConfig& net_config,
                                         net_config.proxy_rules().single_proxies,
                                         net_config.proxy_rules().single_proxies,
                                         net_config.proxy_rules().single_proxies,
-                                        net_config.proxy_rules().single_proxies,
                                         net_config.proxy_rules().bypass_rules);
     case net::ProxyConfig::ProxyRules::Type::PROXY_LIST_PER_SCHEME:
       return OncValueForManualProxyList(
           source, net_config.proxy_rules().proxies_for_http,
           net_config.proxy_rules().proxies_for_https,
-          net_config.proxy_rules().proxies_for_ftp,
           net_config.proxy_rules().fallback_proxies,
           net_config.proxy_rules().bypass_rules);
   }

chromeos/network/proxy/ui_proxy_config_service_unittest.cc

@@ -316,8 +316,7 @@ TEST_F(UIProxyConfigServiceTest, ManualPolicyPref) {
   std::unique_ptr<UIProxyConfigService> service = CreateServiceForUser();
 
   base::Value policy_prefs_config = ProxyConfigDictionary::CreateFixedServers(
-      "http=proxy1:81;https=proxy2:81;ftp=proxy3:81;socks=proxy4:81",
-      "localhost");
+      "http=proxy1:81;https=proxy2:81;socks=proxy3:81", "localhost");
   user_prefs_.SetManagedPref(
       proxy_config::prefs::kProxy,
       base::Value::ToUniquePtrValue(std::move(policy_prefs_config)));
@@ -328,17 +327,15 @@ TEST_F(UIProxyConfigServiceTest, ManualPolicyPref) {
   std::string expected_json = base::ReplaceStringPlaceholders(
       R"({"Type": $1,
           "Manual": {
-            "HTTPProxy": {"Host": $2, "Port": $6},
-            "SecureHTTPProxy": {"Host": $3, "Port": $6},
-            "FTPProxy": {"Host": $4, "Port": $6},
-            "SOCKS": {"Host": $5, "Port": $6}
+            "HTTPProxy": {"Host": $2, "Port": $5},
+            "SecureHTTPProxy": {"Host": $3, "Port": $5},
+            "SOCKS": {"Host": $4, "Port": $5}
           },
-          "ExcludeDomains": $7
+          "ExcludeDomains": $6
           })",
       {UserPolicyOncValue(R"("Manual")"), UserPolicyOncValue(R"("proxy1")"),
        UserPolicyOncValue(R"("proxy2")"), UserPolicyOncValue(R"("proxy3")"),
-       UserPolicyOncValue(R"("proxy4")"), UserPolicyOncValue("81"),
-       UserPolicyOncValue(R"(["localhost"])")},
+       UserPolicyOncValue("81"), UserPolicyOncValue(R"(["localhost"])")},
       nullptr);
   std::unique_ptr<base::Value> expected =
       base::JSONReader::ReadDeprecated(expected_json);
@@ -349,8 +346,8 @@ TEST_F(UIProxyConfigServiceTest, ManualPolicyPref) {
 TEST_F(UIProxyConfigServiceTest, PartialManualPolicyPref) {
   std::unique_ptr<UIProxyConfigService> service = CreateServiceForUser();
 
-  base::Value policy_prefs_config = ProxyConfigDictionary::CreateFixedServers(
-      "http=proxy1:81;ftp=proxy3:83;", "");
+  base::Value policy_prefs_config =
+      ProxyConfigDictionary::CreateFixedServers("http=proxy1:81;", "");
   user_prefs_.SetManagedPref(
       proxy_config::prefs::kProxy,
       base::Value::ToUniquePtrValue(std::move(policy_prefs_config)));
@@ -362,15 +359,13 @@ TEST_F(UIProxyConfigServiceTest, PartialManualPolicyPref) {
       R"({"Type": $1,
           "Manual": {
             "HTTPProxy": {"Host": $2, "Port": $3},
-            "FTPProxy": {"Host": $4, "Port": $5},
-            "SecureHTTPProxy": {"Host": $6, "Port": $7},
-            "SOCKS": {"Host": $6, "Port": $7}
+            "SecureHTTPProxy": {"Host": $4, "Port": $5},
+            "SOCKS": {"Host": $4, "Port": $5}
           },
-          "ExcludeDomains": $8
+          "ExcludeDomains": $6
           })",
       {UserPolicyOncValue(R"("Manual")"), UserPolicyOncValue(R"("proxy1")"),
-       UserPolicyOncValue("81"), UserPolicyOncValue(R"("proxy3")"),
-       UserPolicyOncValue("83"), UserPolicyOncValue(R"("")"),
+       UserPolicyOncValue("81"), UserPolicyOncValue(R"("")"),
        UserPolicyOncValue("0"), UserPolicyOncValue("[]")},
       nullptr);
   std::unique_ptr<base::Value> expected =
@@ -383,7 +378,7 @@ TEST_F(UIProxyConfigServiceTest, ManualPolicyPrefWithPacPreset) {
   std::unique_ptr<UIProxyConfigService> service = CreateServiceForUser();
 
   base::Value policy_prefs_config = ProxyConfigDictionary::CreateFixedServers(
-      "http=proxy:80;https=proxy:80;ftp=proxy:80;socks=proxy:80", "localhost");
+      "http=proxy:80;https=proxy:80;socks=proxy:80", "localhost");
   user_prefs_.SetManagedPref(
       proxy_config::prefs::kProxy,
       base::Value::ToUniquePtrValue(std::move(policy_prefs_config)));
@@ -405,7 +400,6 @@ TEST_F(UIProxyConfigServiceTest, ManualPolicyPrefWithPacPreset) {
           "Manual": {
             "HTTPProxy": {"Host": $2, "Port": $3},
             "SecureHTTPProxy": {"Host": $2, "Port": $3},
-            "FTPProxy": {"Host": $2, "Port": $3},
             "SOCKS": {"Host": $2, "Port": $3}
           },
           "PAC": $4,
@@ -489,8 +483,7 @@ TEST_F(UIProxyConfigServiceTest, ManualExtensionPref) {
 
   base::Value extension_prefs_config =
       ProxyConfigDictionary::CreateFixedServers(
-          "http=proxy1:81;https=proxy2:82;ftp=proxy3:83;socks=proxy4:81",
-          "localhost");
+          "http=proxy1:81;https=proxy2:82;socks=proxy3:81", "localhost");
   user_prefs_.SetExtensionPref(
       proxy_config::prefs::kProxy,
       base::Value::ToUniquePtrValue(std::move(extension_prefs_config)));
@@ -503,10 +496,9 @@ TEST_F(UIProxyConfigServiceTest, ManualExtensionPref) {
           "Manual": {
             "HTTPProxy": {"Host": $2, "Port": $3},
             "SecureHTTPProxy": {"Host": $4, "Port": $5},
-            "FTPProxy": {"Host": $6, "Port": $7},
-            "SOCKS": {"Host": $8, "Port": $3}
+            "SOCKS": {"Host": $6, "Port": $3}
           },
-          "ExcludeDomains": $9
+          "ExcludeDomains": $7
           })",
       {ExtensionControlledOncValue(R"("Manual")"),
        ExtensionControlledOncValue(R"("proxy1")"),
@@ -514,8 +506,6 @@ TEST_F(UIProxyConfigServiceTest, ManualExtensionPref) {
        ExtensionControlledOncValue(R"("proxy2")"),
        ExtensionControlledOncValue("82"),
        ExtensionControlledOncValue(R"("proxy3")"),
-       ExtensionControlledOncValue("83"),
-       ExtensionControlledOncValue(R"("proxy4")"),
        ExtensionControlledOncValue(R"(["localhost"])")},
       nullptr);
   std::unique_ptr<base::Value> expected =
@@ -723,8 +713,7 @@ TEST_F(UIProxyConfigServiceTest, ManualOncUserPolicy) {
            "Manual": {
              "HTTPProxy": {"Host": "proxy1", "Port": 81},
              "SecureHTTPProxy": {"Host": "proxy2", "Port": 82},
-             "FTPProxy": {"Host": "proxy3", "Port": 83},
-             "SOCKS": {"Host": "proxy4", "Port": 83}}}}])",
+             "SOCKS": {"Host": "proxy3", "Port": 83}}}}])",
       {kTestUserWifiGuid}, nullptr);
   user_prefs_.SetManagedPref(::onc::prefs::kOpenNetworkConfiguration,
                              base::JSONReader::ReadDeprecated(onc_config));
@@ -737,15 +726,14 @@ TEST_F(UIProxyConfigServiceTest, ManualOncUserPolicy) {
           "Manual": {
             "HTTPProxy": {"Host": $2, "Port": $3},
             "SecureHTTPProxy": {"Host": $4, "Port": $5},
-            "FTPProxy": {"Host": $6, "Port": $7},
-            "SOCKS": {"Host": $8, "Port": $7}
+            "SOCKS": {"Host": $7, "Port": $6}
           },
-          "ExcludeDomains": $9
+          "ExcludeDomains": $8
           })",
       {UserPolicyOncValue(R"("Manual")"), UserPolicyOncValue(R"("proxy1")"),
        UserPolicyOncValue("81"), UserPolicyOncValue(R"("proxy2")"),
-       UserPolicyOncValue("82"), UserPolicyOncValue(R"("proxy3")"),
-       UserPolicyOncValue("83"), UserPolicyOncValue(R"("proxy4")"),
+       UserPolicyOncValue("82"), UserPolicyOncValue("83"),
+       UserPolicyOncValue(R"("proxy3")"),
        UserPolicyOncValue(R"(["foo.test", "localhost"])")},
       nullptr);
   std::unique_ptr<base::Value> expected =
@@ -775,8 +763,7 @@ TEST_F(UIProxyConfigServiceTest, PartialManualOncUserPolicy) {
           "Manual": {
             "HTTPProxy": {"Host": $2, "Port": $3},
             "SOCKS": {"Host": $4, "Port": $5},
-            "SecureHTTPProxy": {"Host": $6, "Port": $7},
-            "FTPProxy": {"Host": $6, "Port": $7}
+            "SecureHTTPProxy": {"Host": $6, "Port": $7}
           },
           "ExcludeDomains": $8
           })",

components/onc/docs/onc_spec.md

@@ -1131,7 +1131,7 @@ Every network can be configured to use a proxy.
     * (optional) - [ProxyLocation](#ProxyLocation-type)
     * settings for secure HTTP proxy.
 
-* **FTPProxy**
+* **FTPProxy (Unsupported)**
     * (optional) - [ProxyLocation](#ProxyLocation-type)
     * settings for FTP proxy
 

ui/webui/resources/cr_components/chromeos/network/network_proxy.html

@@ -109,13 +109,6 @@
             value="{{proxy_.manual.secureHttpProxy}}"
             label="[[i18n('networkProxyShttp')]]">
         </network-proxy-input>
-        <network-proxy-input
-            on-proxy-input-change="onProxyInputChange_"
-            editable="[[isEditable_('manual.ftpProxy.host', managedProperties,
-                editable, useSharedProxies)]]"
-            value="{{proxy_.manual.ftpProxy}}"
-            label="[[i18n('networkProxyFtp')]]">
-        </network-proxy-input>
         <network-proxy-input
             on-proxy-input-change="onProxyInputChange_"
             editable="[[isEditable_('manual.socks.host', managedProperties,

ui/webui/resources/cr_components/chromeos/network/network_proxy.js

@@ -180,9 +180,6 @@ Polymer({
       if (!proxy.manual.secureHttpProxy) {
         proxy.manual.secureHttpProxy = this.createDefaultProxyLocation_(80);
       }
-      if (!proxy.manual.ftpProxy) {
-        proxy.manual.ftpProxy = this.createDefaultProxyLocation_(80);
-      }
       if (!proxy.manual.socks) {
         proxy.manual.socks = this.createDefaultProxyLocation_(1080);
       }
@@ -236,13 +233,12 @@ Polymer({
       const manual = proxy.manual;
       const httpProxy = manual.httpProxy;
       if (this.proxyMatches_(httpProxy, manual.secureHttpProxy) &&
-          this.proxyMatches_(httpProxy, manual.ftpProxy) &&
           this.proxyMatches_(httpProxy, manual.socks)) {
         // If all four proxies match, enable the 'use same proxy' toggle.
         this.useSameProxy_ = true;
       } else if (
           !manual.secureHttpProxy.host.activeValue &&
-          !manual.ftpProxy.host.activeValue && !manual.socks.host.activeValue) {
+          !manual.socks.host.activeValue) {
         // Otherwise if no proxies other than http have a host value, also
         // enable the 'use same proxy' toggle.
         this.useSameProxy_ = true;
@@ -308,7 +304,6 @@ Polymer({
           httpProxy: this.getProxyLocation_(this.proxy_.manual.httpProxy),
           secureHttpProxy:
               this.getProxyLocation_(this.proxy_.manual.secureHttpProxy),
-          ftpProxy: this.getProxyLocation_(this.proxy_.manual.ftpProxy),
           socks: this.getProxyLocation_(this.proxy_.manual.socks),
         };
       }
@@ -321,8 +316,6 @@ Polymer({
       if (this.useSameProxy_) {
         manual.secureHttpProxy = /** @type {!mojom.ProxyLocation} */ (
             Object.assign({}, defaultProxy));
-        manual.ftpProxy = /** @type {!mojom.ProxyLocation} */ (
-            Object.assign({}, defaultProxy));
         manual.socks = /** @type {!mojom.ProxyLocation} */ (
             Object.assign({}, defaultProxy));
       } else {
@@ -333,9 +326,6 @@ Polymer({
         if (manual.secureHttpProxy && !manual.secureHttpProxy.host) {
           delete manual.secureHttpProxy;
         }
-        if (manual.ftpProxy && !manual.ftpProxy.host) {
-          delete manual.ftpProxy;
-        }
         if (manual.socks && !manual.socks.host) {
           delete manual.socks;
         }
@@ -520,7 +510,6 @@ Polymer({
     }
     return !!httpHost ||
         !!this.get('secureHttpProxy.host.activeValue', manual) ||
-        !!this.get('ftpProxy.host.activeValue', manual) ||
         !!this.get('socks.host.activeValue', manual);
   },