Add and persist a new field in AlternativeServiceInfo to list QUIC verisons advertised

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 <string>
 #include <utility>
 
 #include "base/memory/ptr_util.h"
@@ skipping 1133 matching lines @@
     HostPortPair mapped_origin(origin.host(), origin.port());
     ignore_result(ApplyHostMappingRules(original_url, &mapped_origin));
     QuicServerId server_id(mapped_origin, request_info.privacy_mode);
 
     HostPortPair destination(
         alternative_service_info.alternative_service.host_port_pair());
     ignore_result(ApplyHostMappingRules(original_url, &destination));
 
     if (session_->quic_stream_factory()->CanUseExistingSession(server_id,
                                                                destination)) {
-      return alternative_service_info;
+      // If advertised versions for QUIC is missing for this
+      // AlternaitverServiceInfo, by default append the list of QUIC versions

[Bence, 2017/06/08 17:44:51]

s/AlternaitverServiceInfo/AlternativeServiceInfo/

[Zhongyi Shi, 2017/06/08 23:11:16]

Acknowledged.

+      // supported by this instance of network stack.
+      first_alternative_service_info = alternative_service_info;
+      if (first_alternative_service_info.alternative_service.protocol ==
+              kProtoQUIC &&
+          first_alternative_service_info.advertised_versions().empty()) {
+        first_alternative_service_info.set_advertised_versions(
+            session_->params().quic_supported_versions);
+      }

[Ryan Hamilton, 2017/06/07 20:52:24]

I wonder if we can get away without this code. If we don't have this, then we'll
end up creating a job with an empty QUIC version vector, right? And that vector
will (eventually) be passed down to the QuicStreamFactory. I think the
QuicStreamFactory would be a good place to apply this logic.

What do you think?

[Zhongyi Shi, 2017/06/08 23:11:16]

Ah good idea! I didn't think about this. But yeah, QuicStreamFactory knows
currently supported versions by Chrome, and should be able to figure out what to
do with an empty list of advertised versions.

+      return first_alternative_service_info;
     }
 
     // Cache this entry if we don't have a non-broken Alt-Svc yet.
     if (first_alternative_service_info.alternative_service.protocol ==
         kProtoUnknown)
       first_alternative_service_info = alternative_service_info;
   }
 
   // Ask delegate to mark QUIC as broken for the origin.
   if (quic_advertised && quic_all_broken && delegate != nullptr)
     delegate->OnQuicBroken();
 
+  // If advertised versions for QUIC is missing for this
+  // AlternaitverServiceInfo, by default append the list of QUIC versions
+  // supported by this instance of network stack.
+  if (first_alternative_service_info.alternative_service.protocol ==
+          kProtoQUIC &&
+      first_alternative_service_info.advertised_versions().empty()) {
+    first_alternative_service_info.set_advertised_versions(
+        session_->params().quic_supported_versions);
+  }
+
   return first_alternative_service_info;
 }
 
 bool HttpStreamFactoryImpl::JobController::
     ShouldCreateAlternativeProxyServerJob(
         const ProxyInfo& proxy_info,
         const GURL& url,
         ProxyServer* alternative_proxy_server) const {
   DCHECK(!alternative_proxy_server->is_valid());
 
@@ skipping 117 matching lines @@
     // 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;
 }
 
 }  // namespace net