Extract Proxy Resolution out of HttpStreamFactoryImpl::Job

net/http/http_stream_factory_impl_job_controller.cc

 // Copyright (c) 2016 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_controller.h"
 
 #include <memory>
 #include <string>
 #include <utility>
 
@@ skipping 11 matching lines @@
 #include "net/log/net_log_capture_mode.h"
 #include "net/log/net_log_event_type.h"
 #include "net/log/net_log_source.h"
 #include "net/log/net_log_with_source.h"
 #include "net/proxy/proxy_server.h"
 #include "net/spdy/chromium/spdy_session.h"
 #include "url/url_constants.h"
 
 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;
 
 std::unique_ptr<base::Value> NetLogJobControllerCallback(
     const GURL* url,
     bool is_preconnect,
     NetLogCaptureMode /* capture_mode */) {
   auto dict = base::MakeUnique<base::DictionaryValue>();
   dict->SetString("url", url->possibly_invalid_spec());
   dict->SetBoolean("is_preconnect", is_preconnect);
   return std::move(dict);
 }
 
 HttpStreamFactoryImpl::JobController::JobController(
     HttpStreamFactoryImpl* factory,
     HttpStreamRequest::Delegate* delegate,
     HttpNetworkSession* session,
     JobFactory* job_factory,
     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),
       request_(nullptr),
       delegate_(delegate),
       is_preconnect_(is_preconnect),
       enable_ip_based_pooling_(enable_ip_based_pooling),
       enable_alternative_services_(enable_alternative_services),
       alternative_job_net_error_(OK),
       job_bound_(false),
       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),
       net_log_(
           NetLogWithSource::Make(session->net_log(),
                                  NetLogSourceType::HTTP_STREAM_JOB_CONTROLLER)),
+      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),
       ptr_factory_(this) {
   DCHECK(factory);
   net_log_.BeginEvent(NetLogEventType::HTTP_STREAM_JOB_CONTROLLER,
                       base::Bind(&NetLogJobControllerCallback,
                                  &request_info.url, is_preconnect));
 }
 
 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);
 }
 
 bool HttpStreamFactoryImpl::JobController::for_websockets() {
   return factory_->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|.
   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);
-
+  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 {
+  // FIXME
+  if (next_state_ == STATE_RESOLVE_PROXY_COMPLETE)
+    return session_->proxy_service()->GetLoadState(pac_request_);
   DCHECK(request_);
   DCHECK(main_job_ || alternative_job_);
   if (bound_job_)
     return bound_job_->GetLoadState();
 
   // Just pick the first one.
   return main_job_ ? main_job_->GetLoadState()
                    : alternative_job_->GetLoadState();
 }
 
@@ skipping 15 matching lines @@
     bound_job_ = nullptr;
   }
   MaybeNotifyFactoryOfCompletion();
 }
 
 int HttpStreamFactoryImpl::JobController::RestartTunnelWithProxyAuth() {
   DCHECK(bound_job_);
   return bound_job_->RestartTunnelWithProxyAuth();
 }
 
+int HttpStreamFactoryImpl::JobController::ReconsiderProxyAfterError(Job* job,
+                                                                    int error) {
+  DCHECK(!pac_request_);
+  DCHECK(session_);
+
+  if (!job->reconsider_proxy())
+    return error;
+
+  // 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;
+}
+
 void HttpStreamFactoryImpl::JobController::SetPriority(
     RequestPriority priority) {
   if (main_job_) {
     main_job_->SetPriority(priority);
   }
   if (alternative_job_) {
     alternative_job_->SetPriority(priority);
   }
 }
 
@@ skipping 71 matching lines @@
                                             stream);
 }
 
 void HttpStreamFactoryImpl::JobController::OnStreamFailed(
     Job* job,
     int status,
     const SSLConfig& used_ssl_config) {
   if (job->job_type() == ALTERNATIVE) {
     DCHECK_EQ(alternative_job_.get(), job);
     OnAlternativeJobFailed(status);
+    OnAlternativeProxyJobFailed(status);
   }
 
   MaybeResumeMainJob(job, base::TimeDelta());
 
   if (IsJobOrphaned(job)) {
     // We have bound a job to the associated Request, |job| has been orphaned.
     OnOrphanedJobComplete(job);
     return;
   }
 
@@ skipping 11 matching lines @@
       } else {
         DCHECK(job->job_type() == ALTERNATIVE);
         alternative_job_.reset();
       }
       return;
     } else {
       BindJob(job);
     }
   }
 
+  status = ReconsiderProxyAfterError(job, status);
+  if (next_state_ == STATE_RESOLVE_PROXY_COMPLETE) {
+    RunLoop(OK);
+    return;
+  }
   request_->OnStreamFailed(status, used_ssl_config);
 }
 
 void HttpStreamFactoryImpl::JobController::OnCertificateError(
     Job* job,
     int status,
     const SSLConfig& used_ssl_config,
     const SSLInfo& ssl_info) {
   MaybeResumeMainJob(job, base::TimeDelta());
 
@@ skipping 73 matching lines @@
     BindJob(job);
   request_->OnNeedsProxyAuth(proxy_response, used_ssl_config, used_proxy_info,
                              auth_controller);
 }
 
 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()));
-}
-
 void HttpStreamFactoryImpl::JobController::OnNewSpdySessionReady(
     Job* job,
     const base::WeakPtr<SpdySession>& spdy_session,
     bool direct) {
   DCHECK(job);
   DCHECK(job->using_spdy());
   DCHECK(!is_preconnect_);
 
   bool is_job_orphaned = IsJobOrphaned(job);
 
@@ skipping 228 matching lines @@
   return base::trace_event::EstimateMemoryUsage(main_job_) +
          base::trace_event::EstimateMemoryUsage(alternative_job_);
 }
 
 WebSocketHandshakeStreamBase::CreateHelper* HttpStreamFactoryImpl::
     JobController::websocket_handshake_stream_create_helper() {
   DCHECK(request_);
   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::OnResolveProxyError(int error) {
   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()));
+  if (!request_)
+    return;
+  request_->OnStreamFailed(error, server_ssl_config_);
+}
+
+void HttpStreamFactoryImpl::JobController::RunLoop(int result) {
+  DCHECK_NE(next_state_, STATE_NONE);
+  do {
+    State state = next_state_;
+    next_state_ = STATE_NONE;
+    switch (state) {
+      case STATE_RESOLVE_PROXY:
+        DoResolveProxy();
+        break;
+      case STATE_RESOLVE_PROXY_COMPLETE:
+        DoResolveProxyComplete(result);
+        break;
+      case STATE_CREATE_JOBS:
+        DoCreateJobs();
+        break;
+      default:
+        NOTREACHED() << "bad state";
+        break;
+    }
+  } while (next_state_ != STATE_NONE);

[Bence, 2017/04/26 13:55:39]

Do you not need to exit the loop if ResolveProxy returns ERR_IO_PENDING?

[xunjieli, 2017/04/27 18:15:54]

Done. Good catch. I have added a test case for this.

+}
+
+void 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;
+  }
+
+  HostPortPair destination(HostPortPair::FromURL(request_info_.url));
+  GURL origin_url = ApplyHostMappingRules(request_info_.url, &destination);
+
+  int rv = session_->proxy_service()->ResolveProxy(
+      origin_url, request_info_.method, &proxy_info_, io_callback_,
+      &pac_request_, session_->params().proxy_delegate, net_log_);
+  if (rv != OK && rv != ERR_IO_PENDING) {
+    // Proxy resolution failed synchronously.
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::Bind(&HttpStreamFactoryImpl::JobController::OnResolveProxyError,
+                   ptr_factory_.GetWeakPtr(), rv));
+  }
+}
+
+void 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) {
+    OnResolveProxyError(rv);
+    return;
+  }
+
+  next_state_ = STATE_CREATE_JOBS;
+  return;
+}
+
+void HttpStreamFactoryImpl::JobController::DoCreateJobs() {
+  DCHECK(!main_job_);
+  DCHECK(!alternative_job_);
+
+  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_);
 
-  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() << ")";
+  // 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;
 
-    DCHECK(!request_info.url.SchemeIs(url::kFtpScheme));
-    HostPortPair alternative_destination(alternative_service.host_port_pair());
-    ignore_result(
-        ApplyHostMappingRules(request_info.url, &alternative_destination));
+  if (session_->IsQuicEnabled())
+    supported_proxies |= ProxyServer::SCHEME_QUIC;
+  proxy_info_.RemoveProxiesWithoutScheme(supported_proxies);
 
-    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()));
+  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_->CreateJob(
+        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_);
+  } else {
+    main_job_.reset(job_factory_->CreateJob(
+        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()));
+    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()
+               << ")";
 
-    main_job_is_blocked_ = true;
-    alternative_job_->Start(request_->stream_type());
-  } else {
-    can_start_alternative_proxy_job_ = true;
+      DCHECK(!request_info_.url.SchemeIs(url::kFtpScheme));
+      HostPortPair alternative_destination(
+          alternative_service.host_port_pair());
+      ignore_result(
+          ApplyHostMappingRules(request_info_.url, &alternative_destination));
+
+      alternative_job_.reset(job_factory_->CreateJob(
+          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());
+    }
+    if (!alternative_job_) {
+      ProxyServer alternative_proxy_server;
+      if (ShouldCreateAlternativeProxyServerJob(proxy_info_, request_info_.url,
+                                                &alternative_proxy_server)) {
+        DCHECK(main_job_);
+        DCHECK(!main_job_is_blocked_);
+        ProxyInfo alternative_proxy_info;
+        alternative_proxy_info.UseProxyServer(alternative_proxy_server);
+
+        alternative_job_.reset(job_factory_->CreateJob(
+            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;
+
+        base::ThreadTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE, base::Bind(&HttpStreamFactoryImpl::JobController::
+                                      StartAlternativeProxyServerJob,
+                                  ptr_factory_.GetWeakPtr()));
+      }
+    }
+    // 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());
   }
-  // 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;
 }
 
 void HttpStreamFactoryImpl::JobController::BindJob(Job* job) {
   DCHECK(request_);
   DCHECK(job);
   DCHECK(job == alternative_job_.get() || job == main_job_.get());
   DCHECK(!job_bound_);
   DCHECK(!bound_job_);
 
   job_bound_ = true;
@@ skipping 80 matching lines @@
     bool using_spdy) {
   if (request_)
     request_->Complete(was_alpn_negotiated, negotiated_protocol, using_spdy);
 }
 
 void HttpStreamFactoryImpl::JobController::OnAlternativeJobFailed(
     int net_error) {
   DCHECK_EQ(alternative_job_->job_type(), ALTERNATIVE);
   DCHECK_NE(OK, net_error);
 
+  DCHECK(alternative_job_->alternative_service().protocol != kProtoUnknown ||
+         alternative_job_->alternative_proxy_server().is_valid());
+  if (alternative_job_->alternative_proxy_server().is_valid())
+    return;
+
   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
     // |main_job_|.
     // If |request_| is bound to a different job, then it is being
     // successfully serverd by the main job.
     ReportBrokenAlternativeService();
   }
 }
 
+void HttpStreamFactoryImpl::JobController::OnAlternativeProxyJobFailed(
+    int net_error) {
+  DCHECK_EQ(alternative_job_->job_type(), ALTERNATIVE);
+  DCHECK_NE(OK, net_error);
+
+  if (!alternative_job_->alternative_proxy_server().is_valid())
+    return;
+  // 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() {
   if (!request_ && !main_job_ && !alternative_job_) {
     DCHECK(!bound_job_);
     factory_->OnJobControllerComplete(this);
   }
 }
 
 GURL HttpStreamFactoryImpl::JobController::ApplyHostMappingRules(
@@ skipping 132 matching lines @@
 
   // Ask delegate to mark QUIC as broken for the origin.
   if (quic_advertised && quic_all_broken && delegate != nullptr)
     delegate->OnQuicBroken();
 
   return first_alternative_service;
 }
 
 bool HttpStreamFactoryImpl::JobController::
     ShouldCreateAlternativeProxyServerJob(
-        Job* job,
         const ProxyInfo& proxy_info,
         const GURL& url,
         ProxyServer* alternative_proxy_server) const {
   DCHECK(!alternative_proxy_server->is_valid());
 
   if (!enable_alternative_services_)
     return false;
 
   if (!can_start_alternative_proxy_job_) {
     // Either an alternative service job or an alternative proxy server job has
     // already been started.
     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.
     return false;
   }
 
   if (!url.SchemeIs(url::kHttpScheme)) {
     // Only HTTP URLs can be fetched through alternative proxy server, since the
     // alternative proxy server may not support fetching of URLs with other
     // schemes.
     return false;
   }
 
   ProxyDelegate* proxy_delegate = session_->params().proxy_delegate;
   if (!proxy_delegate)
     return false;
-
   proxy_delegate->GetAlternativeProxy(url, proxy_info.proxy_server(),
                                       alternative_proxy_server);
 
   if (!alternative_proxy_server->is_valid())
     return false;
 
   DCHECK(!(*alternative_proxy_server == proxy_info.proxy_server()));
 
   if (!alternative_proxy_server->is_https() &&
       !alternative_proxy_server->is_quic()) {
@@ skipping 39 matching lines @@
     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);
 }
 
 }  // namespace net