Collect all ConnectionAttempts from both sockets in TransportConnectJob.

net/http/http_stream_factory_impl_job.cc

Index: net/http/http_stream_factory_impl_job.cc
diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc
index 1172bf1f6dcd3d1f649322cf15b925dcb1ea5c5f..1d17b087a903ee955c2670303cfd4f2a2a90e695 100644
--- a/net/http/http_stream_factory_impl_job.cc
+++ b/net/http/http_stream_factory_impl_job.cc
@@ -296,7 +296,7 @@ void HttpStreamFactoryImpl::Job::OnStreamReadyCallback() {
   DCHECK(!IsPreconnecting());
   DCHECK(!stream_factory_->for_websockets_);
 
-  MaybeCopyConnectionAttemptsFromClientSocketHandleToRequest();
+  MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
   if (IsOrphaned()) {
     stream_factory_->OnOrphanedJobComplete(this);
@@ -319,7 +319,7 @@ void HttpStreamFactoryImpl::Job::OnWebSocketHandshakeStreamReadyCallback() {
   // never be ready.
   DCHECK(!IsOrphaned());
 
-  MaybeCopyConnectionAttemptsFromClientSocketHandleToRequest();
+  MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
   request_->Complete(was_npn_negotiated(),
                      protocol_negotiated(),
@@ -341,7 +341,7 @@ void HttpStreamFactoryImpl::Job::OnNewSpdySessionReadyCallback() {
   base::WeakPtr<SpdySession> spdy_session = new_spdy_session_;
   new_spdy_session_.reset();
 
-  MaybeCopyConnectionAttemptsFromClientSocketHandleToRequest();
+  MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
   // TODO(jgraettinger): Notify the factory, and let that notify |request_|,
   // rather than notifying |request_| directly.
@@ -362,7 +362,7 @@ void HttpStreamFactoryImpl::Job::OnNewSpdySessionReadyCallback() {
 void HttpStreamFactoryImpl::Job::OnStreamFailedCallback(int result) {
   DCHECK(!IsPreconnecting());
 
-  MaybeCopyConnectionAttemptsFromClientSocketHandleToRequest();
+  MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
   if (IsOrphaned())
     stream_factory_->OnOrphanedJobComplete(this);
@@ -375,7 +375,7 @@ void HttpStreamFactoryImpl::Job::OnCertificateErrorCallback(
     int result, const SSLInfo& ssl_info) {
   DCHECK(!IsPreconnecting());
 
-  MaybeCopyConnectionAttemptsFromClientSocketHandleToRequest();
+  MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
   if (IsOrphaned())
     stream_factory_->OnOrphanedJobComplete(this);
@@ -1526,12 +1526,23 @@ HttpStreamFactoryImpl::Job::GetSocketGroup() const {
   return ClientSocketPoolManager::NORMAL_GROUP;
 }
 
+// If the connection succeeds, failed connection attempts leading up to the
+// success will be returned via the successfully connected socket. If the
+// connection fails, failed connection attempts will be returned via the
+// ClientSocketHandle. Check whether a socket was returned and copy the
+// connection attempts from the proper place.
 void HttpStreamFactoryImpl::Job::
-    MaybeCopyConnectionAttemptsFromClientSocketHandleToRequest() {
+    MaybeCopyConnectionAttemptsFromSocketOrHandle() {
   if (IsOrphaned() || !connection_)
     return;
 
-  request_->AddConnectionAttempts(connection_->connection_attempts());
+  if (connection_->socket()) {

[Randy Smith (Not in Mondays), 2015/05/14 19:42:05]

Is there a way to write a unit test to test this code?

[Deprecated (see juliatuttle), 2015/05/14 20:02:35]

I think it is implicitly tested by the tests at the URLRequest layer -- the
fallback ones cover the success case, and the basic failure ones cover the
failure case.

+    ConnectionAttempts socket_attempts;
+    connection_->socket()->GetConnectionAttempts(&socket_attempts);
+    request_->AddConnectionAttempts(socket_attempts);
+  } else {
+    request_->AddConnectionAttempts(connection_->connection_attempts());
+  }
 }
 
 }  // namespace net