diff --git a/net/BUILD.gn b/net/BUILD.gn index 15fc75fcdc16c8..ca693519072821 100644 --- a/net/BUILD.gn +++ b/net/BUILD.gn @@ -846,6 +846,8 @@ component("net") { "socket/client_socket_pool_manager_impl.h", "socket/connect_job.cc", "socket/connect_job.h", + "socket/connect_job_factory.cc", + "socket/connect_job_factory.h", "socket/connection_attempts.h", "socket/datagram_client_socket.h", "socket/datagram_server_socket.h", @@ -4308,6 +4310,7 @@ test("net_unittests") { "quic/test_quic_crypto_client_config_handle.h", "socket/client_socket_pool_base_unittest.cc", "socket/client_socket_pool_unittest.cc", + "socket/connect_job_factory_unittest.cc", "socket/connect_job_test_util.cc", "socket/connect_job_test_util.h", "socket/connect_job_unittest.cc", diff --git a/net/http/http_proxy_connect_job.cc b/net/http/http_proxy_connect_job.cc index 1cca4aed86c95e..7fe4413f54425e 100644 --- a/net/http/http_proxy_connect_job.cc +++ b/net/http/http_proxy_connect_job.cc @@ -4,6 +4,7 @@ #include "net/http/http_proxy_connect_job.h" +#include #include #include "base/bind.h" @@ -156,6 +157,18 @@ HttpProxySocketParams::HttpProxySocketParams( HttpProxySocketParams::~HttpProxySocketParams() = default; +std::unique_ptr HttpProxyConnectJob::Factory::Create( + RequestPriority priority, + const SocketTag& socket_tag, + const CommonConnectJobParams* common_connect_job_params, + scoped_refptr params, + ConnectJob::Delegate* delegate, + const NetLogWithSource* net_log) { + return std::make_unique( + priority, socket_tag, common_connect_job_params, std::move(params), + delegate, net_log); +} + HttpProxyConnectJob::HttpProxyConnectJob( RequestPriority priority, const SocketTag& socket_tag, diff --git a/net/http/http_proxy_connect_job.h b/net/http/http_proxy_connect_job.h index a676ea53b2bc3a..9281ce9236b716 100644 --- a/net/http/http_proxy_connect_job.h +++ b/net/http/http_proxy_connect_job.h @@ -88,6 +88,20 @@ class NET_EXPORT_PRIVATE HttpProxySocketParams class NET_EXPORT_PRIVATE HttpProxyConnectJob : public ConnectJob, public ConnectJob::Delegate { public: + class NET_EXPORT_PRIVATE Factory { + public: + Factory() = default; + virtual ~Factory() = default; + + virtual std::unique_ptr Create( + RequestPriority priority, + const SocketTag& socket_tag, + const CommonConnectJobParams* common_connect_job_params, + scoped_refptr params, + ConnectJob::Delegate* delegate, + const NetLogWithSource* net_log); + }; + HttpProxyConnectJob(RequestPriority priority, const SocketTag& socket_tag, const CommonConnectJobParams* common_connect_job_params, diff --git a/net/socket/client_socket_pool.cc b/net/socket/client_socket_pool.cc index e4115cef0195b3..f3ec46ff9b0f21 100644 --- a/net/socket/client_socket_pool.cc +++ b/net/socket/client_socket_pool.cc @@ -4,6 +4,7 @@ #include "net/socket/client_socket_pool.h" +#include #include #include "base/bind.h" @@ -11,11 +12,13 @@ #include "base/feature_list.h" #include "net/base/features.h" #include "net/base/host_port_pair.h" +#include "net/base/proxy_server.h" #include "net/dns/public/secure_dns_policy.h" #include "net/http/http_proxy_connect_job.h" #include "net/log/net_log_event_type.h" #include "net/log/net_log_with_source.h" #include "net/socket/connect_job.h" +#include "net/socket/connect_job_factory.h" #include "net/socket/socks_connect_job.h" #include "net/socket/ssl_connect_job.h" #include "net/socket/stream_socket.h" @@ -139,7 +142,13 @@ void ClientSocketPool::set_used_idle_socket_timeout(base::TimeDelta timeout) { g_used_idle_socket_timeout_s = timeout.InSeconds(); } -ClientSocketPool::ClientSocketPool() = default; +ClientSocketPool::ClientSocketPool( + bool is_for_websockets, + const CommonConnectJobParams* common_connect_job_params, + std::unique_ptr connect_job_factory) + : is_for_websockets_(is_for_websockets), + common_connect_job_params_(common_connect_job_params), + connect_job_factory_(std::move(connect_job_factory)) {} void ClientSocketPool::NetLogTcpClientSocketPoolRequestedSocket( const NetLogWithSource& net_log, @@ -162,8 +171,6 @@ std::unique_ptr ClientSocketPool::CreateConnectJob( scoped_refptr socket_params, const ProxyServer& proxy_server, const absl::optional& proxy_annotation_tag, - bool is_for_websockets, - const CommonConnectJobParams* common_connect_job_params, RequestPriority request_priority, SocketTag socket_tag, ConnectJob::Delegate* delegate) { @@ -174,34 +181,34 @@ std::unique_ptr ClientSocketPool::CreateConnectJob( OnHostResolutionCallback resolution_callback; if (using_ssl && proxy_server.is_direct()) { resolution_callback = base::BindRepeating( - &OnHostResolution, common_connect_job_params->spdy_session_pool, + &OnHostResolution, common_connect_job_params_->spdy_session_pool, // TODO(crbug.com/1206799): Pass along as SchemeHostPort. SpdySessionKey(HostPortPair::FromSchemeHostPort(group_id.destination()), proxy_server, group_id.privacy_mode(), SpdySessionKey::IsProxySession::kFalse, socket_tag, group_id.network_isolation_key(), group_id.secure_dns_policy()), - is_for_websockets); + is_for_websockets_); } else if (proxy_server.is_https()) { resolution_callback = base::BindRepeating( - &OnHostResolution, common_connect_job_params->spdy_session_pool, + &OnHostResolution, common_connect_job_params_->spdy_session_pool, SpdySessionKey(proxy_server.host_port_pair(), ProxyServer::Direct(), group_id.privacy_mode(), SpdySessionKey::IsProxySession::kTrue, socket_tag, group_id.network_isolation_key(), group_id.secure_dns_policy()), - is_for_websockets); + is_for_websockets_); } // TODO(crbug.com/1206799): Pass along as SchemeHostPort. - return ConnectJob::CreateConnectJob( + return connect_job_factory_->CreateConnectJob( using_ssl, HostPortPair::FromSchemeHostPort(group_id.destination()), proxy_server, proxy_annotation_tag, socket_params->ssl_config_for_origin(), - socket_params->ssl_config_for_proxy(), is_for_websockets, + socket_params->ssl_config_for_proxy(), is_for_websockets_, group_id.privacy_mode(), resolution_callback, request_priority, socket_tag, group_id.network_isolation_key(), - group_id.secure_dns_policy(), common_connect_job_params, delegate); + group_id.secure_dns_policy(), common_connect_job_params_, delegate); } } // namespace net diff --git a/net/socket/client_socket_pool.h b/net/socket/client_socket_pool.h index 9b86b5d5a9dec0..900051df32bbdf 100644 --- a/net/socket/client_socket_pool.h +++ b/net/socket/client_socket_pool.h @@ -36,7 +36,7 @@ class ProcessMemoryDump; namespace net { class ClientSocketHandle; -struct CommonConnectJobParams; +class ConnectJobFactory; class HttpAuthController; class HttpResponseInfo; class NetLogWithSource; @@ -341,7 +341,9 @@ class NET_EXPORT ClientSocketPool : public LowerLayeredPool { static void set_used_idle_socket_timeout(base::TimeDelta timeout); protected: - ClientSocketPool(); + ClientSocketPool(bool is_for_websockets, + const CommonConnectJobParams* common_connect_job_params, + std::unique_ptr connect_job_factory); void NetLogTcpClientSocketPoolRequestedSocket(const NetLogWithSource& net_log, const GroupId& group_id); @@ -349,18 +351,20 @@ class NET_EXPORT ClientSocketPool : public LowerLayeredPool { // Utility method to log a GroupId with a NetLog event. static base::Value NetLogGroupIdParams(const GroupId& group_id); - static std::unique_ptr CreateConnectJob( + std::unique_ptr CreateConnectJob( GroupId group_id, scoped_refptr socket_params, const ProxyServer& proxy_server, const absl::optional& proxy_annotation_tag, - bool is_for_websockets, - const CommonConnectJobParams* common_connect_job_params, RequestPriority request_priority, SocketTag socket_tag, ConnectJob::Delegate* delegate); private: + const bool is_for_websockets_; + const CommonConnectJobParams* const common_connect_job_params_; + const std::unique_ptr connect_job_factory_; + DISALLOW_COPY_AND_ASSIGN(ClientSocketPool); }; diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc index ce34f2f4644b39..a81712e312cdc2 100644 --- a/net/socket/client_socket_pool_base_unittest.cc +++ b/net/socket/client_socket_pool_base_unittest.cc @@ -48,6 +48,7 @@ #include "net/log/test_net_log_util.h" #include "net/socket/client_socket_factory.h" #include "net/socket/client_socket_handle.h" +#include "net/socket/connect_job_factory.h" #include "net/socket/datagram_client_socket.h" #include "net/socket/socket_performance_watcher.h" #include "net/socket/socket_tag.h" @@ -540,29 +541,10 @@ class TestConnectJob : public ConnectJob { DISALLOW_COPY_AND_ASSIGN(TestConnectJob); }; -class TestConnectJobFactory - : public TransportClientSocketPool::ConnectJobFactory { +class TestConnectJobFactory : public ConnectJobFactory { public: - TestConnectJobFactory(MockClientSocketFactory* client_socket_factory, - NetLog* net_log) - : common_connect_job_params_( - nullptr /* client_socket_factory */, - nullptr /* host_resolver */, - nullptr /* http_auth_cache */, - nullptr /* http_auth_handler_factory */, - nullptr /* spdy_session_pool */, - nullptr /* quic_supported_versions */, - nullptr /* quic_stream_factory */, - nullptr /* proxy_delegate */, - nullptr /* http_user_agent_settings */, - nullptr /* ssl_client_context */, - nullptr /* socket_performance_watcher_factory */, - nullptr /* network_quality_estimator */, - net_log, - nullptr /* websocket_endpoint_lock_manager */), - job_type_(TestConnectJob::kMockJob), - job_types_(nullptr), - client_socket_factory_(client_socket_factory) {} + explicit TestConnectJobFactory(MockClientSocketFactory* client_socket_factory) + : client_socket_factory_(client_socket_factory) {} ~TestConnectJobFactory() override = default; @@ -579,12 +561,21 @@ class TestConnectJobFactory // ConnectJobFactory implementation. - std::unique_ptr NewConnectJob( - ClientSocketPool::GroupId group_id, - scoped_refptr socket_params, + std::unique_ptr CreateConnectJob( + bool using_ssl, + const HostPortPair& endpoint, + const ProxyServer& proxy_server, const absl::optional& proxy_annotation_tag, + const SSLConfig* ssl_config_for_origin, + const SSLConfig* ssl_config_for_proxy, + bool force_tunnel, + PrivacyMode privacy_mode, + const OnHostResolutionCallback& resolution_callback, RequestPriority request_priority, SocketTag socket_tag, + const NetworkIsolationKey& network_isolation_key, + SecureDnsPolicy secure_dns_policy, + const CommonConnectJobParams* common_connect_job_params, ConnectJob::Delegate* delegate) const override { EXPECT_TRUE(!job_types_ || !job_types_->empty()); TestConnectJob::JobType job_type = job_type_; @@ -594,13 +585,12 @@ class TestConnectJobFactory } return std::make_unique( job_type, request_priority, socket_tag, timeout_duration_, - &common_connect_job_params_, delegate, client_socket_factory_); + common_connect_job_params, delegate, client_socket_factory_); } private: - const CommonConnectJobParams common_connect_job_params_; - TestConnectJob::JobType job_type_; - std::list* job_types_; + TestConnectJob::JobType job_type_ = TestConnectJob::kMockJob; + std::list* job_types_ = nullptr; base::TimeDelta timeout_duration_; MockClientSocketFactory* const client_socket_factory_; @@ -669,12 +659,12 @@ class ClientSocketPoolBaseTest : public TestWithTaskEnvironment { ProxyServer proxy_server = ProxyServer::Direct()) { DCHECK(!pool_.get()); std::unique_ptr connect_job_factory = - std::make_unique(&client_socket_factory_, - &net_log_); + std::make_unique(&client_socket_factory_); connect_job_factory_ = connect_job_factory.get(); pool_ = TransportClientSocketPool::CreateForTesting( max_sockets, max_sockets_per_group, unused_idle_socket_timeout, - used_idle_socket_timeout, proxy_server, std::move(connect_job_factory), + used_idle_socket_timeout, proxy_server, /*is_for_websockets=*/false, + &common_connect_job_params_, std::move(connect_job_factory), nullptr /* ssl_config_service */, enable_backup_connect_jobs); } @@ -729,6 +719,21 @@ class ClientSocketPoolBaseTest : public TestWithTaskEnvironment { size_t completion_count() const { return test_base_.completion_count(); } RecordingTestNetLog net_log_; + const CommonConnectJobParams common_connect_job_params_{ + nullptr /* client_socket_factory */, + nullptr /* host_resolver */, + nullptr /* http_auth_cache */, + nullptr /* http_auth_handler_factory */, + nullptr /* spdy_session_pool */, + nullptr /* quic_supported_versions */, + nullptr /* quic_stream_factory */, + nullptr /* proxy_delegate */, + nullptr /* http_user_agent_settings */, + nullptr /* ssl_client_context */, + nullptr /* socket_performance_watcher_factory */, + nullptr /* network_quality_estimator */, + &net_log_, + nullptr /* websocket_endpoint_lock_manager */}; bool connect_backup_jobs_enabled_; MockClientSocketFactory client_socket_factory_; TestConnectJobFactory* connect_job_factory_; diff --git a/net/socket/connect_job.cc b/net/socket/connect_job.cc index 156dd00a80c88e..873bb95824b51d 100644 --- a/net/socket/connect_job.cc +++ b/net/socket/connect_job.cc @@ -20,7 +20,6 @@ #include "net/socket/ssl_connect_job.h" #include "net/socket/stream_socket.h" #include "net/socket/transport_connect_job.h" -#include "net/ssl/ssl_config.h" #include "net/traffic_annotation/network_traffic_annotation.h" namespace net { @@ -96,97 +95,6 @@ ConnectJob::~ConnectJob() { net_log().EndEvent(NetLogEventType::CONNECT_JOB); } -std::unique_ptr ConnectJob::CreateConnectJob( - bool using_ssl, - const HostPortPair& endpoint, - const ProxyServer& proxy_server, - const absl::optional& proxy_annotation_tag, - const SSLConfig* ssl_config_for_origin, - const SSLConfig* ssl_config_for_proxy, - bool force_tunnel, - PrivacyMode privacy_mode, - const OnHostResolutionCallback& resolution_callback, - RequestPriority request_priority, - SocketTag socket_tag, - const NetworkIsolationKey& network_isolation_key, - SecureDnsPolicy secure_dns_policy, - const CommonConnectJobParams* common_connect_job_params, - ConnectJob::Delegate* delegate) { - scoped_refptr http_proxy_params; - scoped_refptr socks_params; - - if (!proxy_server.is_direct()) { - // No need to use a NetworkIsolationKey for looking up the proxy's IP - // address. Cached proxy IP addresses doesn't really expose useful - // information to destination sites, and not caching them has a performance - // cost. - auto proxy_tcp_params = base::MakeRefCounted( - proxy_server.host_port_pair(), NetworkIsolationKey(), secure_dns_policy, - resolution_callback); - - if (proxy_server.is_http_like()) { - scoped_refptr ssl_params; - if (proxy_server.is_secure_http_like()) { - DCHECK(ssl_config_for_proxy); - // Set ssl_params, and unset proxy_tcp_params - ssl_params = base::MakeRefCounted( - std::move(proxy_tcp_params), nullptr, nullptr, - proxy_server.host_port_pair(), *ssl_config_for_proxy, - PRIVACY_MODE_DISABLED, network_isolation_key); - proxy_tcp_params = nullptr; - } - - http_proxy_params = base::MakeRefCounted( - std::move(proxy_tcp_params), std::move(ssl_params), - proxy_server.is_quic(), endpoint, force_tunnel || using_ssl, - *proxy_annotation_tag, network_isolation_key); - } else { - DCHECK(proxy_server.is_socks()); - socks_params = base::MakeRefCounted( - std::move(proxy_tcp_params), - proxy_server.scheme() == ProxyServer::SCHEME_SOCKS5, endpoint, - network_isolation_key, *proxy_annotation_tag); - } - } - - // Deal with SSL - which layers on top of any given proxy. - if (using_ssl) { - DCHECK(ssl_config_for_origin); - scoped_refptr ssl_tcp_params; - if (proxy_server.is_direct()) { - ssl_tcp_params = base::MakeRefCounted( - endpoint, network_isolation_key, secure_dns_policy, - resolution_callback); - } - auto ssl_params = base::MakeRefCounted( - std::move(ssl_tcp_params), std::move(socks_params), - std::move(http_proxy_params), endpoint, *ssl_config_for_origin, - privacy_mode, network_isolation_key); - return std::make_unique( - request_priority, socket_tag, common_connect_job_params, - std::move(ssl_params), delegate, nullptr /* net_log */); - } - - if (proxy_server.is_http_like()) { - return std::make_unique( - request_priority, socket_tag, common_connect_job_params, - std::move(http_proxy_params), delegate, nullptr /* net_log */); - } - - if (proxy_server.is_socks()) { - return std::make_unique( - request_priority, socket_tag, common_connect_job_params, - std::move(socks_params), delegate, nullptr /* net_log */); - } - - DCHECK(proxy_server.is_direct()); - auto tcp_params = base::MakeRefCounted( - endpoint, network_isolation_key, secure_dns_policy, resolution_callback); - return TransportConnectJob::CreateTransportConnectJob( - std::move(tcp_params), request_priority, socket_tag, - common_connect_job_params, delegate, nullptr /* net_log */); -} - std::unique_ptr ConnectJob::PassSocket() { return std::move(socket_); } diff --git a/net/socket/connect_job.h b/net/socket/connect_job.h index ace3b8b7939671..a662e0e156a8d2 100644 --- a/net/socket/connect_job.h +++ b/net/socket/connect_job.h @@ -18,10 +18,8 @@ #include "net/base/load_states.h" #include "net/base/load_timing_info.h" #include "net/base/net_export.h" -#include "net/base/privacy_mode.h" #include "net/base/request_priority.h" #include "net/dns/public/resolve_error_info.h" -#include "net/dns/public/secure_dns_policy.h" #include "net/log/net_log_with_source.h" #include "net/socket/connection_attempts.h" #include "net/socket/socket_tag.h" @@ -41,13 +39,9 @@ class HttpResponseInfo; class HttpUserAgentSettings; class NetLog; class NetLogWithSource; -class NetworkIsolationKey; class NetworkQualityEstimator; -struct NetworkTrafficAnnotationTag; class ProxyDelegate; -class ProxyServer; class SocketPerformanceWatcherFactory; -struct SSLConfig; class StreamSocket; class WebSocketEndpointLockManager; class QuicStreamFactory; @@ -175,26 +169,6 @@ class NET_EXPORT_PRIVATE ConnectJob { NetLogEventType net_log_connect_event_type); virtual ~ConnectJob(); - // Creates a ConnectJob with the specified parameters. - // |common_connect_job_params| and |delegate| must outlive the returned - // ConnectJob. - static std::unique_ptr CreateConnectJob( - bool using_ssl, - const HostPortPair& endpoint, - const ProxyServer& proxy_server, - const absl::optional& proxy_annotation_tag, - const SSLConfig* ssl_config_for_origin, - const SSLConfig* ssl_config_for_proxy, - bool force_tunnel, - PrivacyMode privacy_mode, - const OnHostResolutionCallback& resolution_callback, - RequestPriority request_priority, - SocketTag socket_tag, - const NetworkIsolationKey& network_isolation_key, - SecureDnsPolicy secure_dns_policy, - const CommonConnectJobParams* common_connect_job_params, - ConnectJob::Delegate* delegate); - // Accessors const NetLogWithSource& net_log() { return net_log_; } RequestPriority priority() const { return priority_; } diff --git a/net/socket/connect_job_factory.cc b/net/socket/connect_job_factory.cc new file mode 100644 index 00000000000000..0e9ebf72bedffc --- /dev/null +++ b/net/socket/connect_job_factory.cc @@ -0,0 +1,160 @@ +// Copyright 2021 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. + +#include "net/socket/connect_job_factory.h" + +#include +#include + +#include "base/check.h" +#include "base/memory/scoped_refptr.h" +#include "net/base/host_port_pair.h" +#include "net/base/network_isolation_key.h" +#include "net/base/privacy_mode.h" +#include "net/base/proxy_server.h" +#include "net/base/request_priority.h" +#include "net/dns/public/secure_dns_policy.h" +#include "net/http/http_proxy_connect_job.h" +#include "net/socket/connect_job.h" +#include "net/socket/socket_tag.h" +#include "net/socket/socks_connect_job.h" +#include "net/socket/ssl_connect_job.h" +#include "net/socket/transport_connect_job.h" +#include "net/socket/websocket_transport_connect_job.h" +#include "net/ssl/ssl_config.h" +#include "net/traffic_annotation/network_traffic_annotation.h" +#include "third_party/abseil-cpp/absl/types/optional.h" + +namespace net { + +namespace { + +template +std::unique_ptr CreateFactoryIfNull(std::unique_ptr in) { + if (in) + return in; + return std::make_unique(); +} + +} // namespace + +ConnectJobFactory::ConnectJobFactory( + std::unique_ptr + http_proxy_connect_job_factory, + std::unique_ptr socks_connect_job_factory, + std::unique_ptr ssl_connect_job_factory, + std::unique_ptr transport_connect_job_factory, + std::unique_ptr + websocket_transport_connect_job_factory) + : http_proxy_connect_job_factory_( + CreateFactoryIfNull(std::move(http_proxy_connect_job_factory))), + socks_connect_job_factory_( + CreateFactoryIfNull(std::move(socks_connect_job_factory))), + ssl_connect_job_factory_( + CreateFactoryIfNull(std::move(ssl_connect_job_factory))), + transport_connect_job_factory_( + CreateFactoryIfNull(std::move(transport_connect_job_factory))), + websocket_transport_connect_job_factory_(CreateFactoryIfNull( + std::move(websocket_transport_connect_job_factory))) {} + +ConnectJobFactory::~ConnectJobFactory() = default; + +std::unique_ptr ConnectJobFactory::CreateConnectJob( + bool using_ssl, + const HostPortPair& endpoint, + const ProxyServer& proxy_server, + const absl::optional& proxy_annotation_tag, + const SSLConfig* ssl_config_for_origin, + const SSLConfig* ssl_config_for_proxy, + bool force_tunnel, + PrivacyMode privacy_mode, + const OnHostResolutionCallback& resolution_callback, + RequestPriority request_priority, + SocketTag socket_tag, + const NetworkIsolationKey& network_isolation_key, + SecureDnsPolicy secure_dns_policy, + const CommonConnectJobParams* common_connect_job_params, + ConnectJob::Delegate* delegate) const { + scoped_refptr http_proxy_params; + scoped_refptr socks_params; + + if (!proxy_server.is_direct()) { + // No need to use a NetworkIsolationKey for looking up the proxy's IP + // address. Cached proxy IP addresses doesn't really expose useful + // information to destination sites, and not caching them has a performance + // cost. + auto proxy_tcp_params = base::MakeRefCounted( + proxy_server.host_port_pair(), NetworkIsolationKey(), secure_dns_policy, + resolution_callback); + + if (proxy_server.is_http_like()) { + scoped_refptr ssl_params; + if (proxy_server.is_secure_http_like()) { + DCHECK(ssl_config_for_proxy); + // Set ssl_params, and unset proxy_tcp_params + ssl_params = base::MakeRefCounted( + std::move(proxy_tcp_params), nullptr, nullptr, + proxy_server.host_port_pair(), *ssl_config_for_proxy, + PRIVACY_MODE_DISABLED, network_isolation_key); + proxy_tcp_params = nullptr; + } + + http_proxy_params = base::MakeRefCounted( + std::move(proxy_tcp_params), std::move(ssl_params), + proxy_server.is_quic(), endpoint, force_tunnel || using_ssl, + *proxy_annotation_tag, network_isolation_key); + } else { + DCHECK(proxy_server.is_socks()); + socks_params = base::MakeRefCounted( + std::move(proxy_tcp_params), + proxy_server.scheme() == ProxyServer::SCHEME_SOCKS5, endpoint, + network_isolation_key, *proxy_annotation_tag); + } + } + + // Deal with SSL - which layers on top of any given proxy. + if (using_ssl) { + DCHECK(ssl_config_for_origin); + scoped_refptr ssl_tcp_params; + if (proxy_server.is_direct()) { + ssl_tcp_params = base::MakeRefCounted( + endpoint, network_isolation_key, secure_dns_policy, + resolution_callback); + } + auto ssl_params = base::MakeRefCounted( + std::move(ssl_tcp_params), std::move(socks_params), + std::move(http_proxy_params), endpoint, *ssl_config_for_origin, + privacy_mode, network_isolation_key); + return ssl_connect_job_factory_->Create( + request_priority, socket_tag, common_connect_job_params, + std::move(ssl_params), delegate, /*net_log=*/nullptr); + } + + if (proxy_server.is_http_like()) { + return http_proxy_connect_job_factory_->Create( + request_priority, socket_tag, common_connect_job_params, + std::move(http_proxy_params), delegate, /*net_log=*/nullptr); + } + + if (proxy_server.is_socks()) { + return socks_connect_job_factory_->Create( + request_priority, socket_tag, common_connect_job_params, + std::move(socks_params), delegate, /*net_log=*/nullptr); + } + + DCHECK(proxy_server.is_direct()); + auto tcp_params = base::MakeRefCounted( + endpoint, network_isolation_key, secure_dns_policy, resolution_callback); + if (!common_connect_job_params->websocket_endpoint_lock_manager) { + return transport_connect_job_factory_->Create( + request_priority, socket_tag, common_connect_job_params, tcp_params, + delegate, /*net_log=*/nullptr); + } + + return websocket_transport_connect_job_factory_->Create( + request_priority, socket_tag, common_connect_job_params, tcp_params, + delegate, /*net_log=*/nullptr); +} + +} // namespace net diff --git a/net/socket/connect_job_factory.h b/net/socket/connect_job_factory.h new file mode 100644 index 00000000000000..f102f2a95acaca --- /dev/null +++ b/net/socket/connect_job_factory.h @@ -0,0 +1,82 @@ +// Copyright 2021 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. + +#ifndef NET_SOCKET_CONNECT_JOB_FACTORY_H_ +#define NET_SOCKET_CONNECT_JOB_FACTORY_H_ + +#include + +#include "net/base/privacy_mode.h" +#include "net/base/request_priority.h" +#include "net/dns/public/secure_dns_policy.h" +#include "net/http/http_proxy_connect_job.h" +#include "net/socket/connect_job.h" +#include "net/socket/socket_tag.h" +#include "net/socket/socks_connect_job.h" +#include "net/socket/ssl_connect_job.h" +#include "net/socket/transport_connect_job.h" +#include "net/socket/websocket_transport_connect_job.h" +#include "third_party/abseil-cpp/absl/types/optional.h" + +namespace net { + +class HostPortPair; +class NetworkIsolationKey; +struct NetworkTrafficAnnotationTag; +class ProxyServer; +struct SSLConfig; + +// Common factory for all ConnectJob types. Determines and creates the correct +// ConnectJob depending on the passed in parameters. +class NET_EXPORT_PRIVATE ConnectJobFactory { + public: + // Default factory will be used if passed the default `nullptr`. + explicit ConnectJobFactory( + std::unique_ptr + http_proxy_connect_job_factory = nullptr, + std::unique_ptr socks_connect_job_factory = + nullptr, + std::unique_ptr ssl_connect_job_factory = nullptr, + std::unique_ptr + transport_connect_job_factory = nullptr, + std::unique_ptr + websocket_transport_connect_job_factory = nullptr); + + // Not copyable/movable. Intended for polymorphic use via pointer. + ConnectJobFactory(const ConnectJobFactory&) = delete; + ConnectJobFactory& operator=(const ConnectJobFactory&) = delete; + + virtual ~ConnectJobFactory(); + + // `common_connect_job_params` and `delegate` must outlive the returned + // ConnectJob. + virtual std::unique_ptr CreateConnectJob( + bool using_ssl, + const HostPortPair& endpoint, + const ProxyServer& proxy_server, + const absl::optional& proxy_annotation_tag, + const SSLConfig* ssl_config_for_origin, + const SSLConfig* ssl_config_for_proxy, + bool force_tunnel, + PrivacyMode privacy_mode, + const OnHostResolutionCallback& resolution_callback, + RequestPriority request_priority, + SocketTag socket_tag, + const NetworkIsolationKey& network_isolation_key, + SecureDnsPolicy secure_dns_policy, + const CommonConnectJobParams* common_connect_job_params, + ConnectJob::Delegate* delegate) const; + + private: + std::unique_ptr http_proxy_connect_job_factory_; + std::unique_ptr socks_connect_job_factory_; + std::unique_ptr ssl_connect_job_factory_; + std::unique_ptr transport_connect_job_factory_; + std::unique_ptr + websocket_transport_connect_job_factory_; +}; + +} // namespace net + +#endif // NET_SOCKET_CONNECT_JOB_FACTORY_H_ diff --git a/net/socket/connect_job_factory_unittest.cc b/net/socket/connect_job_factory_unittest.cc new file mode 100644 index 00000000000000..cfcc6a5b3881c5 --- /dev/null +++ b/net/socket/connect_job_factory_unittest.cc @@ -0,0 +1,411 @@ +// Copyright 2021 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. + +#include "net/socket/connect_job_factory.h" + +#include +#include + +#include "base/memory/scoped_refptr.h" +#include "net/base/host_port_pair.h" +#include "net/base/network_isolation_key.h" +#include "net/base/privacy_mode.h" +#include "net/base/proxy_server.h" +#include "net/base/request_priority.h" +#include "net/dns/public/secure_dns_policy.h" +#include "net/http/http_proxy_connect_job.h" +#include "net/log/net_log_with_source.h" +#include "net/socket/connect_job.h" +#include "net/socket/connect_job_test_util.h" +#include "net/socket/socket_tag.h" +#include "net/socket/socks_connect_job.h" +#include "net/socket/ssl_connect_job.h" +#include "net/socket/transport_connect_job.h" +#include "net/socket/websocket_endpoint_lock_manager.h" +#include "net/socket/websocket_transport_connect_job.h" +#include "net/ssl/ssl_config.h" +#include "net/test/test_with_task_environment.h" +#include "net/traffic_annotation/network_traffic_annotation_test_helper.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/abseil-cpp/absl/types/optional.h" + +namespace net { +namespace { + +// Mock HttpProxyConnectJob::Factory that records the `params` used and then +// passes on to a real factory. +class TestHttpProxyConnectJobFactory : public HttpProxyConnectJob::Factory { + public: + std::unique_ptr Create( + RequestPriority priority, + const SocketTag& socket_tag, + const CommonConnectJobParams* common_connect_job_params, + scoped_refptr params, + ConnectJob::Delegate* delegate, + const NetLogWithSource* net_log) override { + params_.push_back(params); + return HttpProxyConnectJob::Factory::Create(priority, socket_tag, + common_connect_job_params, + params, delegate, net_log); + } + + const std::vector>& params() const { + return params_; + } + + private: + std::vector> params_; +}; + +// Mock SOCKSConnectJob::Factory that records the `params` used and then passes +// on to a real factory. +class TestSocksConnectJobFactory : public SOCKSConnectJob::Factory { + public: + std::unique_ptr Create( + RequestPriority priority, + const SocketTag& socket_tag, + const CommonConnectJobParams* common_connect_job_params, + scoped_refptr socks_params, + ConnectJob::Delegate* delegate, + const NetLogWithSource* net_log) override { + params_.push_back(socks_params); + return SOCKSConnectJob::Factory::Create(priority, socket_tag, + common_connect_job_params, + socks_params, delegate, net_log); + } + + const std::vector>& params() const { + return params_; + } + + private: + std::vector> params_; +}; + +// Mock SSLConnectJob::Factory that records the `params` used and then passes on +// to a real factory. +class TestSslConnectJobFactory : public SSLConnectJob::Factory { + public: + std::unique_ptr Create( + RequestPriority priority, + const SocketTag& socket_tag, + const CommonConnectJobParams* common_connect_job_params, + scoped_refptr params, + ConnectJob::Delegate* delegate, + const NetLogWithSource* net_log) override { + params_.push_back(params); + return SSLConnectJob::Factory::Create(priority, socket_tag, + common_connect_job_params, params, + delegate, net_log); + } + + const std::vector>& params() const { + return params_; + } + + private: + std::vector> params_; +}; + +// Mock TransportConnectJob::Factory that records the `params` used and then +// passes on to a real factory. +class TestTransportConnectJobFactory : public TransportConnectJob::Factory { + public: + std::unique_ptr Create( + RequestPriority priority, + const SocketTag& socket_tag, + const CommonConnectJobParams* common_connect_job_params, + const scoped_refptr& params, + ConnectJob::Delegate* delegate, + const NetLogWithSource* net_log) override { + params_.push_back(params); + return TransportConnectJob::Factory::Create(priority, socket_tag, + common_connect_job_params, + params, delegate, net_log); + } + + const std::vector>& params() const { + return params_; + } + + private: + std::vector> params_; +}; + +// Mock WebSocketTransportConnectJob::Factory that records the `params` used and +// then passes on to a real factory. +class TestWebsocketConnectJobFactory + : public WebSocketTransportConnectJob::Factory { + public: + std::unique_ptr Create( + RequestPriority priority, + const SocketTag& socket_tag, + const CommonConnectJobParams* common_connect_job_params, + const scoped_refptr& params, + ConnectJob::Delegate* delegate, + const NetLogWithSource* net_log) override { + params_.push_back(params); + return WebSocketTransportConnectJob::Factory::Create( + priority, socket_tag, common_connect_job_params, params, delegate, + net_log); + } + + const std::vector>& params() const { + return params_; + } + + private: + std::vector> params_; +}; + +class ConnectJobFactoryTest : public TestWithTaskEnvironment { + public: + ConnectJobFactoryTest() { + auto http_proxy_job_factory = + std::make_unique(); + http_proxy_job_factory_ = http_proxy_job_factory.get(); + + auto socks_job_factory = std::make_unique(); + socks_job_factory_ = socks_job_factory.get(); + + auto ssl_job_factory = std::make_unique(); + ssl_job_factory_ = ssl_job_factory.get(); + + auto transport_job_factory = + std::make_unique(); + transport_job_factory_ = transport_job_factory.get(); + + auto websocket_job_factory = + std::make_unique(); + websocket_job_factory_ = websocket_job_factory.get(); + + factory_ = std::make_unique( + std::move(http_proxy_job_factory), std::move(socks_job_factory), + std::move(ssl_job_factory), std::move(transport_job_factory), + std::move(websocket_job_factory)); + } + + protected: + // Gets the total number of ConnectJob creations across all types. + size_t GetCreationCount() const { + return http_proxy_job_factory_->params().size() + + socks_job_factory_->params().size() + + ssl_job_factory_->params().size() + + transport_job_factory_->params().size() + + websocket_job_factory_->params().size(); + } + + const CommonConnectJobParams common_connect_job_params_{ + /*client_socket_factory=*/nullptr, + /*host_resolver=*/nullptr, + /*http_auth_cache=*/nullptr, + /*http_auth_handler_factory=*/nullptr, + /*spdy_session_pool=*/nullptr, + /*quic_supported_versions=*/nullptr, + /*quic_stream_factory=*/nullptr, + /*proxy_delegate=*/nullptr, + /*http_user_agent_settings=*/nullptr, + /*ssl_client_context=*/nullptr, + /*socket_performance_watcher_factory=*/nullptr, + /*network_quality_estimator=*/nullptr, + /*net_log=*/nullptr, + /*websocket_endpoint_lock_manager=*/nullptr}; + TestConnectJobDelegate delegate_; + + TestHttpProxyConnectJobFactory* http_proxy_job_factory_; + TestSocksConnectJobFactory* socks_job_factory_; + TestSslConnectJobFactory* ssl_job_factory_; + TestTransportConnectJobFactory* transport_job_factory_; + TestWebsocketConnectJobFactory* websocket_job_factory_; + + std::unique_ptr factory_; +}; + +TEST_F(ConnectJobFactoryTest, CreateConnectJobWithoutScheme) { + const HostPortPair kEndpoint("test", 82); + + std::unique_ptr job = factory_->CreateConnectJob( + /*using_ssl=*/false, kEndpoint, ProxyServer::Direct(), + /*proxy_annotation_tag=*/absl::nullopt, + /*ssl_config_for_origin=*/nullptr, + /*ssl_config_for_proxy=*/nullptr, + /*force_tunnel=*/false, PrivacyMode::PRIVACY_MODE_DISABLED, + OnHostResolutionCallback(), DEFAULT_PRIORITY, SocketTag(), + NetworkIsolationKey(), SecureDnsPolicy::kAllow, + &common_connect_job_params_, &delegate_); + EXPECT_EQ(GetCreationCount(), 1u); + + ASSERT_THAT(transport_job_factory_->params(), testing::SizeIs(1)); + const TransportSocketParams& params = + *transport_job_factory_->params().front(); + EXPECT_EQ(params.destination(), kEndpoint); +} + +TEST_F(ConnectJobFactoryTest, CreateHttpsConnectJobWithoutScheme) { + const HostPortPair kEndpoint("test", 84); + SSLConfig ssl_config; + + std::unique_ptr job = factory_->CreateConnectJob( + /*using_ssl=*/true, kEndpoint, ProxyServer::Direct(), + /*proxy_annotation_tag=*/absl::nullopt, + /*ssl_config_for_origin=*/&ssl_config, + /*ssl_config_for_proxy=*/nullptr, + /*force_tunnel=*/false, PrivacyMode::PRIVACY_MODE_DISABLED, + OnHostResolutionCallback(), DEFAULT_PRIORITY, SocketTag(), + NetworkIsolationKey(), SecureDnsPolicy::kAllow, + &common_connect_job_params_, &delegate_); + EXPECT_EQ(GetCreationCount(), 1u); + + ASSERT_THAT(ssl_job_factory_->params(), testing::SizeIs(1)); + const SSLSocketParams& params = *ssl_job_factory_->params().front(); + EXPECT_EQ(params.host_and_port(), kEndpoint); + + ASSERT_EQ(params.GetConnectionType(), SSLSocketParams::DIRECT); + const TransportSocketParams& transport_params = + *params.GetDirectConnectionParams(); + EXPECT_EQ(transport_params.destination(), kEndpoint); +} + +TEST_F(ConnectJobFactoryTest, CreateHttpProxyConnectJob) { + const HostPortPair kEndpoint("test", 85); + const ProxyServer kProxy(ProxyServer::SCHEME_HTTP, + HostPortPair("proxy.test", 86)); + + std::unique_ptr job = factory_->CreateConnectJob( + /*using_ssl=*/false, kEndpoint, kProxy, TRAFFIC_ANNOTATION_FOR_TESTS, + /*ssl_config_for_origin=*/nullptr, + /*ssl_config_for_proxy=*/nullptr, + /*force_tunnel=*/false, PrivacyMode::PRIVACY_MODE_DISABLED, + OnHostResolutionCallback(), DEFAULT_PRIORITY, SocketTag(), + NetworkIsolationKey(), SecureDnsPolicy::kAllow, + &common_connect_job_params_, &delegate_); + EXPECT_EQ(GetCreationCount(), 1u); + + ASSERT_THAT(http_proxy_job_factory_->params(), testing::SizeIs(1)); + const HttpProxySocketParams& params = + *http_proxy_job_factory_->params().front(); + EXPECT_FALSE(params.is_quic()); + EXPECT_EQ(params.endpoint(), kEndpoint); + + ASSERT_TRUE(params.transport_params()); + const TransportSocketParams& transport_params = *params.transport_params(); + EXPECT_EQ(transport_params.destination(), kProxy.host_port_pair()); +} + +TEST_F(ConnectJobFactoryTest, CreateHttpProxyConnectJobForHttps) { + const HostPortPair kEndpoint("test", 87); + const ProxyServer kProxy(ProxyServer::SCHEME_HTTP, + HostPortPair("proxy.test", 88)); + SSLConfig ssl_config; + + std::unique_ptr job = factory_->CreateConnectJob( + /*using_ssl=*/true, kEndpoint, kProxy, TRAFFIC_ANNOTATION_FOR_TESTS, + /*ssl_config_for_origin=*/&ssl_config, + /*ssl_config_for_proxy=*/nullptr, + /*force_tunnel=*/false, PrivacyMode::PRIVACY_MODE_DISABLED, + OnHostResolutionCallback(), DEFAULT_PRIORITY, SocketTag(), + NetworkIsolationKey(), SecureDnsPolicy::kAllow, + &common_connect_job_params_, &delegate_); + EXPECT_EQ(GetCreationCount(), 1u); + + ASSERT_THAT(ssl_job_factory_->params(), testing::SizeIs(1)); + const SSLSocketParams& params = *ssl_job_factory_->params().front(); + EXPECT_EQ(params.host_and_port(), kEndpoint); + + ASSERT_EQ(params.GetConnectionType(), SSLSocketParams::HTTP_PROXY); + const HttpProxySocketParams& proxy_params = + *params.GetHttpProxyConnectionParams(); + EXPECT_FALSE(proxy_params.is_quic()); + EXPECT_EQ(proxy_params.endpoint(), kEndpoint); + + ASSERT_TRUE(proxy_params.transport_params()); + const TransportSocketParams& transport_params = + *proxy_params.transport_params(); + EXPECT_EQ(transport_params.destination(), kProxy.host_port_pair()); +} + +TEST_F(ConnectJobFactoryTest, CreateHttpsProxyConnectJob) { + const HostPortPair kEndpoint("test", 89); + const ProxyServer kProxy(ProxyServer::SCHEME_HTTPS, + HostPortPair("proxy.test", 90)); + SSLConfig ssl_config; + + std::unique_ptr job = factory_->CreateConnectJob( + /*using_ssl=*/false, kEndpoint, kProxy, TRAFFIC_ANNOTATION_FOR_TESTS, + /*ssl_config_for_origin=*/nullptr, + /*ssl_config_for_proxy=*/&ssl_config, + /*force_tunnel=*/false, PrivacyMode::PRIVACY_MODE_DISABLED, + OnHostResolutionCallback(), DEFAULT_PRIORITY, SocketTag(), + NetworkIsolationKey(), SecureDnsPolicy::kAllow, + &common_connect_job_params_, &delegate_); + EXPECT_EQ(GetCreationCount(), 1u); + + ASSERT_THAT(http_proxy_job_factory_->params(), testing::SizeIs(1)); + const HttpProxySocketParams& params = + *http_proxy_job_factory_->params().front(); + EXPECT_FALSE(params.is_quic()); + EXPECT_EQ(params.endpoint(), kEndpoint); + + ASSERT_TRUE(params.ssl_params()); + const SSLSocketParams& ssl_params = *params.ssl_params(); + EXPECT_EQ(ssl_params.host_and_port(), kProxy.host_port_pair()); + + ASSERT_EQ(ssl_params.GetConnectionType(), SSLSocketParams::DIRECT); + const TransportSocketParams& transport_params = + *ssl_params.GetDirectConnectionParams(); + EXPECT_EQ(transport_params.destination(), kProxy.host_port_pair()); +} + +TEST_F(ConnectJobFactoryTest, CreateSocksProxyConnectJob) { + const HostPortPair kEndpoint("test", 91); + const ProxyServer kProxy(ProxyServer::SCHEME_SOCKS5, + HostPortPair("proxy.test", 92)); + + std::unique_ptr job = factory_->CreateConnectJob( + /*using_ssl=*/false, kEndpoint, kProxy, TRAFFIC_ANNOTATION_FOR_TESTS, + /*ssl_config_for_origin=*/nullptr, + /*ssl_config_for_proxy=*/nullptr, + /*force_tunnel=*/false, PrivacyMode::PRIVACY_MODE_DISABLED, + OnHostResolutionCallback(), DEFAULT_PRIORITY, SocketTag(), + NetworkIsolationKey(), SecureDnsPolicy::kAllow, + &common_connect_job_params_, &delegate_); + EXPECT_EQ(GetCreationCount(), 1u); + + ASSERT_THAT(socks_job_factory_->params(), testing::SizeIs(1)); + const SOCKSSocketParams& params = *socks_job_factory_->params().front(); + EXPECT_EQ(params.destination(), kEndpoint); + EXPECT_TRUE(params.is_socks_v5()); + + const TransportSocketParams& transport_params = *params.transport_params(); + EXPECT_EQ(transport_params.destination(), kProxy.host_port_pair()); +} + +TEST_F(ConnectJobFactoryTest, CreateWebsocketConnectJob) { + const HostPortPair kEndpoint("test", 93); + + WebSocketEndpointLockManager websocket_endpoint_lock_manager; + CommonConnectJobParams common_connect_job_params = common_connect_job_params_; + common_connect_job_params.websocket_endpoint_lock_manager = + &websocket_endpoint_lock_manager; + + std::unique_ptr job = factory_->CreateConnectJob( + /*using_ssl=*/false, kEndpoint, ProxyServer::Direct(), + /*proxy_annotation_tag=*/absl::nullopt, + /*ssl_config_for_origin=*/nullptr, + /*ssl_config_for_proxy=*/nullptr, + /*force_tunnel=*/false, PrivacyMode::PRIVACY_MODE_DISABLED, + OnHostResolutionCallback(), DEFAULT_PRIORITY, SocketTag(), + NetworkIsolationKey(), SecureDnsPolicy::kAllow, + &common_connect_job_params, &delegate_); + EXPECT_EQ(GetCreationCount(), 1u); + + ASSERT_THAT(websocket_job_factory_->params(), testing::SizeIs(1)); + const TransportSocketParams& params = + *websocket_job_factory_->params().front(); + EXPECT_EQ(params.destination(), kEndpoint); +} + +} // namespace +} // namespace net diff --git a/net/socket/socks_connect_job.cc b/net/socket/socks_connect_job.cc index 97ee6f2f5cf196..c5824014384eaf 100644 --- a/net/socket/socks_connect_job.cc +++ b/net/socket/socks_connect_job.cc @@ -37,6 +37,18 @@ SOCKSSocketParams::SOCKSSocketParams( SOCKSSocketParams::~SOCKSSocketParams() = default; +std::unique_ptr SOCKSConnectJob::Factory::Create( + RequestPriority priority, + const SocketTag& socket_tag, + const CommonConnectJobParams* common_connect_job_params, + scoped_refptr socks_params, + ConnectJob::Delegate* delegate, + const NetLogWithSource* net_log) { + return std::make_unique( + priority, socket_tag, common_connect_job_params, std::move(socks_params), + delegate, net_log); +} + SOCKSConnectJob::SOCKSConnectJob( RequestPriority priority, const SocketTag& socket_tag, diff --git a/net/socket/socks_connect_job.h b/net/socket/socks_connect_job.h index 21db95cf3727b4..960332816468d6 100644 --- a/net/socket/socks_connect_job.h +++ b/net/socket/socks_connect_job.h @@ -69,6 +69,20 @@ class NET_EXPORT_PRIVATE SOCKSSocketParams class NET_EXPORT_PRIVATE SOCKSConnectJob : public ConnectJob, public ConnectJob::Delegate { public: + class NET_EXPORT_PRIVATE Factory { + public: + Factory() = default; + virtual ~Factory() = default; + + virtual std::unique_ptr Create( + RequestPriority priority, + const SocketTag& socket_tag, + const CommonConnectJobParams* common_connect_job_params, + scoped_refptr socks_params, + ConnectJob::Delegate* delegate, + const NetLogWithSource* net_log); + }; + SOCKSConnectJob(RequestPriority priority, const SocketTag& socket_tag, const CommonConnectJobParams* common_connect_job_params, diff --git a/net/socket/ssl_connect_job.cc b/net/socket/ssl_connect_job.cc index 8d67c8cd2f6b39..18791fc03a07e4 100644 --- a/net/socket/ssl_connect_job.cc +++ b/net/socket/ssl_connect_job.cc @@ -5,6 +5,7 @@ #include "net/socket/ssl_connect_job.h" #include +#include #include #include "base/bind.h" @@ -102,6 +103,18 @@ SSLSocketParams::GetHttpProxyConnectionParams() const { return http_proxy_params_; } +std::unique_ptr SSLConnectJob::Factory::Create( + RequestPriority priority, + const SocketTag& socket_tag, + const CommonConnectJobParams* common_connect_job_params, + scoped_refptr params, + ConnectJob::Delegate* delegate, + const NetLogWithSource* net_log) { + return std::make_unique(priority, socket_tag, + common_connect_job_params, + std::move(params), delegate, net_log); +} + SSLConnectJob::SSLConnectJob( RequestPriority priority, const SocketTag& socket_tag, diff --git a/net/socket/ssl_connect_job.h b/net/socket/ssl_connect_job.h index f5ea9b9a4b8ac1..40cd4d8560c0b4 100644 --- a/net/socket/ssl_connect_job.h +++ b/net/socket/ssl_connect_job.h @@ -87,6 +87,20 @@ class NET_EXPORT_PRIVATE SSLSocketParams class NET_EXPORT_PRIVATE SSLConnectJob : public ConnectJob, public ConnectJob::Delegate { public: + class NET_EXPORT_PRIVATE Factory { + public: + Factory() = default; + virtual ~Factory() = default; + + virtual std::unique_ptr Create( + RequestPriority priority, + const SocketTag& socket_tag, + const CommonConnectJobParams* common_connect_job_params, + scoped_refptr params, + ConnectJob::Delegate* delegate, + const NetLogWithSource* net_log); + }; + // Note: the SSLConnectJob does not own |messenger| so it must outlive the // job. SSLConnectJob(RequestPriority priority, diff --git a/net/socket/transport_client_socket_pool.cc b/net/socket/transport_client_socket_pool.cc index d428f15aa489af..9323161cac2fa2 100644 --- a/net/socket/transport_client_socket_pool.cc +++ b/net/socket/transport_client_socket_pool.cc @@ -30,6 +30,7 @@ #include "net/log/net_log.h" #include "net/log/net_log_event_type.h" #include "net/log/net_log_source.h" +#include "net/socket/connect_job_factory.h" #include "net/traffic_annotation/network_traffic_annotation.h" #include "url/gurl.h" @@ -72,47 +73,6 @@ const char TransportClientSocketPool::kSocketPoolDestroyed[] = const char TransportClientSocketPool::kSslConfigChanged[] = "SSL configuration changed"; -// ConnectJobFactory implementation that creates the standard ConnectJob -// classes, using SocketParams. -class TransportClientSocketPool::ConnectJobFactoryImpl - : public TransportClientSocketPool::ConnectJobFactory { - public: - ConnectJobFactoryImpl(const ProxyServer& proxy_server, - bool is_for_websockets, - const CommonConnectJobParams* common_connect_job_params) - : proxy_server_(proxy_server), - is_for_websockets_(is_for_websockets), - common_connect_job_params_(common_connect_job_params) { - // This class should not be used with WebSockets. Note that - // |common_connect_job_params| may be nullptr in tests. - DCHECK(!common_connect_job_params || - !common_connect_job_params->websocket_endpoint_lock_manager); - } - - ~ConnectJobFactoryImpl() override = default; - - // TransportClientSocketPool::ConnectJobFactory methods. - std::unique_ptr NewConnectJob( - ClientSocketPool::GroupId group_id, - scoped_refptr socket_params, - const absl::optional& proxy_annotation_tag, - RequestPriority request_priority, - SocketTag socket_tag, - ConnectJob::Delegate* delegate) const override { - return CreateConnectJob(group_id, socket_params, proxy_server_, - proxy_annotation_tag, is_for_websockets_, - common_connect_job_params_, request_priority, - socket_tag, delegate); - } - - private: - const ProxyServer proxy_server_; - const bool is_for_websockets_; - const CommonConnectJobParams* common_connect_job_params_; - - DISALLOW_COPY_AND_ASSIGN(ConnectJobFactoryImpl); -}; - TransportClientSocketPool::Request::Request( ClientSocketHandle* handle, CompletionOnceCallback callback, @@ -163,17 +123,16 @@ TransportClientSocketPool::TransportClientSocketPool( const ProxyServer& proxy_server, bool is_for_websockets, const CommonConnectJobParams* common_connect_job_params) - : TransportClientSocketPool( - max_sockets, - max_sockets_per_group, - unused_idle_socket_timeout, - ClientSocketPool::used_idle_socket_timeout(), - proxy_server, - std::make_unique(proxy_server, - is_for_websockets, - common_connect_job_params), - common_connect_job_params->ssl_client_context, - true /* connect_backup_jobs_enabled */) {} + : TransportClientSocketPool(max_sockets, + max_sockets_per_group, + unused_idle_socket_timeout, + ClientSocketPool::used_idle_socket_timeout(), + proxy_server, + is_for_websockets, + common_connect_job_params, + std::make_unique(), + common_connect_job_params->ssl_client_context, + true /* connect_backup_jobs_enabled */) {} TransportClientSocketPool::~TransportClientSocketPool() { // Clean up any idle sockets and pending connect jobs. Assert that we have no @@ -199,15 +158,17 @@ TransportClientSocketPool::CreateForTesting( base::TimeDelta unused_idle_socket_timeout, base::TimeDelta used_idle_socket_timeout, const ProxyServer& proxy_server, + bool is_for_websockets, + const CommonConnectJobParams* common_connect_job_params, std::unique_ptr connect_job_factory, SSLClientContext* ssl_client_context, bool connect_backup_jobs_enabled) { return base::WrapUnique( new TransportClientSocketPool( max_sockets, max_sockets_per_group, unused_idle_socket_timeout, - used_idle_socket_timeout, proxy_server, - std::move(connect_job_factory), ssl_client_context, - connect_backup_jobs_enabled)); + used_idle_socket_timeout, proxy_server, is_for_websockets, + common_connect_job_params, std::move(connect_job_factory), + ssl_client_context, connect_backup_jobs_enabled)); } TransportClientSocketPool::CallbackResultPair::CallbackResultPair() @@ -436,9 +397,9 @@ int TransportClientSocketPool::RequestSocketInternal(const GroupId& group_id, group = GetOrCreateGroup(group_id); connecting_socket_count_++; std::unique_ptr owned_connect_job( - connect_job_factory_->NewConnectJob( - group_id, request.socket_params(), request.proxy_annotation_tag(), - request.priority(), request.socket_tag(), group)); + CreateConnectJob(group_id, request.socket_params(), proxy_server_, + request.proxy_annotation_tag(), request.priority(), + request.socket_tag(), group)); owned_connect_job->net_log().AddEvent( NetLogEventType::SOCKET_POOL_CONNECT_JOB_CREATED, [&] { return NetLogCreateConnectJobParams(false /* backup_job */, &group_id); @@ -832,10 +793,15 @@ TransportClientSocketPool::TransportClientSocketPool( base::TimeDelta unused_idle_socket_timeout, base::TimeDelta used_idle_socket_timeout, const ProxyServer& proxy_server, + bool is_for_websockets, + const CommonConnectJobParams* common_connect_job_params, std::unique_ptr connect_job_factory, SSLClientContext* ssl_client_context, bool connect_backup_jobs_enabled) - : idle_socket_count_(0), + : ClientSocketPool(is_for_websockets, + common_connect_job_params, + std::move(connect_job_factory)), + idle_socket_count_(0), connecting_socket_count_(0), handed_out_socket_count_(0), max_sockets_(max_sockets), @@ -843,7 +809,6 @@ TransportClientSocketPool::TransportClientSocketPool( unused_idle_socket_timeout_(unused_idle_socket_timeout), used_idle_socket_timeout_(used_idle_socket_timeout), proxy_server_(proxy_server), - connect_job_factory_(std::move(connect_job_factory)), connect_backup_jobs_enabled_(connect_backup_jobs_enabled && g_connect_backup_jobs_enabled), ssl_client_context_(ssl_client_context) { @@ -1639,8 +1604,9 @@ void TransportClientSocketPool::Group::OnBackupJobTimerFired( Request* request = unbound_requests_.FirstMax().value().get(); std::unique_ptr owned_backup_job = - client_socket_pool_->connect_job_factory_->NewConnectJob( - group_id, request->socket_params(), request->proxy_annotation_tag(), + client_socket_pool_->CreateConnectJob( + group_id, request->socket_params(), + client_socket_pool_->proxy_server_, request->proxy_annotation_tag(), request->priority(), request->socket_tag(), this); owned_backup_job->net_log().AddEvent( NetLogEventType::SOCKET_POOL_CONNECT_JOB_CREATED, [&] { diff --git a/net/socket/transport_client_socket_pool.h b/net/socket/transport_client_socket_pool.h index b711242e380fd4..80f432de5ffb0a 100644 --- a/net/socket/transport_client_socket_pool.h +++ b/net/socket/transport_client_socket_pool.h @@ -50,6 +50,7 @@ class ProcessMemoryDump; namespace net { struct CommonConnectJobParams; +class ConnectJobFactory; struct NetLogSource; struct NetworkTrafficAnnotationTag; @@ -150,23 +151,6 @@ class NET_EXPORT_PRIVATE TransportClientSocketPool DISALLOW_COPY_AND_ASSIGN(Request); }; - class ConnectJobFactory { - public: - ConnectJobFactory() {} - virtual ~ConnectJobFactory() {} - - virtual std::unique_ptr NewConnectJob( - ClientSocketPool::GroupId group_id, - scoped_refptr socket_params, - const absl::optional& proxy_annotation_tag, - RequestPriority request_priority, - SocketTag socket_tag, - ConnectJob::Delegate* delegate) const = 0; - - private: - DISALLOW_COPY_AND_ASSIGN(ConnectJobFactory); - }; - TransportClientSocketPool( int max_sockets, int max_sockets_per_group, @@ -186,6 +170,8 @@ class NET_EXPORT_PRIVATE TransportClientSocketPool base::TimeDelta unused_idle_socket_timeout, base::TimeDelta used_idle_socket_timeout, const ProxyServer& proxy_server, + bool is_for_websockets, + const CommonConnectJobParams* common_connect_job_params, std::unique_ptr connect_job_factory, SSLClientContext* ssl_client_context, bool connect_backup_jobs_enabled); @@ -281,8 +267,6 @@ class NET_EXPORT_PRIVATE TransportClientSocketPool void OnSSLConfigForServerChanged(const HostPortPair& server) override; private: - class ConnectJobFactoryImpl; - // Entry for a persistent socket which became idle at time |start_time|. struct IdleSocket { IdleSocket() : socket(nullptr) {} @@ -614,6 +598,8 @@ class NET_EXPORT_PRIVATE TransportClientSocketPool base::TimeDelta unused_idle_socket_timeout, base::TimeDelta used_idle_socket_timeout, const ProxyServer& proxy_server, + bool is_for_websockets, + const CommonConnectJobParams* common_connect_job_params, std::unique_ptr connect_job_factory, SSLClientContext* ssl_client_context, bool connect_backup_jobs_enabled); @@ -805,8 +791,6 @@ class NET_EXPORT_PRIVATE TransportClientSocketPool const ProxyServer proxy_server_; - const std::unique_ptr connect_job_factory_; - // TODO(vandebo) Remove when backup jobs move to TransportClientSocketPool bool connect_backup_jobs_enabled_; diff --git a/net/socket/transport_connect_job.cc b/net/socket/transport_connect_job.cc index 555b0e7e71fb64..240c0e2931673d 100644 --- a/net/socket/transport_connect_job.cc +++ b/net/socket/transport_connect_job.cc @@ -60,6 +60,18 @@ TransportSocketParams::TransportSocketParams( TransportSocketParams::~TransportSocketParams() = default; +std::unique_ptr TransportConnectJob::Factory::Create( + RequestPriority priority, + const SocketTag& socket_tag, + const CommonConnectJobParams* common_connect_job_params, + const scoped_refptr& params, + Delegate* delegate, + const NetLogWithSource* net_log) { + return std::make_unique(priority, socket_tag, + common_connect_job_params, + params, delegate, net_log); +} + // TODO(eroman): The use of this constant needs to be re-evaluated. The time // needed for TCPClientSocketXXX::Connect() can be arbitrarily long, since // the address list may contain many alternatives, and most of those may diff --git a/net/socket/transport_connect_job.h b/net/socket/transport_connect_job.h index 6fbf0b9a3926fe..83871a52194ab8 100644 --- a/net/socket/transport_connect_job.h +++ b/net/socket/transport_connect_job.h @@ -81,6 +81,20 @@ class NET_EXPORT_PRIVATE TransportConnectJob : public ConnectJob { RACE_IPV6_SOLO, }; + class NET_EXPORT_PRIVATE Factory { + public: + Factory() = default; + virtual ~Factory() = default; + + virtual std::unique_ptr Create( + RequestPriority priority, + const SocketTag& socket_tag, + const CommonConnectJobParams* common_connect_job_params, + const scoped_refptr& params, + Delegate* delegate, + const NetLogWithSource* net_log); + }; + // TransportConnectJobs will time out after this many seconds. Note this is // the total time, including both host resolution and TCP connect() times. static const int kTimeoutInSeconds; diff --git a/net/socket/websocket_transport_client_socket_pool.cc b/net/socket/websocket_transport_client_socket_pool.cc index 37488be81de529..7d32868a25137c 100644 --- a/net/socket/websocket_transport_client_socket_pool.cc +++ b/net/socket/websocket_transport_client_socket_pool.cc @@ -22,6 +22,7 @@ #include "net/log/net_log_source_type.h" #include "net/socket/client_socket_handle.h" #include "net/socket/connect_job.h" +#include "net/socket/connect_job_factory.h" #include "net/socket/websocket_endpoint_lock_manager.h" #include "net/socket/websocket_transport_connect_job.h" #include "net/traffic_annotation/network_traffic_annotation.h" @@ -33,12 +34,14 @@ WebSocketTransportClientSocketPool::WebSocketTransportClientSocketPool( int max_sockets_per_group, const ProxyServer& proxy_server, const CommonConnectJobParams* common_connect_job_params) - : proxy_server_(proxy_server), - common_connect_job_params_(common_connect_job_params), + : ClientSocketPool(/*is_for_websockets=*/true, + common_connect_job_params, + std::make_unique()), + proxy_server_(proxy_server), max_sockets_(max_sockets), handed_out_socket_count_(0), flushing_(false) { - DCHECK(common_connect_job_params_->websocket_endpoint_lock_manager); + DCHECK(common_connect_job_params->websocket_endpoint_lock_manager); } WebSocketTransportClientSocketPool::~WebSocketTransportClientSocketPool() { @@ -105,7 +108,6 @@ int WebSocketTransportClientSocketPool::RequestSocket( std::unique_ptr connect_job = CreateConnectJob(group_id, params, proxy_server_, proxy_annotation_tag, - true /* is_for_websockets */, common_connect_job_params_, priority, SocketTag(), connect_job_delegate.get()); int result = connect_job_delegate->Connect(std::move(connect_job)); diff --git a/net/socket/websocket_transport_client_socket_pool.h b/net/socket/websocket_transport_client_socket_pool.h index e6438a3c07133d..d82281f2d5d948 100644 --- a/net/socket/websocket_transport_client_socket_pool.h +++ b/net/socket/websocket_transport_client_socket_pool.h @@ -200,7 +200,6 @@ class NET_EXPORT_PRIVATE WebSocketTransportClientSocketPool bool DeleteStalledRequest(ClientSocketHandle* handle); const ProxyServer proxy_server_; - const CommonConnectJobParams* const common_connect_job_params_; std::set pending_callbacks_; PendingConnectsMap pending_connects_; StalledRequestQueue stalled_request_queue_; diff --git a/net/socket/websocket_transport_connect_job.cc b/net/socket/websocket_transport_connect_job.cc index 4993f3e62c79bc..ae3d73fb684512 100644 --- a/net/socket/websocket_transport_connect_job.cc +++ b/net/socket/websocket_transport_connect_job.cc @@ -26,6 +26,19 @@ namespace net { +std::unique_ptr +WebSocketTransportConnectJob::Factory::Create( + RequestPriority priority, + const SocketTag& socket_tag, + const CommonConnectJobParams* common_connect_job_params, + const scoped_refptr& params, + Delegate* delegate, + const NetLogWithSource* net_log) { + return std::make_unique( + priority, socket_tag, common_connect_job_params, params, delegate, + net_log); +} + WebSocketTransportConnectJob::WebSocketTransportConnectJob( RequestPriority priority, const SocketTag& socket_tag, diff --git a/net/socket/websocket_transport_connect_job.h b/net/socket/websocket_transport_connect_job.h index b4aae0735f58bf..ab739039418306 100644 --- a/net/socket/websocket_transport_connect_job.h +++ b/net/socket/websocket_transport_connect_job.h @@ -38,6 +38,20 @@ class WebSocketTransportConnectSubJob; // logging), and provide performance information to SocketPerformanceWatcher. class NET_EXPORT_PRIVATE WebSocketTransportConnectJob : public ConnectJob { public: + class NET_EXPORT_PRIVATE Factory { + public: + Factory() = default; + virtual ~Factory() = default; + + virtual std::unique_ptr Create( + RequestPriority priority, + const SocketTag& socket_tag, + const CommonConnectJobParams* common_connect_job_params, + const scoped_refptr& params, + Delegate* delegate, + const NetLogWithSource* net_log); + }; + WebSocketTransportConnectJob( RequestPriority priority, const SocketTag& socket_tag, diff --git a/services/network/proxy_resolving_client_socket.cc b/services/network/proxy_resolving_client_socket.cc index 71b2e6df5ddee9..e26a3bac78a8e1 100644 --- a/services/network/proxy_resolving_client_socket.cc +++ b/services/network/proxy_resolving_client_socket.cc @@ -28,6 +28,7 @@ #include "net/log/net_log_source_type.h" #include "net/proxy_resolution/configured_proxy_resolution_service.h" #include "net/proxy_resolution/proxy_resolution_request.h" +#include "net/socket/connect_job_factory.h" #include "net/socket/socket_tag.h" #include "net/ssl/ssl_config.h" #include "net/traffic_annotation/network_traffic_annotation.h" @@ -40,9 +41,11 @@ ProxyResolvingClientSocket::ProxyResolvingClientSocket( const net::CommonConnectJobParams* common_connect_job_params, const GURL& url, const net::NetworkIsolationKey& network_isolation_key, - bool use_tls) + bool use_tls, + const net::ConnectJobFactory* connect_job_factory) : network_session_(network_session), common_connect_job_params_(common_connect_job_params), + connect_job_factory_(connect_job_factory), url_(url), network_isolation_key_(network_isolation_key), use_tls_(use_tls), @@ -52,6 +55,7 @@ ProxyResolvingClientSocket::ProxyResolvingClientSocket( // TODO(xunjieli): Handle invalid URLs more gracefully (at mojo API layer // or when the request is created). DCHECK(url_.is_valid()); + DCHECK(connect_job_factory_); } ProxyResolvingClientSocket::~ProxyResolvingClientSocket() {} @@ -298,7 +302,7 @@ int ProxyResolvingClientSocket::DoInitConnection() { // // TODO(mmenke): Investigate that. net::SSLConfig ssl_config; - connect_job_ = net::ConnectJob::CreateConnectJob( + connect_job_ = connect_job_factory_->CreateConnectJob( use_tls_, net::HostPortPair::FromURL(url_), proxy_info_.proxy_server(), proxy_annotation_tag, &ssl_config, &ssl_config, true /* force_tunnel */, net::PRIVACY_MODE_DISABLED, net::OnHostResolutionCallback(), diff --git a/services/network/proxy_resolving_client_socket.h b/services/network/proxy_resolving_client_socket.h index 6ce2a2bd215908..73ccb82d981b23 100644 --- a/services/network/proxy_resolving_client_socket.h +++ b/services/network/proxy_resolving_client_socket.h @@ -29,6 +29,7 @@ namespace net { struct CommonConnectJobParams; +class ConnectJobFactory; class HttpAuthController; class HttpResponseInfo; class HttpNetworkSession; @@ -45,20 +46,21 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) ProxyResolvingClientSocket : public net::StreamSocket, public net::ConnectJob::Delegate { public: - // Constructs a new ProxyResolvingClientSocket. |url|'s host and port specify + // Constructs a new ProxyResolvingClientSocket. `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::ProxyResolutionService - // before the url is disclosed to the PAC script. If |use_tls|, this will try - // to do a tls connect instead of a regular tcp connect. |network_session| and - // |common_connect_job_params| must outlive |this|. + // before the url is disclosed to the PAC script. If `use_tls`, this will try + // to do a tls connect instead of a regular tcp connect. `network_session`, + // `common_connect_job_params`, and `connect_job_factory` must outlive `this`. ProxyResolvingClientSocket( net::HttpNetworkSession* network_session, const net::CommonConnectJobParams* common_connect_job_params, const GURL& url, const net::NetworkIsolationKey& network_isolation_key, - bool use_tls); + bool use_tls, + const net::ConnectJobFactory* connect_job_factory); ~ProxyResolvingClientSocket() override; // net::StreamSocket implementation. @@ -129,6 +131,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) ProxyResolvingClientSocket net::HttpNetworkSession* network_session_; const net::CommonConnectJobParams* common_connect_job_params_; + const net::ConnectJobFactory* connect_job_factory_; std::unique_ptr connect_job_; std::unique_ptr socket_; diff --git a/services/network/proxy_resolving_client_socket_factory.cc b/services/network/proxy_resolving_client_socket_factory.cc index 99c3886ed36771..7313f145dcd063 100644 --- a/services/network/proxy_resolving_client_socket_factory.cc +++ b/services/network/proxy_resolving_client_socket_factory.cc @@ -98,7 +98,7 @@ ProxyResolvingClientSocketFactory::CreateSocket( network_session_->http_auth_cache()->CopyProxyEntriesFrom(*other_auth_cache); return std::make_unique( network_session_.get(), common_connect_job_params_.get(), url, - network_isolation_key, use_tls); + network_isolation_key, use_tls, connect_job_factory_.get()); } } // namespace network diff --git a/services/network/proxy_resolving_client_socket_factory.h b/services/network/proxy_resolving_client_socket_factory.h index 01410d980d93ef..8974abbbf7208b 100644 --- a/services/network/proxy_resolving_client_socket_factory.h +++ b/services/network/proxy_resolving_client_socket_factory.h @@ -10,6 +10,7 @@ #include "base/component_export.h" #include "base/macros.h" #include "base/memory/ref_counted.h" +#include "net/socket/connect_job_factory.h" #include "net/ssl/ssl_config.h" #include "url/gurl.h" @@ -61,6 +62,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) ProxyResolvingClientSocketFactory { std::unique_ptr network_session_; std::unique_ptr common_connect_job_params_; net::URLRequestContext* request_context_; + std::unique_ptr connect_job_factory_ = + std::make_unique(); DISALLOW_COPY_AND_ASSIGN(ProxyResolvingClientSocketFactory); }; diff --git a/services/network/proxy_resolving_client_socket_unittest.cc b/services/network/proxy_resolving_client_socket_unittest.cc index c4f797ffd69bc2..e88392e1daa549 100644 --- a/services/network/proxy_resolving_client_socket_unittest.cc +++ b/services/network/proxy_resolving_client_socket_unittest.cc @@ -26,6 +26,7 @@ #include "net/proxy_resolution/proxy_config_service_fixed.h" #include "net/proxy_resolution/proxy_config_with_annotation.h" #include "net/socket/client_socket_pool_manager.h" +#include "net/socket/connect_job_factory.h" #include "net/socket/socket_test_util.h" #include "net/spdy/spdy_test_util_common.h" #include "net/test/gtest_util.h" @@ -236,13 +237,15 @@ TEST_P(ProxyResolvingClientSocketTest, NetworkIsolationKeyWithH2Proxy) { ssl_data2.next_proto = net::kProtoHTTP2; session_deps.socket_factory->AddSSLSocketDataProvider(&ssl_data2); + net::ConnectJobFactory connect_job_factory; + // Connect to kDestination1 using kNetworkIsolationKey1. It should use a new // H2 session. net::CommonConnectJobParams common_connect_job_params = http_network_session->CreateCommonConnectJobParams(); ProxyResolvingClientSocket socket1( http_network_session.get(), &common_connect_job_params, kDestination1, - kNetworkIsolationKey1, false /* use_tls */); + kNetworkIsolationKey1, false /* use_tls */, &connect_job_factory); net::TestCompletionCallback callback1; int result = socket1.Connect(callback1.callback()); EXPECT_THAT(callback1.GetResult(result), net::test::IsOk()); @@ -251,7 +254,7 @@ TEST_P(ProxyResolvingClientSocketTest, NetworkIsolationKeyWithH2Proxy) { // H2 session. ProxyResolvingClientSocket socket2( http_network_session.get(), &common_connect_job_params, kDestination2, - kNetworkIsolationKey2, false /* use_tls */); + kNetworkIsolationKey2, false /* use_tls */, &connect_job_factory); net::TestCompletionCallback callback2; result = socket2.Connect(callback2.callback()); EXPECT_THAT(callback2.GetResult(result), net::test::IsOk()); @@ -262,7 +265,7 @@ TEST_P(ProxyResolvingClientSocketTest, NetworkIsolationKeyWithH2Proxy) { // first H2 session. ProxyResolvingClientSocket socket3( http_network_session.get(), &common_connect_job_params, kDestination3, - kNetworkIsolationKey1, false /* use_tls */); + kNetworkIsolationKey1, false /* use_tls */, &connect_job_factory); net::TestCompletionCallback callback3; result = socket3.Connect(callback3.callback()); EXPECT_THAT(callback3.GetResult(result), net::test::IsOk());