diff options
author | ttuttle <ttuttle@chromium.org> | 2015-04-28 09:17:47 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-04-28 16:18:24 +0000 |
commit | 1f2d7e9e15aee462a5e80c703f6a7d4886e23c25 (patch) | |
tree | 5fc62eba6de448a7ced34f3aa9160792ad878a21 | |
parent | 58b00a59bff3c44ef6e41f77d2afba4408f4564a (diff) | |
download | chromium_src-1f2d7e9e15aee462a5e80c703f6a7d4886e23c25.zip chromium_src-1f2d7e9e15aee462a5e80c703f6a7d4886e23c25.tar.gz chromium_src-1f2d7e9e15aee462a5e80c703f6a7d4886e23c25.tar.bz2 |
Plumb connection attempts from (non-proxy) ConnectJobs to HttpNetworkTransaction.
Log connection attempts in {Transport,SSL}ConnectJob and copy them into the ClientSocketHandle. Form there, copy them up the stack through the HttpStreamRequest (on job completion) to the HttpNetworkTransaction (on stream creation or failure).
SPDY is covered by this change -- sockets for new SPDY sessions are connected on the same path as sockets for HTTP connections, and then passed off to the SPDY code.
QUIC is not covered by this change -- sockets for new QUIC sessions are created in the QUIC code, and Chrome's current QUIC behavior does not let us see those results easily. My current plan is to log those *in* the QUIC code and then grab them on the next request that would use that QUIC session. I still need to see if this is actually practical.
BUG=480565
Review URL: https://codereview.chromium.org/1006643002
Cr-Commit-Position: refs/heads/master@{#327298}
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) { |