Change QuicStreamRequest::Request() to take a preferred QuicVersion so that

net/http/http_stream_factory_impl_job.cc

 // Copyright (c) 2012 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.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/bind.h"
@@ skipping 144 matching lines @@
                                 JobType job_type,
                                 HttpNetworkSession* session,
                                 const HttpRequestInfo& request_info,
                                 RequestPriority priority,
                                 const ProxyInfo& proxy_info,
                                 const SSLConfig& server_ssl_config,
                                 const SSLConfig& proxy_ssl_config,
                                 HostPortPair destination,
                                 GURL origin_url,
                                 NextProto alternative_protocol,
+                                QuicVersion quic_version,
                                 const ProxyServer& alternative_proxy_server,
                                 bool enable_ip_based_pooling,
                                 NetLog* net_log)
     : request_info_(request_info),
       priority_(priority),
       proxy_info_(proxy_info),
       server_ssl_config_(server_ssl_config),
       proxy_ssl_config_(proxy_ssl_config),
       net_log_(
           NetLogWithSource::Make(net_log, NetLogSourceType::HTTP_STREAM_JOB)),
       io_callback_(base::Bind(&Job::OnIOComplete, base::Unretained(this))),
       connection_(new ClientSocketHandle),
       session_(session),
       state_(STATE_NONE),
       next_state_(STATE_NONE),
       destination_(destination),
       origin_url_(origin_url),
       alternative_proxy_server_(alternative_proxy_server),
       enable_ip_based_pooling_(enable_ip_based_pooling),
       delegate_(delegate),
       job_type_(job_type),
       using_ssl_(origin_url_.SchemeIs(url::kHttpsScheme) ||
                  origin_url_.SchemeIs(url::kWssScheme)),
       using_quic_(
           alternative_protocol == kProtoQUIC ||
           ShouldForceQuic(session, destination, origin_url, proxy_info)),
+      quic_version_(quic_version),
       expect_spdy_(alternative_protocol == kProtoHTTP2 && !using_quic_),
       using_spdy_(false),
       should_reconsider_proxy_(false),
       quic_request_(session_->quic_stream_factory()),
       using_existing_quic_session_(false),
       establishing_tunnel_(false),
       was_alpn_negotiated_(false),
       negotiated_protocol_(kProtoUnknown),
       num_streams_(0),
       spdy_session_direct_(
           !(proxy_info.is_https() && origin_url_.SchemeIs(url::kHttpScheme))),
       spdy_session_key_(using_quic_
                             ? SpdySessionKey()
                             : GetSpdySessionKey(spdy_session_direct_,
                                                 proxy_info_.proxy_server(),
                                                 origin_url_,
                                                 request_info_.privacy_mode)),
       stream_type_(HttpStreamRequest::BIDIRECTIONAL_STREAM),
       init_connection_already_resumed_(false),
       ptr_factory_(this) {
+  if (quic_version_ == QUIC_VERSION_UNSUPPORTED &&
+      ShouldForceQuic(session, destination, origin_url, proxy_info))

[Ryan Hamilton, 2017/07/06 13:27:45]

Why does this only apply when "ShouldForceQuic" is true?

[Zhongyi Shi, 2017/07/06 18:13:39]

When we create the Jobs in JobController, we pass over the quic_version if the
Job created is an alternative job and uses QUIC. When we create the main job, we
always pass in a QUIC_VERSION_UNSUPPORTED.

However, Job could also later figure out to use QUIC if the origin has forced
QUIC on. That's why we want to change |quic_version_| to a valid version in this
case.

[Ryan Hamilton, 2017/07/06 19:00:04]

(I guess an alternative would be to pass in supported_versions_[0] for the Main
job, to handle this case?
Ah! That makes sense. Can you add a comment to this effect? Also, I assume that
this DCHECK would be valid?

if (using_quic_) 
  DCHECK(quic_version != QUIC_VERSION_UNSUPPORTED)

[Zhongyi Shi, 2017/07/06 19:40:56]

This could work. However, it might also be confusing. A job is not using QUIC
but have a quic_version_ specified. Besides, JobController who creates the Job
could not designate a quic version for the Job when creating.
Done. 

+    quic_version_ = session->params().quic_supported_versions[0];

[Ryan Hamilton, 2017/07/06 13:27:45]

nit: {}s since the if predicate is more than one line.

[Zhongyi Shi, 2017/07/06 18:13:40]

Done.

+
   DCHECK(session);
   if (alternative_protocol != kProtoUnknown) {
     // The job cannot have protocol requirements dictated by alternative service
     // and have an alternative proxy server set at the same time, since
     // alternative services are used for requests that are fetched directly,
     // while the alternative proxy server is used for requests that should be
     // fetched using proxy.
     DCHECK(!alternative_proxy_server_.is_valid());
     // If the alternative service protocol is specified, then the job type must
     // be either ALTERNATIVE or PRECONNECT.
@@ skipping 670 matching lines @@
       replacements.ClearRef();
       url = url.ReplaceComponents(replacements);
     } else {
       DCHECK(using_ssl_);
       // The certificate of a QUIC alternative server is expected to be valid
       // for the origin of the request (in addition to being valid for the
       // server itself).
       destination = destination_;
       ssl_config = &server_ssl_config_;
     }
-    int rv =
-        quic_request_.Request(destination, request_info_.privacy_mode,
-                              ssl_config->GetCertVerifyFlags(), url,
-                              request_info_.method, net_log_, io_callback_);
+    int rv = quic_request_.Request(
+        destination, quic_version_, request_info_.privacy_mode,
+        ssl_config->GetCertVerifyFlags(), url, request_info_.method, net_log_,
+        io_callback_);
     if (rv == OK) {
       using_existing_quic_session_ = true;
     } else {
       // There's no available QUIC session. Inform the delegate how long to
       // delay the main job.
       if (rv == ERR_IO_PENDING) {
         delegate_->MaybeSetWaitTimeForMainJob(
             quic_request_.GetTimeDelayForWaitingJob());
       }
     }
@@ skipping 545 matching lines @@
     const ProxyInfo& proxy_info,
     const SSLConfig& server_ssl_config,
     const SSLConfig& proxy_ssl_config,
     HostPortPair destination,
     GURL origin_url,
     bool enable_ip_based_pooling,
     NetLog* net_log) {
   return base::MakeUnique<HttpStreamFactoryImpl::Job>(
       delegate, job_type, session, request_info, priority, proxy_info,
       server_ssl_config, proxy_ssl_config, destination, origin_url,
-      kProtoUnknown, ProxyServer(), enable_ip_based_pooling, net_log);
+      kProtoUnknown, QUIC_VERSION_UNSUPPORTED, ProxyServer(),
+      enable_ip_based_pooling, net_log);
 }
 
 std::unique_ptr<HttpStreamFactoryImpl::Job>
 HttpStreamFactoryImpl::JobFactory::CreateAltSvcJob(
     HttpStreamFactoryImpl::Job::Delegate* delegate,
     HttpStreamFactoryImpl::JobType job_type,
     HttpNetworkSession* session,
     const HttpRequestInfo& request_info,
     RequestPriority priority,
     const ProxyInfo& proxy_info,
     const SSLConfig& server_ssl_config,
     const SSLConfig& proxy_ssl_config,
     HostPortPair destination,
     GURL origin_url,
     NextProto alternative_protocol,
+    QuicVersion quic_version,
     bool enable_ip_based_pooling,
     NetLog* net_log) {
   return base::MakeUnique<HttpStreamFactoryImpl::Job>(
       delegate, job_type, session, request_info, priority, proxy_info,
       server_ssl_config, proxy_ssl_config, destination, origin_url,
-      alternative_protocol, ProxyServer(), enable_ip_based_pooling, net_log);
+      alternative_protocol, quic_version, ProxyServer(),
+      enable_ip_based_pooling, net_log);
 }
 
 std::unique_ptr<HttpStreamFactoryImpl::Job>
 HttpStreamFactoryImpl::JobFactory::CreateAltProxyJob(
     HttpStreamFactoryImpl::Job::Delegate* delegate,
     HttpStreamFactoryImpl::JobType job_type,
     HttpNetworkSession* session,
     const HttpRequestInfo& request_info,
     RequestPriority priority,
     const ProxyInfo& proxy_info,
     const SSLConfig& server_ssl_config,
     const SSLConfig& proxy_ssl_config,
     HostPortPair destination,
     GURL origin_url,
     const ProxyServer& alternative_proxy_server,
     bool enable_ip_based_pooling,
     NetLog* net_log) {
   return base::MakeUnique<HttpStreamFactoryImpl::Job>(
       delegate, job_type, session, request_info, priority, proxy_info,
       server_ssl_config, proxy_ssl_config, destination, origin_url,
-      kProtoUnknown, alternative_proxy_server, enable_ip_based_pooling,
-      net_log);
+      kProtoUnknown, QUIC_VERSION_UNSUPPORTED, alternative_proxy_server,
+      enable_ip_based_pooling, net_log);
 }
 
 }  // namespace net