JobController 1: Adding a new class HttpStreamFactoryImpl::JobController

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 152ec3f3d71fd62294f9d379d4eb677affd24ce8..401e8a8d1c53b32a9f826056f8c81cee7a40f117 100644
--- a/net/http/http_stream_factory_impl_job.cc
+++ b/net/http/http_stream_factory_impl_job.cc
@@ -35,6 +35,7 @@
 #include "net/http/http_request_info.h"
 #include "net/http/http_server_properties.h"
 #include "net/http/http_stream_factory.h"
+#include "net/http/http_stream_factory_impl_job_controller.h"
 #include "net/http/http_stream_factory_impl_request.h"
 #include "net/log/net_log.h"
 #include "net/quic/quic_http_stream.h"
@@ -154,7 +155,8 @@ std::unique_ptr<base::Value> NetLogHttpStreamProtoCallback(
   return std::move(dict);
 }
 
-HttpStreamFactoryImpl::Job::Job(HttpStreamFactoryImpl* stream_factory,
+HttpStreamFactoryImpl::Job::Job(JobController* job_controller,
+                                JobType job_type,
                                 HttpNetworkSession* session,
                                 const HttpRequestInfo& request_info,
                                 RequestPriority priority,
@@ -163,7 +165,8 @@ HttpStreamFactoryImpl::Job::Job(HttpStreamFactoryImpl* stream_factory,
                                 HostPortPair destination,
                                 GURL origin_url,
                                 NetLog* net_log)
-    : Job(stream_factory,
+    : Job(job_controller,
+          job_type,
           session,
           request_info,
           priority,
@@ -174,7 +177,8 @@ HttpStreamFactoryImpl::Job::Job(HttpStreamFactoryImpl* stream_factory,
           AlternativeService(),
           net_log) {}
 
-HttpStreamFactoryImpl::Job::Job(HttpStreamFactoryImpl* stream_factory,
+HttpStreamFactoryImpl::Job::Job(JobController* job_controller,
+                                JobType job_type,
                                 HttpNetworkSession* session,
                                 const HttpRequestInfo& request_info,
                                 RequestPriority priority,
@@ -184,8 +188,7 @@ HttpStreamFactoryImpl::Job::Job(HttpStreamFactoryImpl* stream_factory,
                                 GURL origin_url,
                                 AlternativeService alternative_service,
                                 NetLog* net_log)
-    : request_(NULL),
-      request_info_(request_info),
+    : request_info_(request_info),
       priority_(priority),
       server_ssl_config_(server_ssl_config),
       proxy_ssl_config_(proxy_ssl_config),
@@ -193,12 +196,13 @@ HttpStreamFactoryImpl::Job::Job(HttpStreamFactoryImpl* stream_factory,
       io_callback_(base::Bind(&Job::OnIOComplete, base::Unretained(this))),
       connection_(new ClientSocketHandle),
       session_(session),
-      stream_factory_(stream_factory),
       next_state_(STATE_NONE),
       pac_request_(NULL),
       destination_(destination),
       origin_url_(origin_url),
       alternative_service_(alternative_service),
+      job_controller_(job_controller),
+      job_type_(job_type),
       blocking_job_(NULL),
       waiting_job_(NULL),
       using_ssl_(false),
@@ -216,7 +220,6 @@ HttpStreamFactoryImpl::Job::Job(HttpStreamFactoryImpl* stream_factory,
       other_job_status_(STATUS_RUNNING),
       stream_type_(HttpStreamRequest::BIDIRECTIONAL_STREAM),
       ptr_factory_(this) {
-  DCHECK(stream_factory);
   DCHECK(session);
   if (IsQuicAlternative()) {
     DCHECK(session_->params().enable_quic);
@@ -243,11 +246,9 @@ HttpStreamFactoryImpl::Job::~Job() {
     stream_->Close(true /* not reusable */);
 }
 
-void HttpStreamFactoryImpl::Job::Start(Request* request) {
-  DCHECK(request);
-  request_ = request;
-  // Saves |stream_type_|, since request is nulled when job is orphaned.
-  stream_type_ = request_->stream_type();
+void HttpStreamFactoryImpl::Job::Start(
+    HttpStreamRequest::StreamType stream_type) {
+  stream_type_ = stream_type;
   StartInternal();
 }
 
@@ -326,9 +327,7 @@ void HttpStreamFactoryImpl::Job::Resume(Job* job,
     ResumeAfterDelay();
 }
 
-void HttpStreamFactoryImpl::Job::Orphan(const Request* request) {
-  DCHECK_EQ(request_, request);
-  request_ = NULL;
+void HttpStreamFactoryImpl::Job::Orphan() {

[Randy Smith (Not in Mondays), 2016/05/16 20:46:39]

A parallel comment to the one on this methods declaration which may render that
moot: We've moved the concept of "orphaned" partially up into the JobController
(as evidenced by the fact that there's no longer an IsOrphaned() method on this
class).  Could we move it completely up there?  The tests below, except for
blocking_job_ which I *think* the controller has independent information on, are
all reaching into the controller for information.  Can we instead, if the
controller wants to Orphan() a job, have it just call a method that does the
disconnect and then delete the job, if the appropriate conditions are satisfied?

[Zhongyi Shi, 2016/05/16 23:08:53]

We could, but the removal of blocking_job_ and waiting_job_ is not in this CL,
so we probably won't want to do it here. In addition, "if the controller wants
to Orphan() a job, have it just call a method that does the disconnect and then
delete the job": I thought this is what orphan will do eventually once we
removed blocking_job and waiting_job (which are used for Job scheduling). I
didn't plan to send it early, but perhaps it's a good ref:
https://codereview.chromium.org/1952423002/diff/20001/net/http/http_stream_fa...


Note we might want to move calls around to the caller of Job::Orphan and rename
the method in the final patch. 

[Randy Smith (Not in Mondays), 2016/05/23 21:39:26]

I'm good with doing it in anther CL.  I'm happy to discuss what the method
should be named in that CL :-}.

   net_log_.AddEvent(NetLog::TYPE_HTTP_STREAM_JOB_ORPHANED);
   if (blocking_job_) {
     // We've been orphaned, but there's a job we're blocked on. Don't bother
@@ -336,19 +335,19 @@ void HttpStreamFactoryImpl::Job::Orphan(const Request* request) {
     DCHECK(blocking_job_->waiting_job_);
     blocking_job_->waiting_job_ = NULL;
     blocking_job_ = NULL;
-    if (stream_factory_->for_websockets_ &&
-        connection_ && connection_->socket()) {
+    if (job_controller_->for_websockets() && connection_ &&
+        connection_->socket()) {
       connection_->socket()->Disconnect();
     }
-    stream_factory_->OnOrphanedJobComplete(this);
-  } else if (stream_factory_->for_websockets_) {
+    job_controller_->OnOrphanedJobComplete(this);
+  } else if (job_controller_->for_websockets()) {
     // We cancel this job because a WebSocketHandshakeStream can't be created
     // without a WebSocketHandshakeStreamBase::CreateHelper which is stored in
-    // the Request class and isn't accessible from this job.
+    // the Request class and isn't retrievable by this job.

[Randy Smith (Not in Mondays), 2016/05/16 20:46:39]

nit: Is the comment reference to the Request class still valid?

[Zhongyi Shi, 2016/05/16 23:08:53]

Yup. The only thing I changed is the word "accessible" as we used to have a
request_ entry in Job. But now we need to query JobController to retrieve
Request. There's no change to the ownership of
WebSocketHandshakeStreamBase::CreateHelper.

[Randy Smith (Not in Mondays), 2016/05/23 21:39:26]

I'll worry about the architecture in the next CL, so this comment doesn't
request any action, but just for explanation: The point of this refactor (to my
mind) is making it so that the Job class doesn't need to know about the Request
class.  Having a reference from Job->Request, even if only in a comment,
suggests that we haven't completely managed this.  But that's tied in with my
other concerns about the "Orphan()" interface, so it's more approriate to deal
wiht them in the next CL.

     if (connection_ && connection_->socket()) {
       connection_->socket()->Disconnect();
     }
-    stream_factory_->OnOrphanedJobComplete(this);
+    job_controller_->OnOrphanedJobComplete(this);

[Randy Smith (Not in Mondays), 2016/05/16 20:46:39]

consistency nit: Elsewhere in this file there are comments if the controller may
have deleted this on controller notifications; should there be one here too? 
(And above.)

[Zhongyi Shi, 2016/05/16 23:08:53]

Done.

   }
 }
 
@@ -417,40 +416,27 @@ bool HttpStreamFactoryImpl::Job::CanUseExistingSpdySession() const {
 
 void HttpStreamFactoryImpl::Job::OnStreamReadyCallback() {
   DCHECK(stream_.get());
-  DCHECK(!IsPreconnecting());
-  DCHECK(!stream_factory_->for_websockets_);
+  DCHECK(job_type_ != PRECONNECT);
+  DCHECK(!job_controller_->for_websockets());
 
   UMA_HISTOGRAM_TIMES("Net.HttpStreamFactoryJob.StreamReadyCallbackTime",
                       base::TimeTicks::Now() - job_stream_ready_start_time_);
 
   MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
-  if (IsOrphaned()) {
-    stream_factory_->OnOrphanedJobComplete(this);
-  } else {
-    request_->Complete(was_npn_negotiated(), protocol_negotiated(),
-                       using_spdy());
-    request_->OnStreamReady(this, server_ssl_config_, proxy_info_,
-                            stream_.release());
-  }
+  job_controller_->OnStreamReady(this, server_ssl_config_, proxy_info_);
   // |this| may be deleted after this call.
 }
 
 void HttpStreamFactoryImpl::Job::OnWebSocketHandshakeStreamReadyCallback() {
   DCHECK(websocket_stream_);
-  DCHECK(!IsPreconnecting());
-  DCHECK(stream_factory_->for_websockets_);
-  // An orphaned WebSocket job will be closed immediately and
-  // never be ready.
-  DCHECK(!IsOrphaned());
+  DCHECK(job_type_ != PRECONNECT);
+  DCHECK(job_controller_->for_websockets());
 
   MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
-  request_->Complete(was_npn_negotiated(), protocol_negotiated(), using_spdy());
-  request_->OnWebSocketHandshakeStreamReady(this,
-                                            server_ssl_config_,
-                                            proxy_info_,
-                                            websocket_stream_.release());
+  job_controller_->OnWebSocketHandshakeStreamReady(
+      this, server_ssl_config_, proxy_info_, websocket_stream_.release());
   // |this| may be deleted after this call.
 }
 
@@ -459,21 +445,14 @@ void HttpStreamFactoryImpl::Job::OnBidirectionalStreamImplReadyCallback() {
 
   MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
-  if (IsOrphaned()) {
-    stream_factory_->OnOrphanedJobComplete(this);
-  } else {
-    request_->Complete(was_npn_negotiated(), protocol_negotiated(),
-                       using_spdy());
-    request_->OnBidirectionalStreamImplReady(
-        this, server_ssl_config_, proxy_info_,
-        bidirectional_stream_impl_.release());
-  }
-// |this| may be deleted after this call.
+  job_controller_->OnBidirectionalStreamImplReady(this, server_ssl_config_,
+                                                  proxy_info_);
+  // |this| may be deleted after this call.
 }
 
 void HttpStreamFactoryImpl::Job::OnNewSpdySessionReadyCallback() {
   DCHECK(stream_.get() || bidirectional_stream_impl_.get());
-  DCHECK(!IsPreconnecting());
+  DCHECK(job_type_ != PRECONNECT);
   DCHECK(using_spdy());
   // Note: an event loop iteration has passed, so |new_spdy_session_| may be
   // NULL at this point if the SpdySession closed immediately after creation.
@@ -482,107 +461,70 @@ void HttpStreamFactoryImpl::Job::OnNewSpdySessionReadyCallback() {
 
   MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
-  // TODO(jgraettinger): Notify the factory, and let that notify |request_|,
-  // rather than notifying |request_| directly.
-  if (IsOrphaned()) {
-    if (spdy_session) {
-      stream_factory_->OnNewSpdySessionReady(
-          spdy_session, spdy_session_direct_, server_ssl_config_, proxy_info_,
-          was_npn_negotiated(), protocol_negotiated(), using_spdy(), net_log_);
-    }
-    stream_factory_->OnOrphanedJobComplete(this);
-  } else {
-    if (stream_type_ == HttpStreamRequest::BIDIRECTIONAL_STREAM) {
-      DCHECK(bidirectional_stream_impl_);
-      request_->OnNewSpdySessionReady(this, /*spdy_http_stream=*/nullptr,
-                                      std::move(bidirectional_stream_impl_),
-                                      spdy_session, spdy_session_direct_);
-    } else {
-      DCHECK(stream_);
-      request_->OnNewSpdySessionReady(this, std::move(stream_),
-                                      /** bidirectional_stream_impl=*/nullptr,
-                                      spdy_session, spdy_session_direct_);
-    }
-  }
+  job_controller_->OnNewSpdySessionReady(this, spdy_session,
+                                         spdy_session_direct_);
+
   // |this| may be deleted after this call.
 }
 
 void HttpStreamFactoryImpl::Job::OnStreamFailedCallback(int result) {
-  DCHECK(!IsPreconnecting());
+  DCHECK(job_type_ != PRECONNECT);
 
   MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
-  if (IsOrphaned()) {
-    stream_factory_->OnOrphanedJobComplete(this);
-  } else {
-    SSLFailureState ssl_failure_state =
-        connection_ ? connection_->ssl_failure_state() : SSL_FAILURE_NONE;
-    request_->OnStreamFailed(this, result, server_ssl_config_,
-                             ssl_failure_state);
-  }
+  SSLFailureState ssl_failure_state =
+      connection_ ? connection_->ssl_failure_state() : SSL_FAILURE_NONE;
+
+  job_controller_->OnStreamFailed(this, result, server_ssl_config_,
+                                  ssl_failure_state);
   // |this| may be deleted after this call.
 }
 
 void HttpStreamFactoryImpl::Job::OnCertificateErrorCallback(
     int result, const SSLInfo& ssl_info) {
-  DCHECK(!IsPreconnecting());
+  DCHECK(job_type_ != PRECONNECT);
 
   MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
-  if (IsOrphaned())
-    stream_factory_->OnOrphanedJobComplete(this);
-  else
-    request_->OnCertificateError(this, result, server_ssl_config_, ssl_info);
+  job_controller_->OnCertificateError(this, result, server_ssl_config_,
+                                      ssl_info);
   // |this| may be deleted after this call.
 }
 
 void HttpStreamFactoryImpl::Job::OnNeedsProxyAuthCallback(
     const HttpResponseInfo& response,
     HttpAuthController* auth_controller) {
-  DCHECK(!IsPreconnecting());
-  if (IsOrphaned())
-    stream_factory_->OnOrphanedJobComplete(this);
-  else
-    request_->OnNeedsProxyAuth(
-        this, response, server_ssl_config_, proxy_info_, auth_controller);
+  DCHECK(job_type_ != PRECONNECT);
+
+  job_controller_->OnNeedsProxyAuth(this, response, server_ssl_config_,
+                                    proxy_info_, auth_controller);
   // |this| may be deleted after this call.
 }
 
 void HttpStreamFactoryImpl::Job::OnNeedsClientAuthCallback(
     SSLCertRequestInfo* cert_info) {
-  DCHECK(!IsPreconnecting());
-  if (IsOrphaned())
-    stream_factory_->OnOrphanedJobComplete(this);
-  else
-    request_->OnNeedsClientAuth(this, server_ssl_config_, cert_info);
+  DCHECK(job_type_ != PRECONNECT);
+
+  job_controller_->OnNeedsClientAuth(this, server_ssl_config_, cert_info);
   // |this| may be deleted after this call.
 }
 
 void HttpStreamFactoryImpl::Job::OnHttpsProxyTunnelResponseCallback(
     const HttpResponseInfo& response_info,
     HttpStream* stream) {
-  DCHECK(!IsPreconnecting());
-  if (IsOrphaned())
-    stream_factory_->OnOrphanedJobComplete(this);
-  else
-    request_->OnHttpsProxyTunnelResponse(
-        this, response_info, server_ssl_config_, proxy_info_, stream);
+  DCHECK(job_type_ != PRECONNECT);
+
+  job_controller_->OnHttpsProxyTunnelResponse(
+      this, response_info, server_ssl_config_, proxy_info_, stream);
   // |this| may be deleted after this call.
 }
 
 void HttpStreamFactoryImpl::Job::OnPreconnectsComplete() {
-  DCHECK(!request_);
   if (new_spdy_session_.get()) {
-    stream_factory_->OnNewSpdySessionReady(new_spdy_session_,
-                                           spdy_session_direct_,
-                                           server_ssl_config_,
-                                           proxy_info_,
-                                           was_npn_negotiated(),
-                                           protocol_negotiated(),
-                                           using_spdy(),
-                                           net_log_);
-  }
-  stream_factory_->OnPreconnectsComplete(this);
+    job_controller_->OnNewSpdySessionReady(this, new_spdy_session_,
+                                           spdy_session_direct_);
+  }
+  job_controller_->OnPreconnectsComplete(this);
   // |this| may be deleted after this call.
 }
 
@@ -620,7 +562,7 @@ int HttpStreamFactoryImpl::Job::RunLoop(int result) {
   // if there was one.
   DCHECK(result == OK || waiting_job_ == NULL);
 
-  if (IsPreconnecting()) {
+  if (job_type_ == PRECONNECT) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
         base::Bind(&HttpStreamFactoryImpl::Job::OnPreconnectsComplete,
@@ -692,7 +634,7 @@ int HttpStreamFactoryImpl::Job::RunLoop(int result) {
         base::ThreadTaskRunnerHandle::Get()->PostTask(
             FROM_HERE, base::Bind(&Job::OnNewSpdySessionReadyCallback,
                                   ptr_factory_.GetWeakPtr()));
-      } else if (stream_factory_->for_websockets_) {
+      } else if (job_controller_->for_websockets()) {
         DCHECK(websocket_stream_);
         base::ThreadTaskRunnerHandle::Get()->PostTask(
             FROM_HERE, base::Bind(&Job::OnWebSocketHandshakeStreamReadyCallback,
@@ -804,16 +746,16 @@ int HttpStreamFactoryImpl::Job::StartInternal() {
 int HttpStreamFactoryImpl::Job::DoStart() {
   valid_spdy_session_pool_.reset(new ValidSpdySessionPool(
       session_->spdy_session_pool(), origin_url_, IsSpdyAlternative()));
+  const BoundNetLog* net_log = job_controller_->GetNetLog(this);
 
-  net_log_.BeginEvent(
-      NetLog::TYPE_HTTP_STREAM_JOB,
-      base::Bind(&NetLogHttpStreamJobCallback,
-                 request_ ? request_->net_log().source() : NetLog::Source(),
-                 &request_info_.url, &origin_url_, &alternative_service_,
-                 priority_));
-  if (request_) {
-    request_->net_log().AddEvent(NetLog::TYPE_HTTP_STREAM_REQUEST_STARTED_JOB,
-                                 net_log_.source().ToEventParametersCallback());
+  if (net_log) {
+    net_log_.BeginEvent(
+        NetLog::TYPE_HTTP_STREAM_JOB,
+        base::Bind(&NetLogHttpStreamJobCallback, net_log->source(),
+                   &request_info_.url, &origin_url_, &alternative_service_,
+                   priority_));
+    net_log->AddEvent(NetLog::TYPE_HTTP_STREAM_REQUEST_STARTED_JOB,
+                      net_log_.source().ToEventParametersCallback());
   }
 
   // Don't connect to restricted ports.
@@ -1048,7 +990,7 @@ int HttpStreamFactoryImpl::Job::DoInitConnection() {
     if (spdy_session) {
       // If we're preconnecting, but we already have a SpdySession, we don't
       // actually need to preconnect any sockets, so we're done.
-      if (IsPreconnecting())
+      if (job_type_ == PRECONNECT)
         return OK;
       using_spdy_ = true;
       next_state_ = STATE_CREATE_STREAM;
@@ -1056,9 +998,10 @@ int HttpStreamFactoryImpl::Job::DoInitConnection() {
       return OK;
     }
   }
-  if (request_ && !request_->HasSpdySessionKey() && using_ssl_) {
-    // Update the spdy session key for the request that launched this job.
-    request_->SetSpdySessionKey(spdy_session_key);
+  if (using_ssl_) {
+    // Ask |job_controller_| to update the spdy session key for the request
+    // that launched this job.
+    job_controller_->SetSpdySessionKey(this, spdy_session_key);
   }
 
   // OK, there's no available SPDY session. Let |waiting_job_| resume if it's
@@ -1083,8 +1026,8 @@ int HttpStreamFactoryImpl::Job::DoInitConnection() {
     }
   }
 
-  if (IsPreconnecting()) {
-    DCHECK(!stream_factory_->for_websockets_);
+  if (job_type_ == PRECONNECT) {
+    DCHECK(!job_controller_->for_websockets());
     return PreconnectSocketsForHttpRequest(
         GetSocketGroup(), destination_, request_info_.extra_headers,
         request_info_.load_flags, priority_, session_, proxy_info_, expect_spdy,
@@ -1099,7 +1042,7 @@ int HttpStreamFactoryImpl::Job::DoInitConnection() {
           ? base::Bind(&Job::OnHostResolution, session_->spdy_session_pool(),
                        spdy_session_key, origin_url_)
           : OnHostResolutionCallback();
-  if (stream_factory_->for_websockets_) {
+  if (job_controller_->for_websockets()) {
     // TODO(ricea): Re-enable NPN when WebSockets over SPDY is supported.
     SSLConfig websocket_server_ssl_config = server_ssl_config_;
     websocket_server_ssl_config.alpn_protos.clear();
@@ -1124,7 +1067,7 @@ int HttpStreamFactoryImpl::Job::DoInitConnectionComplete(int result) {
     waiting_job_->Resume(this, base::TimeDelta());
     waiting_job_ = NULL;
   }
-  if (IsPreconnecting()) {
+  if (job_type_ == PRECONNECT) {
     if (using_quic_)
       return result;
     DCHECK_EQ(OK, result);
@@ -1317,7 +1260,7 @@ int HttpStreamFactoryImpl::Job::SetSpdyHttpStreamOrBidirectionalStreamImpl(
     bool direct) {
   // TODO(ricea): Restore the code for WebSockets over SPDY once it's
   // implemented.
-  if (stream_factory_->for_websockets_)
+  if (job_controller_->for_websockets())
     return ERR_NOT_IMPLEMENTED;
   if (stream_type_ == HttpStreamRequest::BIDIRECTIONAL_STREAM) {
     bidirectional_stream_impl_.reset(new BidirectionalStreamSpdyImpl(session));
@@ -1362,11 +1305,11 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() {
     bool using_proxy = (proxy_info_.is_http() || proxy_info_.is_https()) &&
                        (request_info_.url.SchemeIs("http") ||
                         request_info_.url.SchemeIs("ftp"));
-    if (stream_factory_->for_websockets_) {
-      DCHECK(request_);
-      DCHECK(request_->websocket_handshake_stream_create_helper());
+    if (job_controller_->for_websockets()) {
+      DCHECK(job_type_ != PRECONNECT);
+      DCHECK(job_controller_->websocket_handshake_stream_create_helper());
       websocket_stream_.reset(
-          request_->websocket_handshake_stream_create_helper()
+          job_controller_->websocket_handshake_stream_create_helper()
               ->CreateBasicStream(std::move(connection_), using_proxy));
     } else {
       stream_.reset(new HttpBasicStream(connection_.release(), using_proxy));
@@ -1488,8 +1431,7 @@ void HttpStreamFactoryImpl::Job::ReturnToStateInitConnection(
     connection_->socket()->Disconnect();
   connection_->Reset();
 
-  if (request_)
-    request_->RemoveRequestFromSpdySessionRequestMap();
+  job_controller_->RemoveRequestFromSpdySessionRequestMap(this);
 
   next_state_ = STATE_INIT_CONNECTION;
 }
@@ -1631,8 +1573,7 @@ int HttpStreamFactoryImpl::Job::ReconsiderProxyAfterError(int error) {
     if (connection_->socket())
       connection_->socket()->Disconnect();
     connection_->Reset();
-    if (request_)
-      request_->RemoveRequestFromSpdySessionRequestMap();
+    job_controller_->RemoveRequestFromSpdySessionRequestMap(this);
     next_state_ = STATE_RESOLVE_PROXY_COMPLETE;
   } else {
     // If ReconsiderProxyAfterError() failed synchronously, it means
@@ -1683,15 +1624,6 @@ void HttpStreamFactoryImpl::Job::SwitchToSpdyMode() {
     using_spdy_ = true;
 }
 
-bool HttpStreamFactoryImpl::Job::IsPreconnecting() const {
-  DCHECK_GE(num_streams_, 0);
-  return num_streams_ > 0;
-}
-
-bool HttpStreamFactoryImpl::Job::IsOrphaned() const {
-  return !IsPreconnecting() && !request_;
-}
-
 void HttpStreamFactoryImpl::Job::ReportJobSucceededForRequest() {
   if (using_existing_quic_session_) {
     // If an existing session was used, then no TCP connection was
@@ -1802,16 +1734,15 @@ HttpStreamFactoryImpl::Job::GetSocketGroup() const {
 // connection attempts from the proper place.
 void HttpStreamFactoryImpl::Job::
     MaybeCopyConnectionAttemptsFromSocketOrHandle() {
-  if (IsOrphaned() || !connection_)
+  if (!connection_)
     return;
 
+  ConnectionAttempts socket_attempts = connection_->connection_attempts();
   if (connection_->socket()) {
-    ConnectionAttempts socket_attempts;
     connection_->socket()->GetConnectionAttempts(&socket_attempts);
-    request_->AddConnectionAttempts(socket_attempts);
-  } else {
-    request_->AddConnectionAttempts(connection_->connection_attempts());
   }
+
+  job_controller_->AddConnectionAttemptsToRequest(this, socket_attempts);
 }
 
 }  // namespace net