From a9b8be01f9a8e738babcf1422e171e5adeee04a3 Mon Sep 17 00:00:00 2001 From: Eric Orth Date: Tue, 29 Jun 2021 23:09:08 +0000 Subject: [PATCH] Implement ConnectJobFactory Refactor intended to make ConnectJob construction logic more testable. Each ConnectJob type now has its own mockable Factory class to allow replacing the individual construction in tests. The new ConnectJobFactory contains the logic that was previously in ConnectJob::CreateConnectJob() but now in a stateful class to keep the individual type factories and allow replacing them for tests. The ConnectJobFactory instances are owned by the ClientSocketPools (or by network::ProxyResolvingClientSocketFactory for mojo socket creation). ConnectJobFactory also replaces the previously-used TransportClientSocketPool::ConnectJobFactory. But where that lived in TransportClientSocketPool and called into ClientSocketPool::CreateConnectJob() (where ClientSocketPool added a bit of extra logic to create an OnHostResolutionCallback), now the ConnectJobFactory lives in the ClientSocketPool and the extra callback stuff is done before calling into the factory. Change-Id: Id28959a7eb2180399f3a79ed64a323b1ee225461 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2986127 Commit-Queue: Eric Orth Commit-Queue: Matt Menke Auto-Submit: Eric Orth Reviewed-by: Matt Menke Cr-Commit-Position: refs/heads/master@{#897145} --- net/BUILD.gn | 3 + net/http/http_proxy_connect_job.cc | 13 + net/http/http_proxy_connect_job.h | 14 + net/socket/client_socket_pool.cc | 27 +- net/socket/client_socket_pool.h | 14 +- .../client_socket_pool_base_unittest.cc | 69 +-- net/socket/connect_job.cc | 92 ---- net/socket/connect_job.h | 26 -- net/socket/connect_job_factory.cc | 160 +++++++ net/socket/connect_job_factory.h | 82 ++++ net/socket/connect_job_factory_unittest.cc | 411 ++++++++++++++++++ net/socket/socks_connect_job.cc | 12 + net/socket/socks_connect_job.h | 14 + net/socket/ssl_connect_job.cc | 13 + net/socket/ssl_connect_job.h | 14 + net/socket/transport_client_socket_pool.cc | 90 ++-- net/socket/transport_client_socket_pool.h | 26 +- net/socket/transport_connect_job.cc | 12 + net/socket/transport_connect_job.h | 14 + .../websocket_transport_client_socket_pool.cc | 10 +- .../websocket_transport_client_socket_pool.h | 1 - net/socket/websocket_transport_connect_job.cc | 13 + net/socket/websocket_transport_connect_job.h | 14 + .../network/proxy_resolving_client_socket.cc | 8 +- .../network/proxy_resolving_client_socket.h | 13 +- .../proxy_resolving_client_socket_factory.cc | 2 +- .../proxy_resolving_client_socket_factory.h | 3 + .../proxy_resolving_client_socket_unittest.cc | 9 +- 28 files changed, 915 insertions(+), 264 deletions(-) create mode 100644 net/socket/connect_job_factory.cc create mode 100644 net/socket/connect_job_factory.h create mode 100644 net/socket/connect_job_factory_unittest.cc 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());