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 ba5896a8794695fd9f575b3623af122e55cfe00c..1e7ea76ade0fcbbf8bf9106caa33ab44ac0352c3 100644
--- a/net/http/http_stream_factory_impl_job_controller.cc
+++ b/net/http/http_stream_factory_impl_job_controller.cc
@@ -28,22 +28,6 @@
 
 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;
@@ -66,9 +50,7 @@ HttpStreamFactoryImpl::JobController::JobController(
     const HttpRequestInfo& request_info,
     bool is_preconnect,
     bool enable_ip_based_pooling,
-    bool enable_alternative_services,
-    const SSLConfig& server_ssl_config,
-    const SSLConfig& proxy_ssl_config)
+    bool enable_alternative_services)
     : factory_(factory),
       session_(session),
       job_factory_(job_factory),
@@ -82,16 +64,8 @@ HttpStreamFactoryImpl::JobController::JobController(
       main_job_is_blocked_(false),
       main_job_is_resumed_(false),
       bound_job_(nullptr),
-      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),
+      can_start_alternative_proxy_job_(false),
+      privacy_mode_(PRIVACY_MODE_DISABLED),
       net_log_(
           NetLogWithSource::Make(session->net_log(),
                                  NetLogSourceType::HTTP_STREAM_JOB_CONTROLLER)),
@@ -106,12 +80,6 @@ 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_);
-    session_->proxy_service()->CancelPacRequest(pac_request_);
-  }
   net_log_.EndEvent(NetLogEventType::HTTP_STREAM_JOB_CONTROLLER);
 }
 
@@ -119,60 +87,80 @@ bool HttpStreamFactoryImpl::JobController::for_websockets() {
   return factory_->for_websockets_;
 }
 
-std::unique_ptr<HttpStreamFactoryImpl::Request>
-HttpStreamFactoryImpl::JobController::Start(
+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) {
+    RequestPriority priority,
+    const SSLConfig& server_ssl_config,
+    const SSLConfig& proxy_ssl_config) {
   DCHECK(factory_);
   DCHECK(!request_);
 
-  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();
+  privacy_mode_ = request_info.privacy_mode;
 
+  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());
 
-  RunLoop(OK);
-  return request;
+  CreateJobs(request_info, priority, server_ssl_config, proxy_ssl_config,
+             delegate, stream_type);
+
+  return request_;
 }
 
-void HttpStreamFactoryImpl::JobController::Preconnect(int num_streams) {
+void HttpStreamFactoryImpl::JobController::Preconnect(
+    int num_streams,
+    const HttpRequestInfo& request_info,
+    const SSLConfig& server_ssl_config,
+    const SSLConfig& proxy_ssl_config) {
   DCHECK(!main_job_);
   DCHECK(!alternative_job_);
   DCHECK(is_preconnect_);
 
-  stream_type_ = HttpStreamRequest::HTTP_STREAM;
-  num_streams_ = num_streams;
+  privacy_mode_ = request_info.privacy_mode;
 
-  RunLoop(OK);
+  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);
 }
 
 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();
-  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;
+
+  // Just pick the first one.
+  return main_job_ ? main_job_->GetLoadState()
+                   : alternative_job_->GetLoadState();
 }
 
 void HttpStreamFactoryImpl::JobController::OnRequestComplete() {
@@ -215,7 +203,7 @@ void HttpStreamFactoryImpl::JobController::OnStreamReady(
     const SSLConfig& used_ssl_config) {
   DCHECK(job);
 
-  factory_->OnStreamReady(job->proxy_info(), request_info_.privacy_mode);
+  factory_->OnStreamReady(job->proxy_info(), privacy_mode_);
 
   if (IsJobOrphaned(job)) {
     // We have bound a job to the associated Request, |job| has been orphaned.
@@ -290,11 +278,7 @@ void HttpStreamFactoryImpl::JobController::OnStreamFailed(
     const SSLConfig& used_ssl_config) {
   if (job->job_type() == ALTERNATIVE) {
     DCHECK_EQ(alternative_job_.get(), job);
-    if (alternative_job_->alternative_proxy_server().is_valid()) {
-      OnAlternativeProxyJobFailed(status);
-    } else {
-      OnAlternativeServiceJobFailed(status);
-    }
+    OnAlternativeJobFailed(status);
   }
 
   MaybeResumeMainJob(job, base::TimeDelta());
@@ -326,11 +310,6 @@ void HttpStreamFactoryImpl::JobController::OnStreamFailed(
     }
   }
 
-  status = ReconsiderProxyAfterError(job, status);
-  if (next_state_ == STATE_RESOLVE_PROXY_COMPLETE) {
-    RunLoop(OK);
-    return;
-  }
   request_->OnStreamFailed(status, used_ssl_config);
 }
 
@@ -421,8 +400,46 @@ void HttpStreamFactoryImpl::JobController::OnNeedsProxyAuth(
 
 bool HttpStreamFactoryImpl::JobController::OnInitConnection(
     const ProxyInfo& proxy_info) {
-  return factory_->OnInitConnection(*this, proxy_info,
-                                    request_info_.privacy_mode);
+  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(
@@ -673,176 +690,53 @@ WebSocketHandshakeStreamBase::CreateHelper* HttpStreamFactoryImpl::
   return request_->websocket_handshake_stream_create_helper();
 }
 
-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_);
-  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() {
+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) {
   DCHECK(!main_job_);
   DCHECK(!alternative_job_);
+  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);
+  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()));
 
   // 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.
-    main_job_ = job_factory_->CreateAltSvcJob(

[Zhongyi Shi, 2017/05/22 19:05:39]

FYI, Ryan and I were just looking at the code today, and we found the original
proxy resolution CL might cause a regression bug here to create a AltSvc Job for
main_job directly in the preconnect case.

In fact, we should check whether the alternative_service is valid, if not,
create a TCP job. Else, set destination from the alternative_service and
ApplyHostMappingRules, then create a alt svc job. 

[xunjieli, 2017/05/22 19:15:00]

Acknowledged. Good catch! Will do this when relanding the patch. Thanks, Cherie.

-        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_ = 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_ = 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());
+    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()));
 
     main_job_is_blocked_ = true;
     alternative_job_->Start(request_->stream_type());
   } else {
-    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());
-    }
+    can_start_alternative_proxy_job_ = true;
   }
   // 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) {
@@ -938,14 +832,20 @@ void HttpStreamFactoryImpl::JobController::MarkRequestComplete(
     request_->Complete(was_alpn_negotiated, negotiated_protocol, using_spdy);
 }
 
-void HttpStreamFactoryImpl::JobController::OnAlternativeServiceJobFailed(
+void HttpStreamFactoryImpl::JobController::OnAlternativeJobFailed(
     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;
-  failed_alternative_service_ = alternative_job_->alternative_service();
+
+  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();
+  }
 
   if (IsJobOrphaned(alternative_job_.get())) {
     // If |request_| is gone then it must have been successfully served by
@@ -956,38 +856,32 @@ void HttpStreamFactoryImpl::JobController::OnAlternativeServiceJobFailed(
   }
 }
 
-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);
+  DCHECK(failed_alternative_service_.protocol != kProtoUnknown ||
+         failed_alternative_proxy_server_.is_valid());
   DCHECK_NE(OK, 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);
+  UMA_HISTOGRAM_SPARSE_SLOWLY("Net.AlternateServiceFailed",
+                              -alternative_job_net_error_);
 
-  if (error_to_report == ERR_NETWORK_CHANGED ||
-      error_to_report == ERR_INTERNET_DISCONNECTED) {
+  if (alternative_job_net_error_ == ERR_NETWORK_CHANGED ||
+      alternative_job_net_error_ == ERR_INTERNET_DISCONNECTED) {
     // No need to mark alternative service or proxy as broken.
     return;
   }
 
-  HistogramBrokenAlternateProtocolLocation(
-      BROKEN_ALTERNATE_PROTOCOL_LOCATION_HTTP_STREAM_FACTORY_IMPL_JOB_ALT);
-  session_->http_server_properties()->MarkAlternativeServiceBroken(
-      failed_alternative_service_);
+  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_);
+  }
 }
 
 void HttpStreamFactoryImpl::JobController::MaybeNotifyFactoryOfCompletion() {
@@ -997,12 +891,6 @@ 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) {
@@ -1156,6 +1044,7 @@ HttpStreamFactoryImpl::JobController::GetAlternativeServiceInfoInternal(
 
 bool HttpStreamFactoryImpl::JobController::
     ShouldCreateAlternativeProxyServerJob(
+        Job* job,
         const ProxyInfo& proxy_info,
         const GURL& url,
         ProxyServer* alternative_proxy_server) const {
@@ -1170,6 +1059,18 @@ 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.
@@ -1186,6 +1087,7 @@ 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);
 
@@ -1233,58 +1135,15 @@ void HttpStreamFactoryImpl::JobController::ReportAlternateProtocolUsage(
                                   proxy_server_used);
 }
 
-bool HttpStreamFactoryImpl::JobController::IsJobOrphaned(Job* job) const {
-  return !request_ || (job_bound_ && bound_job_ != job);
+void HttpStreamFactoryImpl::JobController::StartAlternativeProxyServerJob() {
+  if (!alternative_job_ || !request_)
+    return;
+  DCHECK(alternative_job_->alternative_proxy_server().is_valid());
+  alternative_job_->Start(request_->stream_type());
 }
 
-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;
+bool HttpStreamFactoryImpl::JobController::IsJobOrphaned(Job* job) const {
+  return !request_ || (job_bound_ && bound_job_ != job);
 }
 
 }  // namespace net