Race preconnects to HTTP2 proxies that support alternate proxies

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 80 matching lines @@
 
 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_);
 
   is_preconnect_ = true;
+
+  can_start_alternative_proxy_job_ =
+      session_->params().race_preconnects_to_proxies;
+
   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);
 
   if (alternative_service.protocol != kProtoUnknown) {
     if (session_->params().quic_disable_preconnect_if_0rtt &&
         alternative_service.protocol == kProtoQUIC &&
         session_->quic_stream_factory()->ZeroRTTEnabledFor(QuicServerId(
@@ skipping 272 matching lines @@
     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(is_preconnect_ || job->job_type() == MAIN);
+  DCHECK_EQ(main_job_.get(), job);
   DCHECK(!alternative_job_);
   DCHECK(!main_job_is_blocked_);
+  DCHECK(alternative_proxy_server.is_quic());

[Ryan Hamilton, 2017/01/23 18:05:53]

Why is this guaranteed to be true?

[tbansal1, 2017/01/23 21:48:22]

Removed. (Currently it is true because QUIC is the only alternative service).

[Ryan Hamilton, 2017/01/24 19:36:59]

We also support HTTP/2 based Alt-Svc.

+
+  if (is_preconnect_) {
+    // Restrict the number of streams to 1 since the proxy server supports spdy.
+    job->RestrictNumStreamsToOne();

[Ryan Hamilton, 2017/01/23 18:05:53]

It seems like this is a bit orthogonal to racing alt-proxies, isn't it?

[tbansal1, 2017/01/23 21:48:22]

The preconnections for non-proxy http2 jobs restrict the number of preconnect
streams to 1.
https://cs.chromium.org/chromium/src/net/http/http_stream_factory_impl_job.cc...

This CL enables preconnects for proxy http2 jobs. So, we should also restrict
the preconnect streams to 1 for proxy http2/quic jobs.

I can do it in a separate CL too, but then I would need to plumb |num_streams|
from the Job to
HttpStreamFactoryImpl::JobController::StartAlternativeProxyServerJob() method
below.

[Ryan Hamilton, 2017/01/24 19:36:59]

Ah, I see. You're saying that "restrict to 1 stream" is inherently part of
preconnect, and since this CL implements a new preconnect mechanism, we need to
do the same thing. Did I get that right?

+  }
 
   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,
-      job->net_log().net_log()));
-  AttachJob(alternative_job_.get());
+      this, is_preconnect_ ? PRECONNECT : ALTERNATIVE, session_, request_info,
+      priority, server_ssl_config, proxy_ssl_config, destination, origin_url,
+      alternative_proxy_server, job->net_log().net_log()));
+
+  if (!is_preconnect_) {
+    // Preconnect jobs do not have an associated request. So, the preconnect
+    // job can't be attached.
+    AttachJob(alternative_job_.get());
+  }
 
   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()));
 }
@@ skipping 59 matching lines @@
     factory->OnNewSpdySessionReady(spdy_session, direct, used_ssl_config,
                                    used_proxy_info, was_alpn_negotiated,
                                    negotiated_protocol, using_spdy, net_log);
   }
   if (is_job_orphaned) {
     OnOrphanedJobComplete(job);
   }
 }
 
 void HttpStreamFactoryImpl::JobController::OnPreconnectsComplete(Job* job) {
-  DCHECK_EQ(main_job_.get(), job);
-  main_job_.reset();
-  factory_->OnPreconnectsCompleteInternal();
-  MaybeNotifyFactoryOfCompletion();
+  DCHECK(job);
+  DCHECK(main_job_.get() == job || alternative_job_.get() == job);
+  DCHECK_EQ(PRECONNECT, job->job_type());
+  if (is_preconnect_ && main_job_ && alternative_job_) {
+    UMA_HISTOGRAM_EXACT_LINEAR("Net.QuicAlternativeProxy.PreconnectJobsRaced",
+                               1, 2);

[Ryan Hamilton, 2017/01/23 18:05:53]

I don't understand what this histogram is recording?

[tbansal1, 2017/01/23 21:48:23]

It is just recording how many times preconnect jobs were raced. I have changed
the variable name.

+  }
+
+  // Note that completion of a preconnect job does not indicate that
+  // preconnection was actually successful since the socket layer currently does
+  // not provide the success or failure status of the (possibly) multiple
+  // preconnect sockets.
+  if (job == main_job_.get())
+    main_job_.reset();
+
+  if (job == alternative_job_.get())
+    alternative_job_.reset();
+
+  if (!main_job_ && !alternative_job_) {
+    factory_->OnPreconnectsCompleteInternal();
+    MaybeNotifyFactoryOfCompletion();
+  }
 }
 
 void HttpStreamFactoryImpl::JobController::OnOrphanedJobComplete(
     const Job* job) {
   if (job->job_type() == MAIN) {
     DCHECK_EQ(main_job_.get(), job);
     main_job_.reset();
   } else {
     DCHECK_EQ(alternative_job_.get(), job);
     alternative_job_.reset();
@@ skipping 176 matching lines @@
     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());
 }
 
 void HttpStreamFactoryImpl::JobController::AttachJob(Job* job) {
   DCHECK(job);
+  DCHECK(!is_preconnect_);
   factory_->request_map_[job] = request_;
 }
 
 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_);
 
@@ skipping 304 matching lines @@
     // 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.
@@ skipping 47 matching lines @@
     HistogramAlternateProtocolUsage(ALTERNATE_PROTOCOL_USAGE_NO_RACE,
                                     proxy_server_used);
     return;
   }
 
   HistogramAlternateProtocolUsage(ALTERNATE_PROTOCOL_USAGE_WON_RACE,
                                   proxy_server_used);
 }
 
 void HttpStreamFactoryImpl::JobController::StartAlternativeProxyServerJob() {
-  if (!alternative_job_ || !request_)
+  if (!alternative_job_)
     return;
   DCHECK(alternative_job_->alternative_proxy_server().is_valid());
-  alternative_job_->Start(request_->stream_type());
+
+  if (!is_preconnect_) {
+    if (!request_)
+      return;
+    DCHECK(alternative_job_->alternative_proxy_server().is_quic());
+    alternative_job_->Start(request_->stream_type());
+  } else {
+    // Restrict the number of streams to 1 since the alternative proxy server
+    // supports request priorities.
+    DCHECK(alternative_job_->alternative_proxy_server().is_quic());
+    alternative_job_->Preconnect(1 /* num_streams */);
+  }
 }
 
 }  // namespace net