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

net/http/http_server_properties.h

Index: net/http/http_server_properties.h
diff --git a/net/http/http_server_properties.h b/net/http/http_server_properties.h
index 27d8b078c8aa2c9ac210329f114af43c467c58f7..b62f200b32c274119a7df851c292612f6c21aa7c 100644
--- a/net/http/http_server_properties.h
+++ b/net/http/http_server_properties.h
@@ -20,6 +20,7 @@
 #include "net/base/net_export.h"
 #include "net/quic/core/quic_bandwidth.h"
 #include "net/quic/core/quic_server_id.h"
+#include "net/quic/core/quic_versions.h"
 #include "net/socket/next_proto.h"
 #include "net/spdy/core/spdy_framer.h"  // TODO(willchan): Reconsider this.
 #include "net/spdy/core/spdy_protocol.h"
@@ -116,24 +117,28 @@ NET_EXPORT_PRIVATE std::ostream& operator<<(
 
 class NET_EXPORT_PRIVATE AlternativeServiceInfo {
  public:
-  AlternativeServiceInfo();
+  static AlternativeServiceInfo CreateHttp2AlternativeServiceInfo(
+      const AlternativeService& alternative_service,
+      base::Time expiration);
 
-  AlternativeServiceInfo(const AlternativeService& alternative_service,
-                         base::Time expiration);
+  static AlternativeServiceInfo CreateQuicAlternativeServiceInfo(
+      const AlternativeService& alternative_service,
+      base::Time expiration,
+      const QuicVersionVector& advertised_versions);
 
-  AlternativeServiceInfo(NextProto protocol,
-                         const std::string& host,
-                         uint16_t port,
-                         base::Time expiration);
+  AlternativeServiceInfo();
+  ~AlternativeServiceInfo();
 
   AlternativeServiceInfo(
       const AlternativeServiceInfo& alternative_service_info);
+
   AlternativeServiceInfo& operator=(
       const AlternativeServiceInfo& alternative_service_info);
 
   bool operator==(const AlternativeServiceInfo& other) const {
     return alternative_service_ == other.alternative_service() &&
-           expiration_ == other.expiration();
+           expiration_ == other.expiration() &&
+           advertised_versions_ == other.advertised_versions();
   }
 
   bool operator!=(const AlternativeServiceInfo& other) const {
@@ -158,15 +163,37 @@ class NET_EXPORT_PRIVATE AlternativeServiceInfo {
     expiration_ = expiration;
   }
 
+  void set_advertised_versions(const QuicVersionVector& advertised_versions) {
+    if (alternative_service_.protocol != kProtoQUIC)

[Bence, 2017/06/16 14:43:40]

DCHECK_EQ(kProtoQUIC, alternative_service.protocol);

[Zhongyi Shi, 2017/06/20 23:23:37]

Although we are expecting this method only be called when alt svc is using QUIC
protocol, there was no API enforcement on the protocol field. 

I think an early return after check will make sure that we are doing the right
thing regardless of the callsite correctness. DCHECK will only fire in a debug
build. Or should we change this to CHECK()?

[Bence, 2017/06/21 12:39:25]

Good points.

One option is to leave it as it is now, and I'm fine with that.

The other option is to make it part of the API (by adding a comment in the
header file), and use a DCHECK().  I have a slight preference to this, but early
return is also fine.

I do not think a CHECK() is necessary: it has the same runtime cost as an early
return, but my understanding is that a CHECK() is usually reserved for
preventing security issues/leaks.  Having a DCHECK() and adequate test coverage
should be enough to guarantee that all callers follow the API requirement.

+      return;
+
+    advertised_versions_ = advertised_versions;
+    std::sort(advertised_versions_.begin(), advertised_versions_.end());
+  }
+
   const AlternativeService& alternative_service() const {
     return alternative_service_;
   }
 
   base::Time expiration() const { return expiration_; }
 
+  const QuicVersionVector& advertised_versions() const {
+    return advertised_versions_;
+  }
+
  private:
+  AlternativeServiceInfo(const AlternativeService& alternative_service,
+                         base::Time expiration,
+                         const QuicVersionVector& advertised_versions);
+
   AlternativeService alternative_service_;
   base::Time expiration_;
+
+  // Lists all the QUIC versions that are advertised by the server and supported
+  // by Chrome. If empty, defaults to versions used by the current instance of
+  // the netstack.
+  // This list MUST be sorted in ascending order.
+  QuicVersionVector advertised_versions_;
 };
 
 struct NET_EXPORT SupportsQuic {
@@ -264,16 +291,27 @@ class NET_EXPORT HttpServerProperties {
   virtual AlternativeServiceInfoVector GetAlternativeServiceInfos(
       const url::SchemeHostPort& origin) = 0;
 
-  // Set a single alternative service for |origin|.  Previous alternative
-  // services for |origin| are discarded.
+  // Set a single HTTP/2 alternative service for |origin|.  Previous
+  // alternative services for |origin| are discarded.
   // |alternative_service.host| may be empty.
   // Return true if |alternative_service_map_| has changed significantly enough
   // that it should be persisted to disk.
-  virtual bool SetAlternativeService(
+  virtual bool SetHttp2AlternativeService(
       const url::SchemeHostPort& origin,
       const AlternativeService& alternative_service,
       base::Time expiration) = 0;
 
+  // Set a single QUIC alternative service for |origin|.  Previous alternative
+  // services for |origin| are discarded.
+  // |alternative_service.host| may be empty.
+  // Return true if |alternative_service_map_| has changed significantly enough
+  // that it should be persisted to disk.
+  virtual bool SetQuicAlternativeService(
+      const url::SchemeHostPort& origin,
+      const AlternativeService& alternative_service,
+      base::Time expiration,
+      const QuicVersionVector& advertised_versions) = 0;
+
   // Set alternative services for |origin|.  Previous alternative services for
   // |origin| are discarded.
   // Hostnames in |alternative_service_info_vector| may be empty.