Moves Jingle ProxyResolvingClientSocket to services/network/

ProxyResolvingClientSocket accesses socket pools, http auth cache,
client certs cache directly.  As a part of the network servicification
effort, all interactions with //net need to be done using mojo APIs.
Socket usage will also be through mojo APIs. This CL moves this class to
services/network so the API surface that we need to export is smaller.

This CL:
- Moves ProxyResolvingClientSocket to services/network.
- Adds a few additional unit tests.

See design doc:  https://docs.google.com/document/d/1iQl_Y2o7vykiPXpZiNbKov-WZbUb4RprmXIk311tTso/edit?usp=sharing.

Bug: 721401

Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Idbc0cb2e3a9713ef111ddd8b5845f27b1445574e
Reviewed-on: https://chromium-review.googlesource.com/763651
Commit-Queue: Helen Li <xunjieli@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526510}

content/browser/renderer_host/p2p/socket_host_tcp.cc

@@ -14,7 +14,6 @@
 #include "content/common/p2p_messages.h"
 #include "ipc/ipc_sender.h"
 #include "jingle/glue/fake_ssl_client_socket.h"
-#include "jingle/glue/proxy_resolving_client_socket.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/socket/client_socket_factory.h"
@@ -23,7 +22,9 @@
 #include "net/socket/tcp_client_socket.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
+#include "services/network/public/cpp/proxy_resolving_client_socket.h"
 #include "third_party/webrtc/media/base/rtputils.h"
+#include "url/gurl.h"
 
 namespace {
 
@@ -120,9 +121,10 @@ bool P2PSocketHostTcpBase::Init(const net::IPEndPoint& local_address,
 
   // The default SSLConfig is good enough for us for now.
   const net::SSLConfig ssl_config;
-  socket_.reset(new jingle_glue::ProxyResolvingClientSocket(
+  socket_ = std::make_unique<network::ProxyResolvingClientSocket>(
       nullptr,  // Default socket pool provided by the net::Proxy.
-      url_context_, ssl_config, dest_host_port_pair));
+      url_context_, ssl_config,
+      GURL("https://" + dest_host_port_pair.ToString()));
 
   int status = socket_->Connect(
       base::Bind(&P2PSocketHostTcpBase::OnConnected,

jingle/BUILD.gn

@@ -13,8 +13,6 @@ if (enable_webrtc || !is_android) {
       "glue/chrome_async_socket.h",
       "glue/fake_ssl_client_socket.cc",
       "glue/fake_ssl_client_socket.h",
-      "glue/proxy_resolving_client_socket.cc",
-      "glue/proxy_resolving_client_socket.h",
       "glue/resolving_client_socket_factory.h",
       "glue/task_pump.cc",
       "glue/task_pump.h",
@@ -32,6 +30,7 @@ if (enable_webrtc || !is_android) {
       "//base",
       "//base/third_party/dynamic_annotations",
       "//net",
+      "//services/network/public/cpp",
     ]
 
     configs += [ "//build/config/compiler:no_size_t_to_int_warning" ]
@@ -39,9 +38,9 @@ if (enable_webrtc || !is_android) {
     if (is_nacl) {
       sources -= [
         "glue/chrome_async_socket.cc",
-        "glue/proxy_resolving_client_socket.cc",
         "glue/xmpp_client_socket_factory.cc",
       ]
+      deps -= [ "//services/network/public/cpp" ]
     }
   }
 
@@ -140,7 +139,6 @@ if (enable_webrtc || !is_android) {
       "glue/logging_unittest.cc",
       "glue/mock_task.cc",
       "glue/mock_task.h",
-      "glue/proxy_resolving_client_socket_unittest.cc",
       "glue/task_pump_unittest.cc",
       "glue/thread_wrapper_unittest.cc",
       "notifier/base/weak_xmpp_client_unittest.cc",

jingle/glue/DEPS

 # Needed by logging_unittest.cc since it tests the overrides.
 include_rules = [
+  "+services/network/public/cpp",
   "+third_party/libjingle_xmpp/task_runner",
   "+third_party/webrtc",
-  '+third_party/webrtc_overrides',
+  "+third_party/webrtc_overrides",
 ]

jingle/glue/xmpp_client_socket_factory.cc

@@ -8,12 +8,12 @@
 
 #include "base/logging.h"
 #include "jingle/glue/fake_ssl_client_socket.h"
-#include "jingle/glue/proxy_resolving_client_socket.h"
 #include "net/socket/client_socket_factory.h"
 #include "net/socket/client_socket_handle.h"
 #include "net/socket/ssl_client_socket.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
+#include "services/network/public/cpp/proxy_resolving_client_socket.h"
 
 namespace jingle_glue {
 
@@ -36,8 +36,9 @@ XmppClientSocketFactory::CreateTransportClientSocket(
     const net::HostPortPair& host_and_port) {
   // TODO(akalin): Use socket pools.
   std::unique_ptr<net::StreamSocket> transport_socket(
-      new ProxyResolvingClientSocket(NULL, request_context_getter_, ssl_config_,
-                                     host_and_port));
+      new network::ProxyResolvingClientSocket(
+          nullptr, request_context_getter_, ssl_config_,
+          GURL("https://" + host_and_port.ToString())));
   return (use_fake_ssl_client_socket_
               ? std::unique_ptr<net::StreamSocket>(
                     new FakeSSLClientSocket(std::move(transport_socket)))

remoting/signaling/DEPS

 include_rules = [
   "+net",
   "+jingle",
+  "+services/network/public/cpp",
   "+third_party/webrtc/rtc_base",
   "+third_party/libjingle_xmpp/xmllite",
   "+third_party/libjingle_xmpp/xmpp",

remoting/signaling/xmpp_signal_strategy.cc

@@ -19,7 +19,6 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
-#include "jingle/glue/proxy_resolving_client_socket.h"
 #include "net/cert/cert_verifier.h"
 #include "net/cert/ct_policy_enforcer.h"
 #include "net/cert/multi_log_ct_verifier.h"
@@ -34,6 +33,7 @@
 #include "remoting/signaling/signaling_address.h"
 #include "remoting/signaling/xmpp_login_handler.h"
 #include "remoting/signaling/xmpp_stream_parser.h"
+#include "services/network/public/cpp/proxy_resolving_client_socket.h"
 #include "third_party/libjingle_xmpp/xmllite/xmlelement.h"
 
 // Use 50 seconds keep-alive interval, in case routers terminate
@@ -178,9 +178,11 @@ void XmppSignalStrategy::Core::Connect() {
   for (auto& observer : listeners_)
     observer.OnSignalStrategyStateChange(CONNECTING);
 
-  socket_.reset(new jingle_glue::ProxyResolvingClientSocket(
+  socket_ = std::make_unique<network::ProxyResolvingClientSocket>(
       socket_factory_, request_context_getter_, net::SSLConfig(),
-      net::HostPortPair(xmpp_server_config_.host, xmpp_server_config_.port)));
+      GURL("https://" +
+           net::HostPortPair(xmpp_server_config_.host, xmpp_server_config_.port)
+               .ToString()));
 
   int result = socket_->Connect(base::Bind(
       &Core::OnSocketConnected, base::Unretained(this)));

services/network/public/cpp/BUILD.gn

@@ -16,6 +16,8 @@ static_library("cpp") {
     "net_adapters.h",
     "network_param_ipc_traits.cc",
     "network_param_ipc_traits.h",
+    "proxy_resolving_client_socket.cc",
+    "proxy_resolving_client_socket.h",
     "url_loader_completion_status.cc",
     "url_loader_completion_status.h",
   ]
@@ -48,12 +50,15 @@ source_set("tests") {
     "mutable_network_traffic_annotation_tag_struct_traits_unittest.cc",
     "mutable_partial_network_traffic_annotation_tag_struct_traits_unittest.cc",
     "network_struct_traits_unittest.cc",
+    "proxy_resolving_client_socket_unittest.cc",
   ]
   deps = [
+    ":cpp",
     ":test_interfaces",
     "//base",
     "//mojo/public/cpp/bindings",
     "//net",
+    "//net:test_support",
     "//testing/gtest",
   ]
 }

jingle/glue/proxy_resolving_client_socket.h → services/network/public/cpp/proxy_resolving_client_socket.h

 // Copyright (c) 2012 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.
-//
-// This StreamSocket implementation wraps a ClientSocketHandle that is created
-// from the client socket pool after resolving proxies.
-
-#ifndef JINGLE_GLUE_PROXY_RESOLVING_CLIENT_SOCKET_H_
-#define JINGLE_GLUE_PROXY_RESOLVING_CLIENT_SOCKET_H_
+#ifndef SERVICES_NETWORK_PROXY_RESOLVING_CLIENT_SOCKET_H_
+#define SERVICES_NETWORK_PROXY_RESOLVING_CLIENT_SOCKET_H_
 
 #include <stdint.h>
 
 #include <memory>
 
 #include "base/compiler_specific.h"
+#include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
@@ -35,22 +32,32 @@ class HttpNetworkSession;
 class URLRequestContextGetter;
 }  // namespace net
 
-// TODO(sanjeevr): Move this to net/
-namespace jingle_glue {
+namespace network {
 
+// This class represents a net::StreamSocket implementation that does proxy
+// resolution for the provided url before establishing a connection. If there is
+// a proxy configured, a connection will be established to the proxy.
+//
+// TODO(xunjieli): https://crbug.com/721401. This class should be private (i.e.
+// moved out of services/network/public/cpp). The functionalities will be
+// exposed only through a mojo interface.
 class ProxyResolvingClientSocket : public net::StreamSocket {
  public:
-  // Constructs a new ProxyResolvingClientSocket. |socket_factory| is
-  // the ClientSocketFactory that will be used by the underlying
-  // HttpNetworkSession.  If |socket_factory| is NULL, the default
-  // socket factory (net::ClientSocketFactory::GetDefaultFactory())
-  // will be used.  |dest_host_port_pair| is the destination for this
-  // socket.  The hostname must be non-empty and the port must be > 0.
+  // Constructs a new ProxyResolvingClientSocket. |socket_factory| is the
+  // ClientSocketFactory that will be used by the underlying HttpNetworkSession.
+  // If |socket_factory| is nullptr, the default socket factory
+  // (net::ClientSocketFactory::GetDefaultFactory()) will be used. |url|'s host
+  // and port specify where a connection will be established to. The full URL
+  // will be only used for proxy resolution. Caller doesn't need to explicitly
+  // sanitize the url, any sensitive data (like embedded usernames and
+  // passwords), and local data (i.e. reference fragment) will be sanitized by
+  // net::ProxyService::ResolveProxyHelper() before the url is disclosed to the
+  // proxy.
   ProxyResolvingClientSocket(
       net::ClientSocketFactory* socket_factory,
       const scoped_refptr<net::URLRequestContextGetter>& request_context_getter,
       const net::SSLConfig& ssl_config,
-      const net::HostPortPair& dest_host_port_pair);
+      const GURL& url);
   ~ProxyResolvingClientSocket() override;
 
   // net::StreamSocket implementation.
@@ -85,18 +92,14 @@ class ProxyResolvingClientSocket : public net::StreamSocket {
   void ApplySocketTag(const net::SocketTag& tag) override;
 
  private:
-  // Proxy resolution and connection functions.
-  void ProcessProxyResolveDone(int status);
-  void ProcessConnectDone(int status);
+  FRIEND_TEST_ALL_PREFIXES(ProxyResolvingClientSocketTest, ConnectToProxy);
+  FRIEND_TEST_ALL_PREFIXES(ProxyResolvingClientSocketTest, ReadWriteErrors);
+
+  void ConnectToProxy(int net_error);
+  void ConnectToProxyDone(int net_error);
 
   void CloseTransportSocket();
-  void RunUserConnectCallback(int status);
   int ReconsiderProxyAfterError(int error);
-  void ReportSuccessfulProxyConnection();
-
-  // Callbacks passed to net APIs.
-  net::CompletionCallback proxy_resolve_callback_;
-  net::CompletionCallback connect_callback_;
 
   std::unique_ptr<net::HttpNetworkSession> network_session_;
 
@@ -106,8 +109,7 @@ class ProxyResolvingClientSocket : public net::StreamSocket {
   const net::SSLConfig ssl_config_;
   net::ProxyService::PacRequest* pac_request_;
   net::ProxyInfo proxy_info_;
-  const net::HostPortPair dest_host_port_pair_;
-  const GURL proxy_url_;
+  const GURL url_;
   bool tried_direct_connect_fallback_;
   net::NetLogWithSource net_log_;
 
@@ -119,6 +121,6 @@ class ProxyResolvingClientSocket : public net::StreamSocket {
   DISALLOW_COPY_AND_ASSIGN(ProxyResolvingClientSocket);
 };
 
-}  // namespace jingle_glue
+}  // namespace network
 
-#endif  // JINGLE_GLUE_PROXY_RESOLVING_CLIENT_SOCKET_H_
+#endif  // SERVICES_NETWORK_PROXY_RESOLVING_CLIENT_SOCKET_H_