JobController 1: Adding a new class HttpStreamFactoryImpl::JobController

net/http/http_stream_factory_impl_job.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_stream_factory_impl_job.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 
@@ skipping 17 matching lines @@
 #include "net/base/port_util.h"
 #include "net/cert/cert_verifier.h"
 #include "net/http/bidirectional_stream_impl.h"
 #include "net/http/http_basic_stream.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_proxy_client_socket.h"
 #include "net/http/http_proxy_client_socket_pool.h"
 #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"
 #include "net/socket/client_socket_handle.h"
 #include "net/socket/client_socket_pool.h"
 #include "net/socket/client_socket_pool_manager.h"
 #include "net/socket/socks_client_socket_pool.h"
 #include "net/socket/ssl_client_socket.h"
 #include "net/socket/ssl_client_socket_pool.h"
 #include "net/spdy/bidirectional_stream_spdy_impl.h"
@@ skipping 99 matching lines @@
     const std::string* proto,
     NetLogCaptureMode /* capture_mode */) {
   std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
 
   dict->SetString("next_proto_status",
                   SSLClientSocket::NextProtoStatusToString(status));
   dict->SetString("proto", *proto);
   return std::move(dict);
 }
 
-HttpStreamFactoryImpl::Job::Job(HttpStreamFactoryImpl* stream_factory,
+HttpStreamFactoryImpl::Job::Job(JobController* job_controller,
                                 HttpNetworkSession* session,
                                 const HttpRequestInfo& request_info,
                                 RequestPriority priority,
                                 const SSLConfig& server_ssl_config,
                                 const SSLConfig& proxy_ssl_config,
                                 HostPortPair destination,
                                 GURL origin_url,
                                 NetLog* net_log)
-    : Job(stream_factory,
+    : Job(job_controller,
           session,
           request_info,
           priority,
           server_ssl_config,
           proxy_ssl_config,
           destination,
           origin_url,
           AlternativeService(),
           net_log) {}
 
-HttpStreamFactoryImpl::Job::Job(HttpStreamFactoryImpl* stream_factory,
+HttpStreamFactoryImpl::Job::Job(JobController* job_controller,
                                 HttpNetworkSession* session,
                                 const HttpRequestInfo& request_info,
                                 RequestPriority priority,
                                 const SSLConfig& server_ssl_config,
                                 const SSLConfig& proxy_ssl_config,
                                 HostPortPair destination,
                                 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),
       net_log_(BoundNetLog::Make(net_log, NetLog::SOURCE_HTTP_STREAM_JOB)),
       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),
       blocking_job_(NULL),
       waiting_job_(NULL),
+      orphaned_(false),
       using_ssl_(false),
       using_spdy_(false),
       using_quic_(false),
       quic_request_(session_->quic_stream_factory()),
       using_existing_quic_session_(false),
       spdy_certificate_error_(OK),
       establishing_tunnel_(false),
       was_npn_negotiated_(false),
       protocol_negotiated_(kProtoUnknown),
       num_streams_(0),
       spdy_session_direct_(false),
       job_status_(STATUS_RUNNING),
       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);
     using_quic_ = true;
   }
 }
 
 HttpStreamFactoryImpl::Job::~Job() {
   net_log_.EndEvent(NetLog::TYPE_HTTP_STREAM_JOB);
 
   // When we're in a partially constructed state, waiting for the user to
   // provide certificate handling information or authentication, we can't reuse
   // this stream at all.
   if (next_state_ == STATE_WAITING_USER_ACTION) {
     connection_->socket()->Disconnect();
     connection_.reset();
   }
 
   if (pac_request_)
     session_->proxy_service()->CancelPacRequest(pac_request_);
 
   // The stream could be in a partial state.  It is not reusable.
   if (stream_.get() && next_state_ != STATE_DONE)
     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();
+  stream_type_ = request->stream_type();

[Ryan Hamilton, 2016/05/06 20:49:01]

nit: In that case, I'd just pass in the stream_type instead of the full request.
You might be able to do this in constructor, though I'm not sure about that.

[Randy Smith (Not in Mondays), 2016/05/09 21:42:09]

Agreed.  Specifically, the design flow sorta promises a lack of interaction
between Request and Job; all such interactions now go through JobController. 
What's left is minimal, but leaving any such connections behind invites future
modifications to increase those connections.  So I'd be a bit ruthless in
removing such references.

[Zhongyi Shi, 2016/05/12 07:26:23]

Done. It's actually done in a subsequent CL, but I will be happy to bring it
ahead.

   StartInternal();
 }
 
 int HttpStreamFactoryImpl::Job::Preconnect(int num_streams) {
   DCHECK_GT(num_streams, 0);
   base::WeakPtr<HttpServerProperties> http_server_properties =
       session_->http_server_properties();
   if (http_server_properties &&
       http_server_properties->SupportsRequestPriority(
           url::SchemeHostPort(request_info_.url))) {
@@ skipping 58 matching lines @@
   // be delayed by the |wait_time_| when it resumes.
   if (next_state_ == STATE_NONE || next_state_ <= STATE_WAIT_FOR_JOB_COMPLETE)
     wait_time_ = delay;
 
   // We know we're blocked if the next_state_ is STATE_WAIT_FOR_JOB_COMPLETE.
   // Unblock |this|.
   if (next_state_ == STATE_WAIT_FOR_JOB_COMPLETE)
     ResumeAfterDelay();
 }
 
 void HttpStreamFactoryImpl::Job::Orphan(const Request* request) {

[Randy Smith (Not in Mondays), 2016/05/09 21:42:09]

This routine no longer uses the request argument; remove it?  (See above for
context.)

[Zhongyi Shi, 2016/05/12 07:26:23]

Done.

-  DCHECK_EQ(request_, request);
-  request_ = NULL;
+  DCHECK(!orphaned_);
+  if (!IsPreconnecting())

[Ryan Hamilton, 2016/05/06 20:49:01]

Why not set this true for the preconnect case?

[Randy Smith (Not in Mondays), 2016/05/09 20:42:18]

I think to keep the old behavior--IsOrphaned() used to return false if
preconnect.

[Zhongyi Shi, 2016/05/12 07:26:23]

orphaned_ indicates whether |this| job has ever been associated with a Request
but then Orphaned. Preconnect job is never orphaned as it's never been
associated with a Request. 

+    orphaned_ = true;
   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
     // racing, just cancel ourself.
     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_ &&

[Randy Smith (Not in Mondays), 2016/05/09 20:42:18]

What aspects of your change changed the lifecycle such that connection_ could be
null at this point now but couldn't before?

[Randy Smith (Not in Mondays), 2016/05/09 21:42:09]

Whoops, sorry, missed that this is identical to previous code.

+        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.
     if (connection_ && connection_->socket()) {
       connection_->socket()->Disconnect();
     }
-    stream_factory_->OnOrphanedJobComplete(this);
+    job_controller_->OnOrphanedJobComplete(this);
   }
 }
 
 void HttpStreamFactoryImpl::Job::SetPriority(RequestPriority priority) {
   priority_ = priority;
   // TODO(akalin): Propagate this to |connection_| and maybe the
   // preconnect state.
 }
 
 bool HttpStreamFactoryImpl::Job::was_npn_negotiated() const {
@@ skipping 49 matching lines @@
   // https://crbug.com/133176
   // TODO(ricea): Add "wss" back to this list when SPDY WebSocket support is
   // working.
   return origin_url_.SchemeIs("https") ||
          proxy_info_.proxy_server().is_https() || IsSpdyAlternative();
 }
 
 void HttpStreamFactoryImpl::Job::OnStreamReadyCallback() {
   DCHECK(stream_.get());
   DCHECK(!IsPreconnecting());
-  DCHECK(!stream_factory_->for_websockets_);
+  DCHECK(!job_controller_->for_websockets());
 
   UMA_HISTOGRAM_TIMES("Net.HttpStreamFactoryJob.StreamReadyCallbackTime",
                       base::TimeTicks::Now() - job_stream_ready_start_time_);
 
   MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
   if (IsOrphaned()) {

[Ryan Hamilton, 2016/05/06 20:49:01]

Ooo! You're going to love (or probably) hate this :>

The controller knows (or could easily know) if a job is the bound or orphaned
job. As a result, I think the job can just do:

job_controller_->MarkRequestComplete(...)
job_controller_->OnStreamReady(...)

and the controller can do the check to see if the job has been orphaned. This
will make the job simpler. (Here and elsewhere). In fact, it wouldn't surprise
me if this meant that the job no longer needed to know about being orphaned,
which would be nice!

[Randy Smith (Not in Mondays), 2016/05/09 21:42:09]

+1, if it can be done.

[Zhongyi Shi, 2016/05/12 07:26:23]

Done.

-    stream_factory_->OnOrphanedJobComplete(this);
+    job_controller_->OnOrphanedJobComplete(this);
   } else {
-    request_->Complete(was_npn_negotiated(), protocol_negotiated(),
-                       using_spdy());
-    request_->OnStreamReady(this, server_ssl_config_, proxy_info_,
-                            stream_.release());
+    job_controller_->MarkRequestComplete(was_npn_negotiated(),
+                                         protocol_negotiated(), using_spdy());
+    job_controller_->OnStreamReady(this, server_ssl_config_, proxy_info_,
+                                   stream_.release());
   }
   // |this| may be deleted after this call.
 }
 
 void HttpStreamFactoryImpl::Job::OnWebSocketHandshakeStreamReadyCallback() {
   DCHECK(websocket_stream_);
   DCHECK(!IsPreconnecting());
-  DCHECK(stream_factory_->for_websockets_);
+  DCHECK(job_controller_->for_websockets());
   // An orphaned WebSocket job will be closed immediately and
   // never be ready.
   DCHECK(!IsOrphaned());
 
   MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
-  request_->Complete(was_npn_negotiated(), protocol_negotiated(), using_spdy());
-  request_->OnWebSocketHandshakeStreamReady(this,
-                                            server_ssl_config_,
-                                            proxy_info_,
-                                            websocket_stream_.release());
+  job_controller_->MarkRequestComplete(was_npn_negotiated(),
+                                       protocol_negotiated(), using_spdy());
+  job_controller_->OnWebSocketHandshakeStreamReady(
+      this, server_ssl_config_, proxy_info_, websocket_stream_.release());
   // |this| may be deleted after this call.
 }
 
 void HttpStreamFactoryImpl::Job::OnBidirectionalStreamImplReadyCallback() {
   DCHECK(bidirectional_stream_impl_);
 
   MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
   if (IsOrphaned()) {
-    stream_factory_->OnOrphanedJobComplete(this);
+    job_controller_->OnOrphanedJobComplete(this);
   } else {
-    request_->Complete(was_npn_negotiated(), protocol_negotiated(),
-                       using_spdy());
-    request_->OnBidirectionalStreamImplReady(
+    job_controller_->MarkRequestComplete(was_npn_negotiated(),
+                                         protocol_negotiated(), using_spdy());
+    job_controller_->OnBidirectionalStreamImplReady(
         this, server_ssl_config_, proxy_info_,
         bidirectional_stream_impl_.release());
   }
 // |this| may be deleted after this call.
 }
 
 void HttpStreamFactoryImpl::Job::OnNewSpdySessionReadyCallback() {
   DCHECK(stream_.get() || bidirectional_stream_impl_.get());
   DCHECK(!IsPreconnecting());
   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.
   base::WeakPtr<SpdySession> spdy_session = new_spdy_session_;
   new_spdy_session_.reset();
 
   MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
-  // TODO(jgraettinger): Notify the factory, and let that notify |request_|,
-  // rather than notifying |request_| directly.
+  // Notify the JobController, and let that notify Request and Factory.
   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_);
+      job_controller_->OnNewSpdySessionReady(
+          this, /*spdy_http_stream=*/nullptr,
+          /* bidirectional_stream_impl=*/nullptr, spdy_session,
+          spdy_session_direct_);
     }
-    stream_factory_->OnOrphanedJobComplete(this);
+    job_controller_->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_);
+      job_controller_->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, std::move(stream_),
+          /* bidirectional_stream_impl=*/nullptr, spdy_session,
+          spdy_session_direct_);
     }
   }
   // |this| may be deleted after this call.
 }
 
 void HttpStreamFactoryImpl::Job::OnStreamFailedCallback(int result) {
   DCHECK(!IsPreconnecting());
 
   MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
   if (IsOrphaned()) {
-    stream_factory_->OnOrphanedJobComplete(this);
+    job_controller_->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);
+    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());
 
   MaybeCopyConnectionAttemptsFromSocketOrHandle();
 
   if (IsOrphaned())
-    stream_factory_->OnOrphanedJobComplete(this);
+    job_controller_->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);
+    job_controller_->OnOrphanedJobComplete(this);
   else
-    request_->OnNeedsProxyAuth(
-        this, response, server_ssl_config_, proxy_info_, auth_controller);
+    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);
+    job_controller_->OnOrphanedJobComplete(this);
   else
-    request_->OnNeedsClientAuth(this, server_ssl_config_, cert_info);
+    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);
+    job_controller_->OnOrphanedJobComplete(this);
   else
-    request_->OnHttpsProxyTunnelResponse(
+    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_);
+    job_controller_->OnNewSpdySessionReady(
+        this, nullptr, nullptr, new_spdy_session_, spdy_session_direct_);
   }
-  stream_factory_->OnPreconnectsComplete(this);
+  job_controller_->OnPreconnectsComplete(this);
   // |this| may be deleted after this call.
 }
 
 // static
 int HttpStreamFactoryImpl::Job::OnHostResolution(
     SpdySessionPool* spdy_session_pool,
     const SpdySessionKey& spdy_session_key,
     const GURL& origin_url,
     const AddressList& addresses,
     const BoundNetLog& net_log) {
@@ skipping 89 matching lines @@
     }
 
     case OK:
       job_status_ = STATUS_SUCCEEDED;
       MaybeMarkAlternativeServiceBroken();
       next_state_ = STATE_DONE;
       if (new_spdy_session_.get()) {
         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,
                                   ptr_factory_.GetWeakPtr()));
       } else if (stream_type_ == HttpStreamRequest::BIDIRECTIONAL_STREAM) {
         if (!bidirectional_stream_impl_) {
           base::ThreadTaskRunnerHandle::Get()->PostTask(
               FROM_HERE, base::Bind(&Job::OnStreamFailedCallback,
                                     ptr_factory_.GetWeakPtr(), ERR_FAILED));
         } else {
@@ skipping 95 matching lines @@
   return rv;
 }
 
 int HttpStreamFactoryImpl::Job::DoStart() {
   valid_spdy_session_pool_.reset(new ValidSpdySessionPool(
       session_->spdy_session_pool(), origin_url_, IsSpdyAlternative()));
 
   net_log_.BeginEvent(
       NetLog::TYPE_HTTP_STREAM_JOB,
       base::Bind(&NetLogHttpStreamJobCallback,
-                 request_ ? request_->net_log().source() : NetLog::Source(),
+                 (!IsPreconnecting() && !orphaned_)
+                     ? job_controller_->GetNetLogFromRequest().source()
+                     : NetLog::Source(),

[Randy Smith (Not in Mondays), 2016/05/09 21:42:09]

Hmmm.  Parallel to Ryan's suggestions above, I'd be tempted to push the source
choice up into the JobController, and have it figure out whether it wants this
logged or not.  

It's a behavior change, but I could imagine at some point we'd want to log these
even if they're preconnecting or orphaned.

[Zhongyi Shi, 2016/05/12 07:26:23]

Done.

                  &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 (!IsPreconnecting() && !orphaned_) {
+    job_controller_->GetNetLogFromRequest().AddEvent(
+        NetLog::TYPE_HTTP_STREAM_REQUEST_STARTED_JOB,
+        net_log_.source().ToEventParametersCallback());
   }
 
   // Don't connect to restricted ports.
   if (!IsPortAllowedForScheme(destination_.port(),
                               request_info_.url.scheme())) {
     if (waiting_job_) {
       waiting_job_->Resume(this, base::TimeDelta());
       waiting_job_ = NULL;
     }
     return ERR_UNSAFE_PORT;
@@ skipping 222 matching lines @@
       // 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())
         return OK;
       using_spdy_ = true;
       next_state_ = STATE_CREATE_STREAM;
       existing_spdy_session_ = spdy_session;
       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 (!IsPreconnecting() && !orphaned_ && using_ssl_) {
+    // Ask |job_controller_| to update the spdy session key for the request
+    // that launched this job.
+    job_controller_->SetSpdySessionKey(spdy_session_key);
   }
 
   // OK, there's no available SPDY session. Let |waiting_job_| resume if it's
   // paused.
   if (waiting_job_) {
     waiting_job_->Resume(this, base::TimeDelta());
     waiting_job_ = NULL;
   }
 
   if (proxy_info_.is_http() || proxy_info_.is_https())
     establishing_tunnel_ = using_ssl_;
 
   const bool expect_spdy = IsSpdyAlternative();
 
   base::WeakPtr<HttpServerProperties> http_server_properties =
       session_->http_server_properties();
   if (http_server_properties) {
     http_server_properties->MaybeForceHTTP11(destination_, &server_ssl_config_);
     if (proxy_info_.is_http() || proxy_info_.is_https()) {
       http_server_properties->MaybeForceHTTP11(
           proxy_info_.proxy_server().host_port_pair(), &proxy_ssl_config_);
     }
   }
 
   if (IsPreconnecting()) {
-    DCHECK(!stream_factory_->for_websockets_);
+    DCHECK(!job_controller_->for_websockets());
     return PreconnectSocketsForHttpRequest(
         GetSocketGroup(), destination_, request_info_.extra_headers,
         request_info_.load_flags, priority_, session_, proxy_info_, expect_spdy,
         server_ssl_config_, proxy_ssl_config_, request_info_.privacy_mode,
         net_log_, num_streams_);
   }
 
   // If we can't use a SPDY session, don't bother checking for one after
   // the hostname is resolved.
   OnHostResolutionCallback resolution_callback =
       CanUseExistingSpdySession()
           ? 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();
     websocket_server_ssl_config.npn_protos.clear();
     return InitSocketHandleForWebSocketRequest(
         GetSocketGroup(), destination_, request_info_.extra_headers,
         request_info_.load_flags, priority_, session_, proxy_info_, expect_spdy,
         websocket_server_ssl_config, proxy_ssl_config_,
         request_info_.privacy_mode, net_log_, connection_.get(),
         resolution_callback, io_callback_);
@@ skipping 197 matching lines @@
 
   // We're always waiting here for the delegate to call us back.
   return ERR_IO_PENDING;
 }
 
 int HttpStreamFactoryImpl::Job::SetSpdyHttpStreamOrBidirectionalStreamImpl(
     base::WeakPtr<SpdySession> session,
     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));
     return OK;
   }
 
   // TODO(willchan): Delete this code, because eventually, the
   // HttpStreamFactoryImpl will be creating all the SpdyHttpStreams, since it
   // will know when SpdySessions become available.
 
@@ skipping 24 matching lines @@
   // need to plumb this through to the connect level.
   if (connection_->socket() && !connection_->is_reused())
     SetSocketMotivation();
 
   if (!using_spdy_) {
     DCHECK(!IsSpdyAlternative());
     // We may get ftp scheme when fetching ftp resources through proxy.
     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(!IsPreconnecting() && !orphaned_);
+      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));
     }
     return OK;
   }
 
   CHECK(!stream_.get());
 
   bool direct = !IsHttpsProxyAndHttpUrl();
@@ skipping 101 matching lines @@
 
   return ReconsiderProxyAfterError(result);
 }
 
 void HttpStreamFactoryImpl::Job::ReturnToStateInitConnection(
     bool close_connection) {
   if (close_connection && connection_->socket())
     connection_->socket()->Disconnect();
   connection_->Reset();
 
-  if (request_)
-    request_->RemoveRequestFromSpdySessionRequestMap();
+  if (!IsPreconnecting() && !orphaned_) {
+    job_controller_->RemoveRequestFromSpdySessionRequestMap();
+  }
 
   next_state_ = STATE_INIT_CONNECTION;
 }
 
 void HttpStreamFactoryImpl::Job::SetSocketMotivation() {
   if (request_info_.motivation == HttpRequestInfo::PRECONNECT_MOTIVATED)
     connection_->socket()->SetSubresourceSpeculation();
   else if (request_info_.motivation == HttpRequestInfo::OMNIBOX_MOTIVATED)
     connection_->socket()->SetOmniboxSpeculation();
   // TODO(mbelshe): Add other motivations (like EARLY_LOAD_MOTIVATED).
@@ skipping 121 matching lines @@
   int rv = session_->proxy_service()->ReconsiderProxyAfterError(
       request_info_.url, request_info_.method, request_info_.load_flags, error,
       &proxy_info_, io_callback_, &pac_request_,
       session_->params().proxy_delegate, net_log_);
   if (rv == OK || rv == ERR_IO_PENDING) {
     // If the error was during connection setup, there is no socket to
     // disconnect.
     if (connection_->socket())
       connection_->socket()->Disconnect();
     connection_->Reset();
-    if (request_)
-      request_->RemoveRequestFromSpdySessionRequestMap();
+    if (!IsPreconnecting() && !orphaned_) {
+      job_controller_->RemoveRequestFromSpdySessionRequestMap();
+    }
     next_state_ = STATE_RESOLVE_PROXY_COMPLETE;
   } else {
     // If ReconsiderProxyAfterError() failed synchronously, it means
     // there was nothing left to fall-back to, so fail the transaction
     // with the last connection error we got.
     // TODO(eroman): This is a confusing contract, make it more obvious.
     rv = error;
   }
 
   return rv;
@@ skipping 35 matching lines @@
 void HttpStreamFactoryImpl::Job::SwitchToSpdyMode() {
   if (HttpStreamFactory::spdy_enabled())
     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
     // started.
     HistogramAlternateProtocolUsage(ALTERNATE_PROTOCOL_USAGE_NO_RACE);
   } else if (IsSpdyAlternative() || IsQuicAlternative()) {
     // This Job was the alternative Job, and hence won the race.
     HistogramAlternateProtocolUsage(ALTERNATE_PROTOCOL_USAGE_WON_RACE);
   } else {
     // This Job was the normal Job, and hence the alternative Job lost the race.
@@ skipping 90 matching lines @@
   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::
     MaybeCopyConnectionAttemptsFromSocketOrHandle() {
-  if (IsOrphaned() || !connection_)
+  if (IsPreconnecting() || IsOrphaned() || !connection_)
     return;
 
   if (connection_->socket()) {
     ConnectionAttempts socket_attempts;
     connection_->socket()->GetConnectionAttempts(&socket_attempts);
-    request_->AddConnectionAttempts(socket_attempts);
+    job_controller_->AddConnectionAttemptsToRequest(socket_attempts);
   } else {
-    request_->AddConnectionAttempts(connection_->connection_attempts());
+    job_controller_->AddConnectionAttemptsToRequest(
+        connection_->connection_attempts());
   }
 }
 
 }  // namespace net