diff options
26 files changed, 253 insertions, 5 deletions
diff --git a/chrome/browser/devtools/devtools_network_transaction.cc b/chrome/browser/devtools/devtools_network_transaction.cc index 6a0f1b5..bd9b8e7 100644 --- a/chrome/browser/devtools/devtools_network_transaction.cc +++ b/chrome/browser/devtools/devtools_network_transaction.cc @@ -10,6 +10,7 @@ #include "net/base/upload_progress.h" #include "net/http/http_network_transaction.h" #include "net/http/http_request_info.h" +#include "net/socket/connection_attempts.h" // Keep in sync with kDevToolsRequestInitiator and // kDevToolsEmulateNetworkConditionsClientId defined in @@ -266,6 +267,12 @@ int DevToolsNetworkTransaction::ResumeNetworkStart() { return network_transaction_->ResumeNetworkStart(); } +void +DevToolsNetworkTransaction::GetConnectionAttempts(net::ConnectionAttempts* out) +const { + network_transaction_->GetConnectionAttempts(out); +} + void DevToolsNetworkTransaction::FireThrottledCallback() { DCHECK(!callback_.is_null()); DCHECK(callback_type_ == READ || callback_type_ == START); diff --git a/chrome/browser/devtools/devtools_network_transaction.h b/chrome/browser/devtools/devtools_network_transaction.h index 4d91fd2..826b002 100644 --- a/chrome/browser/devtools/devtools_network_transaction.h +++ b/chrome/browser/devtools/devtools_network_transaction.h @@ -106,6 +106,7 @@ class DevToolsNetworkTransaction : public net::HttpTransaction { void SetBeforeProxyHeadersSentCallback( const BeforeProxyHeadersSentCallback& callback) override; int ResumeNetworkStart() override; + void GetConnectionAttempts(net::ConnectionAttempts* out) const override; protected: friend class test::DevToolsNetworkControllerHelper; diff --git a/net/http/failing_http_transaction_factory.cc b/net/http/failing_http_transaction_factory.cc index a190597..b871576 100644 --- a/net/http/failing_http_transaction_factory.cc +++ b/net/http/failing_http_transaction_factory.cc @@ -10,6 +10,7 @@ #include "base/message_loop/message_loop.h" #include "net/base/load_timing_info.h" #include "net/base/upload_progress.h" +#include "net/socket/connection_attempts.h" namespace net { @@ -61,6 +62,7 @@ class FailingHttpTransaction : public HttpTransaction { void SetBeforeProxyHeadersSentCallback( const BeforeProxyHeadersSentCallback& callback) override; int ResumeNetworkStart() override; + void GetConnectionAttempts(ConnectionAttempts* out) const override; private: Error error_; @@ -163,6 +165,11 @@ int FailingHttpTransaction::ResumeNetworkStart() { return ERR_FAILED; } +void FailingHttpTransaction::GetConnectionAttempts( + ConnectionAttempts* out) const { + NOTIMPLEMENTED(); +} + } // namespace FailingHttpTransactionFactory::FailingHttpTransactionFactory( diff --git a/net/http/http_cache_transaction.cc b/net/http/http_cache_transaction.cc index 83fea07..fc6b68e 100644 --- a/net/http/http_cache_transaction.cc +++ b/net/http/http_cache_transaction.cc @@ -651,6 +651,17 @@ int HttpCache::Transaction::ResumeNetworkStart() { return ERR_UNEXPECTED; } +void HttpCache::Transaction::GetConnectionAttempts( + ConnectionAttempts* out) const { + ConnectionAttempts new_connection_attempts; + if (network_trans_) + network_trans_->GetConnectionAttempts(&new_connection_attempts); + + out->swap(new_connection_attempts); + out->insert(out->begin(), old_connection_attempts_.begin(), + old_connection_attempts_.end()); +} + //----------------------------------------------------------------------------- void HttpCache::Transaction::DoCallback(int rv) { @@ -2844,6 +2855,10 @@ void HttpCache::Transaction::ResetNetworkTransaction() { if (network_trans_->GetLoadTimingInfo(&load_timing)) old_network_trans_load_timing_.reset(new LoadTimingInfo(load_timing)); total_received_bytes_ += network_trans_->GetTotalReceivedBytes(); + ConnectionAttempts attempts; + network_trans_->GetConnectionAttempts(&attempts); + for (const auto& attempt : attempts) + old_connection_attempts_.push_back(attempt); network_trans_.reset(); } diff --git a/net/http/http_cache_transaction.h b/net/http/http_cache_transaction.h index d9d99e3..cbf0929 100644 --- a/net/http/http_cache_transaction.h +++ b/net/http/http_cache_transaction.h @@ -19,6 +19,7 @@ #include "net/http/http_response_info.h" #include "net/http/http_transaction.h" #include "net/log/net_log.h" +#include "net/socket/connection_attempts.h" namespace net { @@ -144,6 +145,7 @@ class HttpCache::Transaction : public HttpTransaction { void SetBeforeProxyHeadersSentCallback( const BeforeProxyHeadersSentCallback& callback) override; int ResumeNetworkStart() override; + void GetConnectionAttempts(ConnectionAttempts* out) const override; private: static const size_t kNumValidationHeaders = 2; @@ -470,6 +472,8 @@ class HttpCache::Transaction : public HttpTransaction { // before the caller requests load timing information. scoped_ptr<LoadTimingInfo> old_network_trans_load_timing_; + ConnectionAttempts old_connection_attempts_; + // The helper object to use to create WebSocketHandshakeStreamBase // objects. Only relevant when establishing a WebSocket connection. // This is passed to the underlying network transaction. It is stored here in diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc index 3e9074ac..9bf323d 100644 --- a/net/http/http_network_transaction.cc +++ b/net/http/http_network_transaction.cc @@ -571,6 +571,8 @@ void HttpNetworkTransaction::OnHttpsProxyTunnelResponse( HttpStream* stream) { DCHECK_EQ(STATE_CREATE_STREAM_COMPLETE, next_state_); + CopyConnectionAttemptsFromStreamRequest(); + headers_valid_ = true; response_ = response_info; server_ssl_config_ = used_ssl_config; @@ -582,6 +584,11 @@ void HttpNetworkTransaction::OnHttpsProxyTunnelResponse( OnIOComplete(ERR_HTTPS_PROXY_TUNNEL_RESPONSE); } +void HttpNetworkTransaction::GetConnectionAttempts( + ConnectionAttempts* out) const { + *out = connection_attempts_; +} + bool HttpNetworkTransaction::IsSecureRequest() const { return request_->url.SchemeIsSecure(); } @@ -771,6 +778,13 @@ int HttpNetworkTransaction::DoCreateStreamComplete(int result) { FROM_HERE_WITH_EXPLICIT_FUNCTION( "424359 HttpNetworkTransaction::DoCreateStreamComplete")); + // If |result| is ERR_HTTPS_PROXY_TUNNEL_RESPONSE, then + // DoCreateStreamComplete is being called from OnHttpsProxyTunnelResponse, + // which resets the stream request first. Therefore, we have to grab the + // connection attempts in *that* function instead of here in that case. + if (result != ERR_HTTPS_PROXY_TUNNEL_RESPONSE) + CopyConnectionAttemptsFromStreamRequest(); + if (result == OK) { next_state_ = STATE_INIT_STREAM; DCHECK(stream_.get()); @@ -1659,4 +1673,14 @@ std::string HttpNetworkTransaction::DescribeState(State state) { #undef STATE_CASE +void HttpNetworkTransaction::CopyConnectionAttemptsFromStreamRequest() { + DCHECK(stream_request_); + + // Since the transaction can restart with auth credentials, it may create a + // stream more than once. Accumulate all of the connection attempts across + // those streams by appending them to the vector: + for (const auto& attempt : stream_request_->connection_attempts()) + connection_attempts_.push_back(attempt); +} + } // namespace net diff --git a/net/http/http_network_transaction.h b/net/http/http_network_transaction.h index 1b7aac8..a3a9ef7 100644 --- a/net/http/http_network_transaction.h +++ b/net/http/http_network_transaction.h @@ -20,6 +20,7 @@ #include "net/http/http_transaction.h" #include "net/log/net_log.h" #include "net/proxy/proxy_service.h" +#include "net/socket/connection_attempts.h" #include "net/ssl/ssl_config_service.h" #include "net/websockets/websocket_handshake_stream_base.h" @@ -99,6 +100,8 @@ class NET_EXPORT_PRIVATE HttpNetworkTransaction const ProxyInfo& used_proxy_info, HttpStream* stream) override; + void GetConnectionAttempts(ConnectionAttempts* out) const override; + private: friend class HttpNetworkTransactionSSLTest; @@ -257,6 +260,8 @@ class NET_EXPORT_PRIVATE HttpNetworkTransaction void SetStream(HttpStream* stream); + void CopyConnectionAttemptsFromStreamRequest(); + scoped_refptr<HttpAuthController> auth_controllers_[HttpAuth::AUTH_NUM_TARGETS]; @@ -327,6 +332,8 @@ class NET_EXPORT_PRIVATE HttpNetworkTransaction BeforeNetworkStartCallback before_network_start_callback_; BeforeProxyHeadersSentCallback before_proxy_headers_sent_callback_; + ConnectionAttempts connection_attempts_; + DISALLOW_COPY_AND_ASSIGN(HttpNetworkTransaction); }; diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc index 24b0ca1..6db71f7 100644 --- a/net/http/http_network_transaction_unittest.cc +++ b/net/http/http_network_transaction_unittest.cc @@ -62,6 +62,7 @@ #include "net/proxy/proxy_service.h" #include "net/socket/client_socket_factory.h" #include "net/socket/client_socket_pool_manager.h" +#include "net/socket/connection_attempts.h" #include "net/socket/mock_client_socket_pool_manager.h" #include "net/socket/next_proto.h" #include "net/socket/socket_test_util.h" @@ -263,6 +264,7 @@ class HttpNetworkTransactionTest std::string response_data; int64 totalReceivedBytes; LoadTimingInfo load_timing_info; + ConnectionAttempts connection_attempts; }; void SetUp() override { @@ -384,6 +386,7 @@ class HttpNetworkTransactionTest response_headers); out.totalReceivedBytes = trans->GetTotalReceivedBytes(); + trans->GetConnectionAttempts(&out.connection_attempts); return out; } @@ -668,6 +671,7 @@ TEST_P(HttpNetworkTransactionTest, SimpleGET) { EXPECT_EQ("hello world", out.response_data); int64 reads_size = ReadsSize(data_reads, arraysize(data_reads)); EXPECT_EQ(reads_size, out.totalReceivedBytes); + EXPECT_EQ(0u, out.connection_attempts.size()); } // Response with no status line. @@ -12510,6 +12514,11 @@ TEST_P(HttpNetworkTransactionTest, HttpSyncConnectError) { // We don't care whether this succeeds or fails, but it shouldn't crash. HttpRequestHeaders request_headers; trans->GetFullRequestHeaders(&request_headers); + + ConnectionAttempts attempts; + trans->GetConnectionAttempts(&attempts); + ASSERT_EQ(1u, attempts.size()); + EXPECT_EQ(ERR_CONNECTION_REFUSED, attempts[0].result); } TEST_P(HttpNetworkTransactionTest, HttpAsyncConnectError) { @@ -12540,6 +12549,11 @@ TEST_P(HttpNetworkTransactionTest, HttpAsyncConnectError) { // We don't care whether this succeeds or fails, but it shouldn't crash. HttpRequestHeaders request_headers; trans->GetFullRequestHeaders(&request_headers); + + ConnectionAttempts attempts; + trans->GetConnectionAttempts(&attempts); + ASSERT_EQ(1u, attempts.size()); + EXPECT_EQ(ERR_CONNECTION_REFUSED, attempts[0].result); } TEST_P(HttpNetworkTransactionTest, HttpSyncWriteError) { @@ -12885,6 +12899,11 @@ class FakeStreamRequest : public HttpStreamRequest, bool using_spdy() const override { return false; } + const ConnectionAttempts& connection_attempts() const override { + static ConnectionAttempts no_attempts; + return no_attempts; + } + private: RequestPriority priority_; HttpStreamRequest::Delegate* const delegate_; diff --git a/net/http/http_stream_factory.h b/net/http/http_stream_factory.h index e6712623..60ff9d2 100644 --- a/net/http/http_stream_factory.h +++ b/net/http/http_stream_factory.h @@ -16,6 +16,7 @@ #include "net/base/net_export.h" #include "net/base/request_priority.h" #include "net/http/http_server_properties.h" +#include "net/socket/connection_attempts.h" // This file can be included from net/http even though // it is in net/websockets because it doesn't // introduce any link dependency to net/websockets. @@ -170,6 +171,9 @@ class NET_EXPORT_PRIVATE HttpStreamRequest { // Returns true if this stream is being fetched over SPDY. virtual bool using_spdy() const = 0; + + // Returns socket-layer connection attempts made for this stream request. + virtual const ConnectionAttempts& connection_attempts() const = 0; }; // The HttpStreamFactory defines an interface for creating usable HttpStreams. diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc index 10bb51f..b07a461 100644 --- a/net/http/http_stream_factory_impl_job.cc +++ b/net/http/http_stream_factory_impl_job.cc @@ -295,6 +295,9 @@ void HttpStreamFactoryImpl::Job::OnStreamReadyCallback() { DCHECK(stream_.get()); DCHECK(!IsPreconnecting()); DCHECK(!stream_factory_->for_websockets_); + + MaybeCopyConnectionAttemptsFromClientSocketHandleToRequest(); + if (IsOrphaned()) { stream_factory_->OnOrphanedJobComplete(this); } else { @@ -315,6 +318,9 @@ void HttpStreamFactoryImpl::Job::OnWebSocketHandshakeStreamReadyCallback() { // An orphaned WebSocket job will be closed immediately and // never be ready. DCHECK(!IsOrphaned()); + + MaybeCopyConnectionAttemptsFromClientSocketHandleToRequest(); + request_->Complete(was_npn_negotiated(), protocol_negotiated(), using_spdy(), @@ -335,6 +341,8 @@ void HttpStreamFactoryImpl::Job::OnNewSpdySessionReadyCallback() { base::WeakPtr<SpdySession> spdy_session = new_spdy_session_; new_spdy_session_.reset(); + MaybeCopyConnectionAttemptsFromClientSocketHandleToRequest(); + // TODO(jgraettinger): Notify the factory, and let that notify |request_|, // rather than notifying |request_| directly. if (IsOrphaned()) { @@ -353,6 +361,9 @@ void HttpStreamFactoryImpl::Job::OnNewSpdySessionReadyCallback() { void HttpStreamFactoryImpl::Job::OnStreamFailedCallback(int result) { DCHECK(!IsPreconnecting()); + + MaybeCopyConnectionAttemptsFromClientSocketHandleToRequest(); + if (IsOrphaned()) stream_factory_->OnOrphanedJobComplete(this); else @@ -363,6 +374,9 @@ void HttpStreamFactoryImpl::Job::OnStreamFailedCallback(int result) { void HttpStreamFactoryImpl::Job::OnCertificateErrorCallback( int result, const SSLInfo& ssl_info) { DCHECK(!IsPreconnecting()); + + MaybeCopyConnectionAttemptsFromClientSocketHandleToRequest(); + if (IsOrphaned()) stream_factory_->OnOrphanedJobComplete(this); else @@ -1534,4 +1548,12 @@ HttpStreamFactoryImpl::Job::GetSocketGroup() const { return ClientSocketPoolManager::NORMAL_GROUP; } +void HttpStreamFactoryImpl::Job:: + MaybeCopyConnectionAttemptsFromClientSocketHandleToRequest() { + if (IsOrphaned() || !connection_) + return; + + request_->AddConnectionAttempts(connection_->connection_attempts()); +} + } // namespace net diff --git a/net/http/http_stream_factory_impl_job.h b/net/http/http_stream_factory_impl_job.h index e19d0a9..a5793d1 100644 --- a/net/http/http_stream_factory_impl_job.h +++ b/net/http/http_stream_factory_impl_job.h @@ -280,6 +280,8 @@ class HttpStreamFactoryImpl::Job { ClientSocketPoolManager::SocketGroupType GetSocketGroup() const; + void MaybeCopyConnectionAttemptsFromClientSocketHandleToRequest(); + // Record histograms of latency until Connect() completes. static void LogHttpConnectedMetrics(const ClientSocketHandle& handle); diff --git a/net/http/http_stream_factory_impl_request.cc b/net/http/http_stream_factory_impl_request.cc index a884745..d8a4b5b 100644 --- a/net/http/http_stream_factory_impl_request.cc +++ b/net/http/http_stream_factory_impl_request.cc @@ -234,6 +234,11 @@ bool HttpStreamFactoryImpl::Request::using_spdy() const { return using_spdy_; } +const ConnectionAttempts& HttpStreamFactoryImpl::Request::connection_attempts() + const { + return connection_attempts_; +} + void HttpStreamFactoryImpl::Request::RemoveRequestFromSpdySessionRequestMap() { if (spdy_session_key_.get()) { @@ -315,6 +320,12 @@ void HttpStreamFactoryImpl::Request::OnNewSpdySessionReady( } } +void HttpStreamFactoryImpl::Request::AddConnectionAttempts( + const ConnectionAttempts& attempts) { + for (const auto& attempt : attempts) + connection_attempts_.push_back(attempt); +} + void HttpStreamFactoryImpl::Request::OrphanJobsExcept(Job* job) { DCHECK(job); DCHECK(!bound_job_.get()); diff --git a/net/http/http_stream_factory_impl_request.h b/net/http/http_stream_factory_impl_request.h index a70e68e..7bc41af 100644 --- a/net/http/http_stream_factory_impl_request.h +++ b/net/http/http_stream_factory_impl_request.h @@ -9,6 +9,7 @@ #include "base/memory/scoped_ptr.h" #include "net/http/http_stream_factory_impl.h" #include "net/log/net_log.h" +#include "net/socket/connection_attempts.h" #include "net/socket/ssl_client_socket.h" #include "net/spdy/spdy_session_key.h" #include "url/gurl.h" @@ -60,6 +61,10 @@ class HttpStreamFactoryImpl::Request : public HttpStreamRequest { const base::WeakPtr<SpdySession>& spdy_session, bool direct); + // Called by an attached Job to record connection attempts made by the socket + // layer for this stream request. + void AddConnectionAttempts(const ConnectionAttempts& attempts); + WebSocketHandshakeStreamBase::CreateHelper* websocket_handshake_stream_create_helper() { return websocket_handshake_stream_create_helper_; @@ -104,6 +109,7 @@ class HttpStreamFactoryImpl::Request : public HttpStreamRequest { bool was_npn_negotiated() const override; NextProto protocol_negotiated() const override; bool using_spdy() const override; + const ConnectionAttempts& connection_attempts() const override; private: // Used to orphan all jobs in |jobs_| other than |job| which becomes "bound" @@ -133,6 +139,7 @@ class HttpStreamFactoryImpl::Request : public HttpStreamRequest { // Protocol negotiated with the server. NextProto protocol_negotiated_; bool using_spdy_; + ConnectionAttempts connection_attempts_; DISALLOW_COPY_AND_ASSIGN(Request); }; diff --git a/net/http/http_transaction.h b/net/http/http_transaction.h index 52b3c97..5b7ea9b 100644 --- a/net/http/http_transaction.h +++ b/net/http/http_transaction.h @@ -10,6 +10,7 @@ #include "net/base/net_export.h" #include "net/base/request_priority.h" #include "net/base/upload_progress.h" +#include "net/socket/connection_attempts.h" #include "net/websockets/websocket_handshake_stream_base.h" namespace net { @@ -174,6 +175,8 @@ class NET_EXPORT_PRIVATE HttpTransaction { // Resumes the transaction after being deferred. virtual int ResumeNetworkStart() = 0; + + virtual void GetConnectionAttempts(ConnectionAttempts* out) const = 0; }; } // namespace net diff --git a/net/http/http_transaction_test_util.cc b/net/http/http_transaction_test_util.cc index 5951a5c..d6daf0e 100644 --- a/net/http/http_transaction_test_util.cc +++ b/net/http/http_transaction_test_util.cc @@ -439,6 +439,11 @@ int MockNetworkTransaction::ResumeNetworkStart() { return ERR_FAILED; } +void MockNetworkTransaction::GetConnectionAttempts( + ConnectionAttempts* out) const { + NOTIMPLEMENTED(); +} + void MockNetworkTransaction::CallbackLater(const CompletionCallback& callback, int result) { base::MessageLoop::current()->PostTask( diff --git a/net/http/http_transaction_test_util.h b/net/http/http_transaction_test_util.h index e63e433..163b042 100644 --- a/net/http/http_transaction_test_util.h +++ b/net/http/http_transaction_test_util.h @@ -24,6 +24,7 @@ #include "net/http/http_response_headers.h" #include "net/http/http_response_info.h" #include "net/log/net_log.h" +#include "net/socket/connection_attempts.h" namespace net { @@ -218,6 +219,8 @@ class MockNetworkTransaction int ResumeNetworkStart() override; + void GetConnectionAttempts(ConnectionAttempts* out) const override; + CreateHelper* websocket_handshake_stream_create_helper() { return websocket_handshake_stream_create_helper_; } diff --git a/net/net.gypi b/net/net.gypi index e481231..874d961 100644 --- a/net/net.gypi +++ b/net/net.gypi @@ -130,6 +130,7 @@ 'log/net_log_source_type_list.h', 'socket/client_socket_handle.cc', 'socket/client_socket_handle.h', + 'socket/connection_attempts.h', 'socket/next_proto.cc', 'socket/next_proto.h', 'socket/socket.h', diff --git a/net/socket/client_socket_handle.h b/net/socket/client_socket_handle.h index 532aec4..58cf4ca 100644 --- a/net/socket/client_socket_handle.h +++ b/net/socket/client_socket_handle.h @@ -12,6 +12,7 @@ #include "base/memory/scoped_ptr.h" #include "base/time/time.h" #include "net/base/completion_callback.h" +#include "net/base/ip_endpoint.h" #include "net/base/load_states.h" #include "net/base/load_timing_info.h" #include "net/base/net_errors.h" @@ -20,6 +21,7 @@ #include "net/http/http_response_info.h" #include "net/log/net_log.h" #include "net/socket/client_socket_pool.h" +#include "net/socket/connection_attempts.h" #include "net/socket/stream_socket.h" namespace net { @@ -136,6 +138,9 @@ class NET_EXPORT ClientSocketHandle { void set_pending_http_proxy_connection(ClientSocketHandle* connection) { pending_http_proxy_connection_.reset(connection); } + void set_connection_attempts(const ConnectionAttempts& attempts) { + connection_attempts_ = attempts; + } // Only valid if there is no |socket_|. bool is_ssl_error() const { @@ -151,6 +156,9 @@ class NET_EXPORT ClientSocketHandle { ClientSocketHandle* release_pending_http_proxy_connection() { return pending_http_proxy_connection_.release(); } + const ConnectionAttempts& connection_attempts() { + return connection_attempts_; + } StreamSocket* socket() { return socket_.get(); } @@ -200,6 +208,7 @@ class NET_EXPORT ClientSocketHandle { bool is_ssl_error_; HttpResponseInfo ssl_error_response_info_; scoped_ptr<ClientSocketHandle> pending_http_proxy_connection_; + std::vector<ConnectionAttempt> connection_attempts_; base::TimeTicks init_time_; base::TimeDelta setup_time_; diff --git a/net/socket/connection_attempts.h b/net/socket/connection_attempts.h new file mode 100644 index 0000000..241ffb0 --- /dev/null +++ b/net/socket/connection_attempts.h @@ -0,0 +1,29 @@ +// Copyright 2015 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_CONNECTION_ATTEMPTS_H_ +#define NET_SOCKET_CONNECTION_ATTEMPTS_H_ + +#include "net/base/ip_endpoint.h" + +namespace net { + +// A record of an connection attempt made to connect to a host. Includes TCP +// and SSL errors, but not proxy connections. +struct ConnectionAttempt { + ConnectionAttempt(const IPEndPoint endpoint, int result) + : endpoint(endpoint), result(result) {} + + // Address and port the socket layer attempted to connect to. + IPEndPoint endpoint; + + // Net error indicating the result of that attempt. + int result; +}; + +typedef std::vector<ConnectionAttempt> ConnectionAttempts; + +} // namespace net + +#endif // NET_SOCKET_CONNECTION_ATTEMPTS_H_ diff --git a/net/socket/socket_test_util.cc b/net/socket/socket_test_util.cc index a4557a9..78cbae6 100644 --- a/net/socket/socket_test_util.cc +++ b/net/socket/socket_test_util.cc @@ -1942,9 +1942,9 @@ MockTransportClientSocketPool::MockConnectJob::~MockConnectJob() {} int MockTransportClientSocketPool::MockConnectJob::Connect() { int rv = socket_->Connect(base::Bind(&MockConnectJob::OnConnect, base::Unretained(this))); - if (rv == OK) { + if (rv != ERR_IO_PENDING) { user_callback_.Reset(); - OnConnect(OK); + OnConnect(rv); } return rv; } @@ -1976,6 +1976,11 @@ void MockTransportClientSocketPool::MockConnectJob::OnConnect(int rv) { handle_->set_connect_timing(connect_timing); } else { socket_.reset(); + + // Needed to test copying of ConnectionAttempts in SSL ConnectJob. + ConnectionAttempts attempts; + attempts.push_back(ConnectionAttempt(IPEndPoint(), rv)); + handle_->set_connection_attempts(attempts); } handle_ = NULL; diff --git a/net/socket/ssl_client_socket_pool.cc b/net/socket/ssl_client_socket_pool.cc index 3426622..e0a7e3b 100644 --- a/net/socket/ssl_client_socket_pool.cc +++ b/net/socket/ssl_client_socket_pool.cc @@ -163,6 +163,8 @@ void SSLConnectJob::GetAdditionalErrorState(ClientSocketHandle* handle) { handle->set_ssl_error_response_info(error_response_info_); if (!connect_timing_.ssl_start.is_null()) handle->set_is_ssl_error(true); + + handle->set_connection_attempts(connection_attempts_); } void SSLConnectJob::OnIOComplete(int result) { @@ -232,8 +234,11 @@ int SSLConnectJob::DoTransportConnect() { } int SSLConnectJob::DoTransportConnectComplete(int result) { - if (result == OK) + connection_attempts_ = transport_socket_handle_->connection_attempts(); + if (result == OK) { next_state_ = STATE_SSL_CONNECT; + transport_socket_handle_->socket()->GetPeerAddress(&server_address_); + } return result; } @@ -329,6 +334,11 @@ int SSLConnectJob::DoSSLConnectComplete(int result) { connect_timing_.ssl_end = base::TimeTicks::Now(); + if (result != OK && !server_address_.address().empty()) { + connection_attempts_.push_back(ConnectionAttempt(server_address_, result)); + server_address_ = IPEndPoint(); + } + // If we want SPDY over ALPN/NPN, make sure it succeeded. if (params_->want_spdy_over_npn() && !NextProtoIsSPDY(ssl_socket_->GetNegotiatedProtocol())) { diff --git a/net/socket/ssl_client_socket_pool.h b/net/socket/ssl_client_socket_pool.h index 51415f6..3069c8d 100644 --- a/net/socket/ssl_client_socket_pool.h +++ b/net/socket/ssl_client_socket_pool.h @@ -14,6 +14,7 @@ #include "net/http/http_response_info.h" #include "net/socket/client_socket_pool.h" #include "net/socket/client_socket_pool_base.h" +#include "net/socket/connection_attempts.h" #include "net/socket/ssl_client_socket.h" #include "net/ssl/ssl_config_service.h" @@ -165,6 +166,12 @@ class SSLConnectJob : public ConnectJob { HttpResponseInfo error_response_info_; + ConnectionAttempts connection_attempts_; + // The address of the server the connect job is connected to. Populated if + // and only if the connect job is connected *directly* to the server (not + // through an HTTPS CONNECT request or a SOCKS proxy). + IPEndPoint server_address_; + DISALLOW_COPY_AND_ASSIGN(SSLConnectJob); }; diff --git a/net/socket/ssl_client_socket_pool_unittest.cc b/net/socket/ssl_client_socket_pool_unittest.cc index 19b7713..a9d3447 100644 --- a/net/socket/ssl_client_socket_pool_unittest.cc +++ b/net/socket/ssl_client_socket_pool_unittest.cc @@ -227,6 +227,8 @@ TEST_P(SSLClientSocketPoolTest, TCPFail) { EXPECT_FALSE(handle.is_initialized()); EXPECT_FALSE(handle.socket()); EXPECT_FALSE(handle.is_ssl_error()); + ASSERT_EQ(1u, handle.connection_attempts().size()); + EXPECT_EQ(ERR_CONNECTION_FAILED, handle.connection_attempts()[0].result); } TEST_P(SSLClientSocketPoolTest, TCPFailAsync) { @@ -250,6 +252,8 @@ TEST_P(SSLClientSocketPoolTest, TCPFailAsync) { EXPECT_FALSE(handle.is_initialized()); EXPECT_FALSE(handle.socket()); EXPECT_FALSE(handle.is_ssl_error()); + ASSERT_EQ(1u, handle.connection_attempts().size()); + EXPECT_EQ(ERR_CONNECTION_FAILED, handle.connection_attempts()[0].result); } TEST_P(SSLClientSocketPoolTest, BasicDirect) { @@ -271,6 +275,7 @@ TEST_P(SSLClientSocketPoolTest, BasicDirect) { EXPECT_TRUE(handle.is_initialized()); EXPECT_TRUE(handle.socket()); TestLoadTimingInfo(handle); + EXPECT_EQ(0u, handle.connection_attempts().size()); } // Make sure that SSLConnectJob passes on its priority to its diff --git a/net/socket/transport_client_socket_pool.cc b/net/socket/transport_client_socket_pool.cc index 5263e70..f2d1247 100644 --- a/net/socket/transport_client_socket_pool.cc +++ b/net/socket/transport_client_socket_pool.cc @@ -201,10 +201,15 @@ TransportConnectJob::TransportConnectJob( HostResolver* host_resolver, Delegate* delegate, NetLog* net_log) - : ConnectJob(group_name, timeout_duration, priority, delegate, + : ConnectJob(group_name, + timeout_duration, + priority, + delegate, BoundNetLog::Make(net_log, NetLog::SOURCE_CONNECT_JOB)), helper_(params, client_socket_factory, host_resolver, &connect_timing_), - interval_between_connects_(CONNECT_INTERVAL_GT_20MS) { + interval_between_connects_(CONNECT_INTERVAL_GT_20MS), + resolve_result_(OK), + connect_result_(OK) { helper_.SetOnIOComplete(this); } @@ -228,6 +233,23 @@ LoadState TransportConnectJob::GetLoadState() const { return LOAD_STATE_IDLE; } +void TransportConnectJob::GetAdditionalErrorState(ClientSocketHandle* handle) { + // If hostname resolution failed, record an empty endpoint and the result. + // If the actual socket Connect call failed, record the result and the last + // address attempted. + // TODO(ttuttle): Plumb into the socket layer and record *all* attempts. + ConnectionAttempts attempts; + if (resolve_result_ != OK) { + DCHECK_EQ(0u, helper_.addresses().size()); + attempts.push_back(ConnectionAttempt(IPEndPoint(), resolve_result_)); + } else if (connect_result_ != OK) { + DCHECK_LT(0u, helper_.addresses().size()); + attempts.push_back( + ConnectionAttempt(helper_.addresses().back(), connect_result_)); + } + handle->set_connection_attempts(attempts); +} + // static void TransportConnectJob::MakeAddressListStartWithIPv4(AddressList* list) { for (AddressList::iterator i = list->begin(); i != list->end(); ++i) { @@ -248,6 +270,7 @@ int TransportConnectJob::DoResolveHost() { } int TransportConnectJob::DoResolveHostComplete(int result) { + resolve_result_ = result; return helper_.DoResolveHostComplete(result, net_log()); } @@ -360,6 +383,8 @@ int TransportConnectJob::DoTransportConnectComplete(int result) { fallback_addresses_.reset(); } + connect_result_ = result; + return result; } diff --git a/net/socket/transport_client_socket_pool.h b/net/socket/transport_client_socket_pool.h index 48ddd13..de57d18 100644 --- a/net/socket/transport_client_socket_pool.h +++ b/net/socket/transport_client_socket_pool.h @@ -167,6 +167,7 @@ class NET_EXPORT_PRIVATE TransportConnectJob : public ConnectJob { // ConnectJob methods. LoadState GetLoadState() const override; + void GetAdditionalErrorState(ClientSocketHandle* handle) override; // Rolls |addrlist| forward until the first IPv4 address, if any. // WARNING: this method should only be used to implement the prefer-IPv4 hack. @@ -207,6 +208,9 @@ class NET_EXPORT_PRIVATE TransportConnectJob : public ConnectJob { // Track the interval between this connect and previous connect. ConnectInterval interval_between_connects_; + int resolve_result_; + int connect_result_; + DISALLOW_COPY_AND_ASSIGN(TransportConnectJob); }; diff --git a/net/socket/transport_client_socket_pool_unittest.cc b/net/socket/transport_client_socket_pool_unittest.cc index a14e4ef..dbfc506 100644 --- a/net/socket/transport_client_socket_pool_unittest.cc +++ b/net/socket/transport_client_socket_pool_unittest.cc @@ -185,6 +185,7 @@ TEST_F(TransportClientSocketPoolTest, Basic) { EXPECT_TRUE(handle.is_initialized()); EXPECT_TRUE(handle.socket()); TestLoadTimingInfoConnectedNotReused(handle); + EXPECT_EQ(0u, handle.connection_attempts().size()); } // Make sure that TransportConnectJob passes on its priority to its @@ -213,6 +214,9 @@ TEST_F(TransportClientSocketPoolTest, InitHostResolutionFailure) { handle.Init("a", dest, kDefaultPriority, callback.callback(), &pool_, BoundNetLog())); EXPECT_EQ(ERR_NAME_NOT_RESOLVED, callback.WaitForResult()); + ASSERT_EQ(1u, handle.connection_attempts().size()); + EXPECT_TRUE(handle.connection_attempts()[0].endpoint.address().empty()); + EXPECT_EQ(ERR_NAME_NOT_RESOLVED, handle.connection_attempts()[0].result); } TEST_F(TransportClientSocketPoolTest, InitConnectionFailure) { @@ -224,12 +228,20 @@ TEST_F(TransportClientSocketPoolTest, InitConnectionFailure) { handle.Init("a", params_, kDefaultPriority, callback.callback(), &pool_, BoundNetLog())); EXPECT_EQ(ERR_CONNECTION_FAILED, callback.WaitForResult()); + ASSERT_EQ(1u, handle.connection_attempts().size()); + EXPECT_EQ("127.0.0.1:80", + handle.connection_attempts()[0].endpoint.ToString()); + EXPECT_EQ(ERR_CONNECTION_FAILED, handle.connection_attempts()[0].result); // Make the host resolutions complete synchronously this time. host_resolver_->set_synchronous_mode(true); EXPECT_EQ(ERR_CONNECTION_FAILED, handle.Init("a", params_, kDefaultPriority, callback.callback(), &pool_, BoundNetLog())); + ASSERT_EQ(1u, handle.connection_attempts().size()); + EXPECT_EQ("127.0.0.1:80", + handle.connection_attempts()[0].endpoint.ToString()); + EXPECT_EQ(ERR_CONNECTION_FAILED, handle.connection_attempts()[0].result); } TEST_F(TransportClientSocketPoolTest, PendingRequests) { |