Revert "Revert CLs landed in HttpStreamFactoryImpl to track down a crasher"

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 b23470229c3543532a51f1e722091ef68a865467..81413bafee5a828894c8b9f6dd8adba4e67912a8 100644
--- a/net/http/http_stream_factory_impl_job_controller.cc
+++ b/net/http/http_stream_factory_impl_job_controller.cc
@@ -28,6 +28,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;
@@ -50,7 +66,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),
@@ -64,8 +82,16 @@ HttpStreamFactoryImpl::JobController::JobController(
       main_job_is_blocked_(false),
       main_job_is_resumed_(false),
       bound_job_(nullptr),
-      can_start_alternative_proxy_job_(false),
-      privacy_mode_(PRIVACY_MODE_DISABLED),
+      can_start_alternative_proxy_job_(true),
+      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)),
@@ -80,6 +106,12 @@ HttpStreamFactoryImpl::JobController::~JobController() {
   main_job_.reset();
   alternative_job_.reset();
   bound_job_ = nullptr;
+  if (pac_request_) {
+    // TODO(mmenke):  Convert this to a DCHECK once https://crbug.com/723589 is
+    // resolved.
+    CHECK_EQ(STATE_RESOLVE_PROXY_COMPLETE, next_state_);

[mmenke, 2017/05/26 17:28:04]

Think can change this back to a DCHECK, and remove the TODO

[xunjieli, 2017/05/26 17:35:15]

Why? I thought we haven't figured out the root cause of the use-after-free
(ProxyInfo::Reset() problem). Should we keep the CHECKs for now?

+    session_->proxy_service()->CancelPacRequest(pac_request_);
+  }
   net_log_.EndEvent(NetLogEventType::HTTP_STREAM_JOB_CONTROLLER);
 }
 
@@ -87,80 +119,60 @@ bool HttpStreamFactoryImpl::JobController::for_websockets() {
   return factory_->for_websockets_;
 }
 
-HttpStreamFactoryImpl::Request* HttpStreamFactoryImpl::JobController::Start(
-    const HttpRequestInfo& request_info,
+std::unique_ptr<HttpStreamFactoryImpl::Request>
+HttpStreamFactoryImpl::JobController::Start(
     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;
+  stream_type_ = stream_type;
+  priority_ = priority;
+
+  auto request = base::MakeUnique<Request>(
+      request_info_.url, this, delegate,
+      websocket_handshake_stream_create_helper, source_net_log, stream_type);
+  // Keep a raw pointer but release ownership of Request instance.
+  request_ = request.get();
 
-  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|.
   source_net_log.AddEvent(NetLogEventType::HTTP_STREAM_JOB_CONTROLLER_BOUND,
                           net_log_.source().ToEventParametersCallback());
   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);
-
-  return request_;
+  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;
+  stream_type_ = HttpStreamRequest::HTTP_STREAM;
+  num_streams_ = num_streams;
 
-  HostPortPair destination(HostPortPair::FromURL(request_info.url));
-  GURL origin_url = ApplyHostMappingRules(request_info.url, &destination);
-
-  const AlternativeService alternative_service =
-      GetAlternativeServiceInfoFor(request_info, nullptr,
-                                   HttpStreamRequest::HTTP_STREAM)
-          .alternative_service;
-
-  if (alternative_service.protocol != kProtoUnknown) {
-    destination = alternative_service.host_port_pair();
-    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_);
-  DCHECK(main_job_ || alternative_job_);
+  if (next_state_ == STATE_RESOLVE_PROXY_COMPLETE)
+    return session_->proxy_service()->GetLoadState(pac_request_);
   if (bound_job_)
     return bound_job_->GetLoadState();
-
-  // Just pick the first one.
-  return main_job_ ? main_job_->GetLoadState()
-                   : alternative_job_->GetLoadState();
+  if (main_job_)
+    return main_job_->GetLoadState();
+  if (alternative_job_)
+    return alternative_job_->GetLoadState();
+  // When proxy resolution fails, there is no job created and
+  // NotifyRequestFailed() is executed one message loop iteration later.
+  return LOAD_STATE_IDLE;
 }
 
 void HttpStreamFactoryImpl::JobController::OnRequestComplete() {
@@ -203,7 +215,7 @@ void HttpStreamFactoryImpl::JobController::OnStreamReady(
     const SSLConfig& used_ssl_config) {
   DCHECK(job);
 
-  factory_->OnStreamReady(job->proxy_info(), privacy_mode_);
+  factory_->OnStreamReady(job->proxy_info(), request_info_.privacy_mode);
 
   if (IsJobOrphaned(job)) {
     // We have bound a job to the associated Request, |job| has been orphaned.
@@ -278,7 +290,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());
@@ -310,6 +326,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);
 }
 
@@ -400,46 +421,8 @@ void HttpStreamFactoryImpl::JobController::OnNeedsProxyAuth(
 
 bool HttpStreamFactoryImpl::JobController::OnInitConnection(
     const ProxyInfo& proxy_info) {
-  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()));
+  return factory_->OnInitConnection(*this, proxy_info,
+                                    request_info_.privacy_mode);
 }
 
 void HttpStreamFactoryImpl::JobController::OnNewSpdySessionReady(
@@ -688,53 +671,188 @@ 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) {
+    // DoLoop can only fail during proxy resolution step which happens before
+    // any jobs are created. Notify |request_| of the failure one message loop
+    // iteration later to avoid re-entrancy.
+    DCHECK(!main_job_);
+    DCHECK(!alternative_job_);
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(&HttpStreamFactoryImpl::JobController::NotifyRequestFailed,
+                   ptr_factory_.GetWeakPtr(), rv));
+  }
+}
+
+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() {
+  // TODO(mmenke):  Convert this to a DCHECK once https://crbug.com/723589 is
+  // resolved.
+  CHECK(!pac_request_);

[mmenke, 2017/05/26 17:28:04]

DCHECK, remove TODO.

+  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)
+    return rv;
+  // Remove unsupported proxies from the list.
+  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);
+
+  if (proxy_info_.is_empty()) {
+    // No proxies/direct to choose from.
+    return ERR_NO_SUPPORTED_PROXIES;
+  }
+
+  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 =
-      GetAlternativeServiceInfoFor(request_info, delegate, stream_type)
+      GetAlternativeServiceInfoFor(request_info_, delegate_, stream_type_)
           .alternative_service;
 
+  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.
+    if (alternative_service.protocol != kProtoUnknown) {
+      HostPortPair alternative_destination(
+          alternative_service.host_port_pair());
+      ignore_result(
+          ApplyHostMappingRules(request_info_.url, &alternative_destination));
+      main_job_ = job_factory_->CreateAltSvcJob(
+          this, PRECONNECT, session_, request_info_, IDLE, proxy_info_,
+          server_ssl_config_, proxy_ssl_config_, alternative_destination,
+          origin_url, alternative_service, enable_ip_based_pooling_,
+          session_->net_log());
+    } else {
+      main_job_ = job_factory_->CreateMainJob(
+          this, PRECONNECT, session_, request_info_, IDLE, proxy_info_,
+          server_ssl_config_, proxy_ssl_config_, destination, origin_url,
+          enable_ip_based_pooling_, session_->net_log());
+    }
+    main_job_->Preconnect(num_streams_);
+    return OK;
+  }
+  main_job_ = 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());
+  // Alternative Service 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_ = job_factory_->CreateAltSvcJob(
+        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_ = 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());
+    }
   }
   // 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 +948,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,32 +966,38 @@ 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 (alternative_job_net_error_ == ERR_NETWORK_CHANGED ||
-      alternative_job_net_error_ == ERR_INTERNET_DISCONNECTED) {
+  if (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() {
@@ -889,6 +1007,12 @@ void HttpStreamFactoryImpl::JobController::MaybeNotifyFactoryOfCompletion() {
   }
 }
 
+void HttpStreamFactoryImpl::JobController::NotifyRequestFailed(int rv) {
+  if (!request_)
+    return;
+  request_->OnStreamFailed(rv, server_ssl_config_);
+}
+
 GURL HttpStreamFactoryImpl::JobController::ApplyHostMappingRules(
     const GURL& url,
     HostPortPair* endpoint) {
@@ -1042,7 +1166,6 @@ HttpStreamFactoryImpl::JobController::GetAlternativeServiceInfoInternal(
 
 bool HttpStreamFactoryImpl::JobController::
     ShouldCreateAlternativeProxyServerJob(
-        Job* job,
         const ProxyInfo& proxy_info,
         const GURL& url,
         ProxyServer* alternative_proxy_server) const {
@@ -1057,18 +1180,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.
@@ -1085,7 +1196,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);
 
@@ -1133,15 +1243,58 @@ 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) {
+  // ReconsiderProxyAfterError() should only be called when the last job fails.
+  DCHECK(!(alternative_job_ && main_job_));
+  // TODO(mmenke):  Convert this to a DCHECK once https://crbug.com/723589 is
+  // resolved.
+  CHECK(!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());
+  }
+
+  HostPortPair destination(HostPortPair::FromURL(request_info_.url));
+  GURL origin_url = ApplyHostMappingRules(request_info_.url, &destination);
+
+  int rv = session_->proxy_service()->ReconsiderProxyAfterError(
+      origin_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