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

net/http/http_stream_factory_impl_job.cc

Index: net/http/http_stream_factory_impl_job.cc
diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc
index c5a1b9adf12c05f1024cc223005f4d9f8f1723de..8239af664f51ff7bf393a7203eda2e13f0f97729 100644
--- a/net/http/http_stream_factory_impl_job.cc
+++ b/net/http/http_stream_factory_impl_job.cc
@@ -162,6 +162,7 @@ HttpStreamFactoryImpl::Job::Job(Delegate* delegate,
                                 HostPortPair destination,
                                 GURL origin_url,
                                 NextProto alternative_protocol,
+                                QuicVersion quic_version,
                                 const ProxyServer& alternative_proxy_server,
                                 bool enable_ip_based_pooling,
                                 NetLog* net_log)
@@ -188,6 +189,7 @@ HttpStreamFactoryImpl::Job::Job(Delegate* delegate,
       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),
@@ -208,6 +210,10 @@ HttpStreamFactoryImpl::Job::Job(Delegate* delegate,
       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
@@ -898,10 +904,10 @@ int HttpStreamFactoryImpl::Job::DoInitConnectionImpl() {
       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 {
@@ -1467,7 +1473,8 @@ HttpStreamFactoryImpl::JobFactory::CreateMainJob(
   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>
@@ -1483,12 +1490,14 @@ HttpStreamFactoryImpl::JobFactory::CreateAltSvcJob(
     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>
@@ -1509,8 +1518,8 @@ HttpStreamFactoryImpl::JobFactory::CreateAltProxyJob(
   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