Extract Proxy Resolution out of HttpStreamFactoryImpl::Job

net/http/http_stream_factory_impl_job_controller.cc

Index: net/http/http_stream_factory_impl_job_controller.cc
diff --git a/net/http/http_stream_factory_impl_job_controller.cc b/net/http/http_stream_factory_impl_job_controller.cc
index 3978ac26d61d516a09a7fa29d4adda8ccc9ef642..37a44657c8f2367871998a21a7c8b8c3b9e2bf42 100644
--- a/net/http/http_stream_factory_impl_job_controller.cc
+++ b/net/http/http_stream_factory_impl_job_controller.cc
@@ -29,6 +29,22 @@
 
 namespace net {
 
+namespace {
+
+// Returns parameters associated with the proxy resolution.
+std::unique_ptr<base::Value> NetLogHttpStreamJobProxyServerResolved(
+    const ProxyServer& proxy_server,
+    NetLogCaptureMode /* capture_mode */) {
+  std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
+
+  dict->SetString("proxy_server", proxy_server.is_valid()
+                                      ? proxy_server.ToPacString()
+                                      : std::string());
+  return std::move(dict);
+}
+
+}  // namespace
+
 // The maximum time to wait for the alternate job to complete before resuming
 // the main job.
 const int kMaxDelayTimeForMainJobSecs = 3;
@@ -51,7 +67,9 @@ HttpStreamFactoryImpl::JobController::JobController(
     const HttpRequestInfo& request_info,
     bool is_preconnect,
     bool enable_ip_based_pooling,
-    bool enable_alternative_services)
+    bool enable_alternative_services,
+    const SSLConfig& server_ssl_config,
+    const SSLConfig& proxy_ssl_config)
     : factory_(factory),
       session_(session),
       job_factory_(job_factory),
@@ -65,8 +83,17 @@ HttpStreamFactoryImpl::JobController::JobController(
       main_job_is_blocked_(false),
       main_job_is_resumed_(false),
       bound_job_(nullptr),
-      can_start_alternative_proxy_job_(false),
+      can_start_alternative_proxy_job_(true),
       privacy_mode_(PRIVACY_MODE_DISABLED),
+      next_state_(STATE_RESOLVE_PROXY),
+      pac_request_(nullptr),
+      io_callback_(
+          base::Bind(&JobController::OnIOComplete, base::Unretained(this))),
+      request_info_(request_info),
+      server_ssl_config_(server_ssl_config),
+      proxy_ssl_config_(proxy_ssl_config),
+      num_streams_(0),
+      priority_(IDLE),
       net_log_(
           NetLogWithSource::Make(session->net_log(),
                                  NetLogSourceType::HTTP_STREAM_JOB_CONTROLLER)),
@@ -81,6 +108,8 @@ HttpStreamFactoryImpl::JobController::~JobController() {
   main_job_.reset();
   alternative_job_.reset();
   bound_job_ = nullptr;
+  if (pac_request_)
+    session_->proxy_service()->CancelPacRequest(pac_request_);
   net_log_.EndEvent(NetLogEventType::HTTP_STREAM_JOB_CONTROLLER);
 }
 
@@ -89,21 +118,20 @@ bool HttpStreamFactoryImpl::JobController::for_websockets() {
 }
 
 HttpStreamFactoryImpl::Request* HttpStreamFactoryImpl::JobController::Start(
-    const HttpRequestInfo& request_info,
     HttpStreamRequest::Delegate* delegate,
     WebSocketHandshakeStreamBase::CreateHelper*
         websocket_handshake_stream_create_helper,
     const NetLogWithSource& source_net_log,
     HttpStreamRequest::StreamType stream_type,
-    RequestPriority priority,
-    const SSLConfig& server_ssl_config,
-    const SSLConfig& proxy_ssl_config) {
+    RequestPriority priority) {
   DCHECK(factory_);
   DCHECK(!request_);
 
-  privacy_mode_ = request_info.privacy_mode;
+  privacy_mode_ = request_info_.privacy_mode;
+  stream_type_ = stream_type;
+  priority_ = priority;
 
-  request_ = new Request(request_info.url, this, delegate,
+  request_ = new Request(request_info_.url, this, delegate,
                          websocket_handshake_stream_create_helper,
                          source_net_log, stream_type);
   // Associates |net_log_| with |source_net_log|.
@@ -112,47 +140,35 @@ HttpStreamFactoryImpl::Request* HttpStreamFactoryImpl::JobController::Start(
   net_log_.AddEvent(NetLogEventType::HTTP_STREAM_JOB_CONTROLLER_BOUND,
                     source_net_log.source().ToEventParametersCallback());
 
-  CreateJobs(request_info, priority, server_ssl_config, proxy_ssl_config,
-             delegate, stream_type);
-
+  RunLoop(OK);
   return request_;
 }
 
-void HttpStreamFactoryImpl::JobController::Preconnect(
-    int num_streams,
-    const HttpRequestInfo& request_info,
-    const SSLConfig& server_ssl_config,
-    const SSLConfig& proxy_ssl_config) {
+void HttpStreamFactoryImpl::JobController::Preconnect(int num_streams) {
   DCHECK(!main_job_);
   DCHECK(!alternative_job_);
   DCHECK(is_preconnect_);
 
-  privacy_mode_ = request_info.privacy_mode;
+  privacy_mode_ = request_info_.privacy_mode;
+  num_streams_ = num_streams;
 
-  HostPortPair destination(HostPortPair::FromURL(request_info.url));
-  GURL origin_url = ApplyHostMappingRules(request_info.url, &destination);
+  HostPortPair destination(HostPortPair::FromURL(request_info_.url));
+  GURL origin_url = ApplyHostMappingRules(request_info_.url, &destination);
 
   const AlternativeService alternative_service = GetAlternativeServiceFor(
-      request_info, nullptr, HttpStreamRequest::HTTP_STREAM);
+      request_info_, nullptr, HttpStreamRequest::HTTP_STREAM);
 
   if (alternative_service.protocol != kProtoUnknown) {
     destination = alternative_service.host_port_pair();
-    ignore_result(ApplyHostMappingRules(request_info.url, &destination));
+    ignore_result(ApplyHostMappingRules(request_info_.url, &destination));
   }
-
-  // Due to how the socket pools handle priorities and idle sockets, only IDLE
-  // priority currently makes sense for preconnects. The priority for
-  // preconnects is currently ignored (see RequestSocketsForPool()), but could
-  // be used at some point for proxy resolution or something.
-  main_job_.reset(job_factory_->CreateJob(
-      this, PRECONNECT, session_, request_info, IDLE, server_ssl_config,
-      proxy_ssl_config, destination, origin_url, alternative_service,
-      enable_ip_based_pooling_, session_->net_log()));
-  main_job_->Preconnect(num_streams);
+  RunLoop(OK);
 }
 
 LoadState HttpStreamFactoryImpl::JobController::GetLoadState() const {
   DCHECK(request_);
+  if (next_state_ == STATE_RESOLVE_PROXY_COMPLETE)
+    return session_->proxy_service()->GetLoadState(pac_request_);
   DCHECK(main_job_ || alternative_job_);
   if (bound_job_)
     return bound_job_->GetLoadState();
@@ -277,7 +293,11 @@ void HttpStreamFactoryImpl::JobController::OnStreamFailed(
     const SSLConfig& used_ssl_config) {
   if (job->job_type() == ALTERNATIVE) {
     DCHECK_EQ(alternative_job_.get(), job);
-    OnAlternativeJobFailed(status);
+    if (alternative_job_->alternative_proxy_server().is_valid()) {
+      OnAlternativeProxyJobFailed(status);
+    } else {
+      OnAlternativeServiceJobFailed(status);
+    }
   }
 
   MaybeResumeMainJob(job, base::TimeDelta());
@@ -309,6 +329,11 @@ void HttpStreamFactoryImpl::JobController::OnStreamFailed(
     }
   }
 
+  status = ReconsiderProxyAfterError(job, status);
+  if (next_state_ == STATE_RESOLVE_PROXY_COMPLETE) {
+    RunLoop(OK);
+    return;
+  }
   request_->OnStreamFailed(status, used_ssl_config);
 }
 
@@ -402,45 +427,6 @@ bool HttpStreamFactoryImpl::JobController::OnInitConnection(
   return factory_->OnInitConnection(*this, proxy_info, privacy_mode_);
 }
 
-void HttpStreamFactoryImpl::JobController::OnResolveProxyComplete(
-    Job* job,
-    const HttpRequestInfo& request_info,
-    RequestPriority priority,
-    const SSLConfig& server_ssl_config,
-    const SSLConfig& proxy_ssl_config,
-    HttpStreamRequest::StreamType stream_type) {
-  DCHECK(job);
-
-  ProxyServer alternative_proxy_server;
-  if (!ShouldCreateAlternativeProxyServerJob(job, job->proxy_info(),
-                                             request_info.url,
-                                             &alternative_proxy_server)) {
-    return;
-  }
-
-  DCHECK(main_job_);
-  DCHECK_EQ(MAIN, job->job_type());
-  DCHECK(!alternative_job_);
-  DCHECK(!main_job_is_blocked_);
-
-  HostPortPair destination(HostPortPair::FromURL(request_info.url));
-  GURL origin_url = ApplyHostMappingRules(request_info.url, &destination);
-
-  alternative_job_.reset(job_factory_->CreateJob(
-      this, ALTERNATIVE, session_, request_info, priority, server_ssl_config,
-      proxy_ssl_config, destination, origin_url, alternative_proxy_server,
-      enable_ip_based_pooling_, job->net_log().net_log()));
-
-  can_start_alternative_proxy_job_ = false;
-  main_job_is_blocked_ = true;
-
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::Bind(
-          &HttpStreamFactoryImpl::JobController::StartAlternativeProxyServerJob,
-          ptr_factory_.GetWeakPtr()));
-}
-
 void HttpStreamFactoryImpl::JobController::OnNewSpdySessionReady(
     Job* job,
     const base::WeakPtr<SpdySession>& spdy_session,
@@ -689,52 +675,166 @@ WebSocketHandshakeStreamBase::CreateHelper* HttpStreamFactoryImpl::
   return request_->websocket_handshake_stream_create_helper();
 }
 
-void HttpStreamFactoryImpl::JobController::CreateJobs(
-    const HttpRequestInfo& request_info,
-    RequestPriority priority,
-    const SSLConfig& server_ssl_config,
-    const SSLConfig& proxy_ssl_config,
-    HttpStreamRequest::Delegate* delegate,
-    HttpStreamRequest::StreamType stream_type) {
+void HttpStreamFactoryImpl::JobController::OnIOComplete(int result) {
+  RunLoop(result);
+}
+
+void HttpStreamFactoryImpl::JobController::RunLoop(int result) {
+  int rv = DoLoop(result);
+  if (rv == ERR_IO_PENDING)
+    return;
+  if (rv != OK) {
+    DCHECK(!main_job_);
+    DCHECK(!alternative_job_);

[Zhongyi Shi, 2017/05/10 20:28:16]

Could you add some comments here? 

I think this is specifically handling the case where we fail before CREATE_JOBS,
i.e., in proxy resolution, job controller need to notify the request of the
failure. 

[xunjieli, 2017/05/10 23:24:41]

Done.

+
+    if (!request_)
+      return;
+    request_->OnStreamFailed(rv, server_ssl_config_);
+  }
+}
+
+int HttpStreamFactoryImpl::JobController::DoLoop(int rv) {
+  DCHECK_NE(next_state_, STATE_NONE);
+  do {
+    State state = next_state_;
+    next_state_ = STATE_NONE;
+    switch (state) {
+      case STATE_RESOLVE_PROXY:
+        DCHECK_EQ(OK, rv);
+        rv = DoResolveProxy();
+        break;
+      case STATE_RESOLVE_PROXY_COMPLETE:
+        rv = DoResolveProxyComplete(rv);
+        break;
+      case STATE_CREATE_JOBS:
+        DCHECK_EQ(OK, rv);
+        rv = DoCreateJobs();
+        break;
+      default:
+        NOTREACHED() << "bad state";
+        break;
+    }
+  } while (next_state_ != STATE_NONE && rv != ERR_IO_PENDING);
+  return rv;
+}
+
+int HttpStreamFactoryImpl::JobController::DoResolveProxy() {
+  DCHECK(!pac_request_);
+  DCHECK(session_);
+
+  next_state_ = STATE_RESOLVE_PROXY_COMPLETE;
+
+  if (request_info_.load_flags & LOAD_BYPASS_PROXY) {
+    proxy_info_.UseDirect();
+    return OK;
+  }
+
+  HostPortPair destination(HostPortPair::FromURL(request_info_.url));
+  GURL origin_url = ApplyHostMappingRules(request_info_.url, &destination);
+
+  return session_->proxy_service()->ResolveProxy(
+      origin_url, request_info_.method, &proxy_info_, io_callback_,
+      &pac_request_, session_->params().proxy_delegate, net_log_);
+}
+
+int HttpStreamFactoryImpl::JobController::DoResolveProxyComplete(int rv) {
+  DCHECK_NE(ERR_IO_PENDING, rv);
+
+  pac_request_ = nullptr;
+  net_log_.AddEvent(
+      NetLogEventType::HTTP_STREAM_JOB_PROXY_SERVER_RESOLVED,
+      base::Bind(
+          &NetLogHttpStreamJobProxyServerResolved,
+          proxy_info_.is_empty() ? ProxyServer() : proxy_info_.proxy_server()));
+
+  if (rv == OK)
+    next_state_ = STATE_CREATE_JOBS;
+  return rv;
+}
+
+int HttpStreamFactoryImpl::JobController::DoCreateJobs() {
   DCHECK(!main_job_);
   DCHECK(!alternative_job_);
-  HostPortPair destination(HostPortPair::FromURL(request_info.url));
-  GURL origin_url = ApplyHostMappingRules(request_info.url, &destination);
 
-  main_job_.reset(job_factory_->CreateJob(
-      this, MAIN, session_, request_info, priority, server_ssl_config,
-      proxy_ssl_config, destination, origin_url, enable_ip_based_pooling_,
-      net_log_.net_log()));
+  HostPortPair destination(HostPortPair::FromURL(request_info_.url));
+  GURL origin_url = ApplyHostMappingRules(request_info_.url, &destination);
 
   // Create an alternative job if alternative service is set up for this domain.
   const AlternativeService alternative_service =
-      GetAlternativeServiceFor(request_info, delegate, stream_type);
-
+      GetAlternativeServiceFor(request_info_, delegate_, stream_type_);
+
+  // Remove unsupported proxies from the list. ProxyServer::SCHEME_QUIC is not
+  // yet supported.
+  int supported_proxies = ProxyServer::SCHEME_DIRECT |
+                          ProxyServer::SCHEME_HTTP | ProxyServer::SCHEME_HTTPS |
+                          ProxyServer::SCHEME_SOCKS4 |
+                          ProxyServer::SCHEME_SOCKS5;
+
+  if (session_->IsQuicEnabled())
+    supported_proxies |= ProxyServer::SCHEME_QUIC;
+  proxy_info_.RemoveProxiesWithoutScheme(supported_proxies);

[Zhongyi Shi, 2017/05/10 20:28:16]

optional nit: this is trying to clean up proxies from the list, which should
happen in successful DoResolveProxyComplete. It might be better to move this up
to DoResolveProxyComplete(). 

[xunjieli, 2017/05/10 23:24:41]

Done.

+

[Zhongyi Shi, 2017/05/10 20:28:16]

Also, we used to check the proxy_info list is non-empty in
HttpStreamFactoryImpl::Job::DoResolveProxy().

if (proxy_info_.is_empty()) {	
  // No proxies/direct to choose from. This happens when we don't support	
  // any of the proxies in the returned list.	
  result = ERR_NO_SUPPORTED_PROXIES;	
}

In stead of delaying the check in
HttpStreamFactoryImpl::Job::DoInitConnectionImpl() as this patch does, how about
return the error in
HttpStreamFactoryImpl::JobController::DoResolveProxyComplete() and mark the
request fail immediately? Does this logic make sense?

+  if (is_preconnect_) {
+    // Due to how the socket pools handle priorities and idle sockets, only IDLE
+    // priority currently makes sense for preconnects. The priority for
+    // preconnects is currently ignored (see RequestSocketsForPool()), but could
+    // be used at some point for proxy resolution or something.
+    main_job_.reset(job_factory_->CreateAltJob(

[Zhongyi Shi, 2017/05/10 20:28:16]

Hrm, that suggests s/CreateAltJob/CreateAltSvcJob, WDYT?

[xunjieli, 2017/05/10 23:24:41]

Done.

+        this, PRECONNECT, session_, request_info_, IDLE, proxy_info_,
+        server_ssl_config_, proxy_ssl_config_, destination, origin_url,
+        alternative_service, enable_ip_based_pooling_, session_->net_log()));
+    main_job_->Preconnect(num_streams_);
+    return OK;
+  }
+  main_job_.reset(job_factory_->CreateMainJob(
+      this, MAIN, session_, request_info_, priority_, proxy_info_,
+      server_ssl_config_, proxy_ssl_config_, destination, origin_url,
+      enable_ip_based_pooling_, net_log_.net_log()));
+  // AltSvc can only be set for HTTPS requests while Alternative Proxy is set
+  // for HTTP requests.
   if (alternative_service.protocol != kProtoUnknown) {
     // Never share connection with other jobs for FTP requests.
     DVLOG(1) << "Selected alternative service (host: "
              << alternative_service.host_port_pair().host()
              << " port: " << alternative_service.host_port_pair().port() << ")";
 
-    DCHECK(!request_info.url.SchemeIs(url::kFtpScheme));
+    DCHECK(!request_info_.url.SchemeIs(url::kFtpScheme));
     HostPortPair alternative_destination(alternative_service.host_port_pair());
     ignore_result(
-        ApplyHostMappingRules(request_info.url, &alternative_destination));
+        ApplyHostMappingRules(request_info_.url, &alternative_destination));
 
-    alternative_job_.reset(job_factory_->CreateJob(
-        this, ALTERNATIVE, session_, request_info, priority, server_ssl_config,
-        proxy_ssl_config, alternative_destination, origin_url,
-        alternative_service, enable_ip_based_pooling_, net_log_.net_log()));
+    alternative_job_.reset(job_factory_->CreateAltJob(
+        this, ALTERNATIVE, session_, request_info_, priority_, proxy_info_,
+        server_ssl_config_, proxy_ssl_config_, alternative_destination,
+        origin_url, alternative_service, enable_ip_based_pooling_,
+        net_log_.net_log()));
 
     main_job_is_blocked_ = true;
     alternative_job_->Start(request_->stream_type());
   } else {
-    can_start_alternative_proxy_job_ = true;
+    ProxyServer alternative_proxy_server;
+    if (ShouldCreateAlternativeProxyServerJob(proxy_info_, request_info_.url,
+                                              &alternative_proxy_server)) {
+      DCHECK(!main_job_is_blocked_);
+      ProxyInfo alternative_proxy_info;
+      alternative_proxy_info.UseProxyServer(alternative_proxy_server);
+
+      alternative_job_.reset(job_factory_->CreateAltProxyJob(
+          this, ALTERNATIVE, session_, request_info_, priority_,
+          alternative_proxy_info, server_ssl_config_, proxy_ssl_config_,
+          destination, origin_url, alternative_proxy_server,
+          enable_ip_based_pooling_, net_log_.net_log()));
+
+      can_start_alternative_proxy_job_ = false;
+      main_job_is_blocked_ = true;
+      alternative_job_->Start(request_->stream_type());

[Zhongyi Shi, 2017/05/10 20:28:15]

Woohoo! This is much cleaner!

+    }
   }
   // Even if |alternative_job| has already finished, it will not have notified
   // the request yet, since we defer that to the next iteration of the
   // MessageLoop, so starting |main_job_| is always safe.
   main_job_->Start(request_->stream_type());
+  return OK;
 }
 
 void HttpStreamFactoryImpl::JobController::BindJob(Job* job) {
@@ -830,20 +930,14 @@ void HttpStreamFactoryImpl::JobController::MarkRequestComplete(
     request_->Complete(was_alpn_negotiated, negotiated_protocol, using_spdy);
 }
 
-void HttpStreamFactoryImpl::JobController::OnAlternativeJobFailed(
+void HttpStreamFactoryImpl::JobController::OnAlternativeServiceJobFailed(
     int net_error) {
   DCHECK_EQ(alternative_job_->job_type(), ALTERNATIVE);
   DCHECK_NE(OK, net_error);
+  DCHECK_NE(kProtoUnknown, alternative_job_->alternative_service().protocol);
 
   alternative_job_net_error_ = net_error;
-
-  if (alternative_job_->alternative_proxy_server().is_valid()) {
-    failed_alternative_proxy_server_ =
-        alternative_job_->alternative_proxy_server();
-  } else {
-    DCHECK(!failed_alternative_proxy_server_.is_valid());
-    failed_alternative_service_ = alternative_job_->alternative_service();
-  }
+  failed_alternative_service_ = alternative_job_->alternative_service();
 
   if (IsJobOrphaned(alternative_job_.get())) {
     // If |request_| is gone then it must have been successfully served by
@@ -854,33 +948,39 @@ void HttpStreamFactoryImpl::JobController::OnAlternativeJobFailed(
   }
 }
 
+void HttpStreamFactoryImpl::JobController::OnAlternativeProxyJobFailed(
+    int net_error) {
+  DCHECK_EQ(alternative_job_->job_type(), ALTERNATIVE);
+  DCHECK_NE(OK, net_error);
+  DCHECK(alternative_job_->alternative_proxy_server().is_valid());
+
+  // Need to mark alt proxy as broken regardless whether the job is bound.
+  ProxyDelegate* proxy_delegate = session_->params().proxy_delegate;
+  if (proxy_delegate) {
+    proxy_delegate->OnAlternativeProxyBroken(
+        alternative_job_->alternative_proxy_server());
+  }
+}
+
 void HttpStreamFactoryImpl::JobController::ReportBrokenAlternativeService() {
-  DCHECK(failed_alternative_service_.protocol != kProtoUnknown ||
-         failed_alternative_proxy_server_.is_valid());
+  DCHECK(failed_alternative_service_.protocol != kProtoUnknown);
   DCHECK_NE(OK, alternative_job_net_error_);
 
-  UMA_HISTOGRAM_SPARSE_SLOWLY("Net.AlternateServiceFailed",
-                              -alternative_job_net_error_);
+  int error_to_report = alternative_job_net_error_;
+  alternative_job_net_error_ = OK;
+  UMA_HISTOGRAM_SPARSE_SLOWLY("Net.AlternateServiceFailed", -error_to_report);
 
   if (session_->params().quic_do_not_mark_as_broken_on_network_change &&
-      (alternative_job_net_error_ == ERR_NETWORK_CHANGED ||
-       alternative_job_net_error_ == ERR_INTERNET_DISCONNECTED)) {
+      (error_to_report == ERR_NETWORK_CHANGED ||
+       error_to_report == ERR_INTERNET_DISCONNECTED)) {
     // No need to mark alternative service or proxy as broken.
     return;
   }
 
-  if (failed_alternative_proxy_server_.is_valid()) {
-    ProxyDelegate* proxy_delegate = session_->params().proxy_delegate;
-    if (proxy_delegate) {
-      proxy_delegate->OnAlternativeProxyBroken(
-          failed_alternative_proxy_server_);
-    }
-  } else {
-    HistogramBrokenAlternateProtocolLocation(
-        BROKEN_ALTERNATE_PROTOCOL_LOCATION_HTTP_STREAM_FACTORY_IMPL_JOB_ALT);
-    session_->http_server_properties()->MarkAlternativeServiceBroken(
-        failed_alternative_service_);
-  }
+  HistogramBrokenAlternateProtocolLocation(
+      BROKEN_ALTERNATE_PROTOCOL_LOCATION_HTTP_STREAM_FACTORY_IMPL_JOB_ALT);
+  session_->http_server_properties()->MarkAlternativeServiceBroken(
+      failed_alternative_service_);
 }
 
 void HttpStreamFactoryImpl::JobController::MaybeNotifyFactoryOfCompletion() {
@@ -1033,7 +1133,6 @@ HttpStreamFactoryImpl::JobController::GetAlternativeServiceForInternal(
 
 bool HttpStreamFactoryImpl::JobController::
     ShouldCreateAlternativeProxyServerJob(
-        Job* job,
         const ProxyInfo& proxy_info,
         const GURL& url,
         ProxyServer* alternative_proxy_server) const {
@@ -1048,18 +1147,6 @@ bool HttpStreamFactoryImpl::JobController::
     return false;
   }
 
-  if (job->job_type() == ALTERNATIVE) {
-    // If |job| is using alternative service, then alternative proxy server
-    // should not be used.
-    return false;
-  }
-
-  if (is_preconnect_ || job->job_type() == PRECONNECT) {
-    // Preconnects should be fetched using only the main job to keep the
-    // resource utilization down.
-    return false;
-  }
-
   if (proxy_info.is_empty() || proxy_info.is_direct() || proxy_info.is_quic()) {
     // Alternative proxy server job can be created only if |job| fetches the
     // |request_| through a non-QUIC proxy.
@@ -1076,7 +1163,6 @@ bool HttpStreamFactoryImpl::JobController::
   ProxyDelegate* proxy_delegate = session_->params().proxy_delegate;
   if (!proxy_delegate)
     return false;
-
   proxy_delegate->GetAlternativeProxy(url, proxy_info.proxy_server(),
                                       alternative_proxy_server);
 
@@ -1124,15 +1210,51 @@ void HttpStreamFactoryImpl::JobController::ReportAlternateProtocolUsage(
                                   proxy_server_used);
 }
 
-void HttpStreamFactoryImpl::JobController::StartAlternativeProxyServerJob() {
-  if (!alternative_job_ || !request_)
-    return;
-  DCHECK(alternative_job_->alternative_proxy_server().is_valid());
-  alternative_job_->Start(request_->stream_type());
-}
-
 bool HttpStreamFactoryImpl::JobController::IsJobOrphaned(Job* job) const {
   return !request_ || (job_bound_ && bound_job_ != job);
 }
 
+int HttpStreamFactoryImpl::JobController::ReconsiderProxyAfterError(Job* job,
+                                                                    int error) {
+  DCHECK(!pac_request_);
+  DCHECK(session_);
+
+  if (!job->should_reconsider_proxy())
+    return error;
+
+  DCHECK(!job->alternative_proxy_server().is_valid());
+
+  // Do not bypass non-QUIC proxy on ERR_MSG_TOO_BIG.
+  if (!proxy_info_.is_quic() && error == ERR_MSG_TOO_BIG)
+    return error;
+
+  if (request_info_.load_flags & LOAD_BYPASS_PROXY)
+    return error;
+
+  if (proxy_info_.is_https() && proxy_ssl_config_.send_client_cert) {
+    session_->ssl_client_auth_cache()->Remove(
+        proxy_info_.proxy_server().host_port_pair());
+  }
+
+  int rv = session_->proxy_service()->ReconsiderProxyAfterError(
+      request_info_.url, request_info_.method, error, &proxy_info_,
+      io_callback_, &pac_request_, session_->params().proxy_delegate, net_log_);
+  if (rv == OK || rv == ERR_IO_PENDING) {
+    RemoveRequestFromSpdySessionRequestMap();
+    // Abandon all Jobs and start over.
+    job_bound_ = false;
+    bound_job_ = nullptr;
+    alternative_job_.reset();
+    main_job_.reset();
+    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;
+}
+
 }  // namespace net