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 fc19d24e18b103a384f34507ddf30a277dd5582c..14d412c7b7ad1f77c83e946f27dcee7d4be97fdb 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"
@@ -115,24 +116,26 @@ NET_EXPORT_PRIVATE std::ostream& operator<<(
     const AlternativeService& alternative_service);
 
 struct NET_EXPORT AlternativeServiceInfo {
-  AlternativeServiceInfo() : alternative_service() {}
+  AlternativeServiceInfo();
+  ~AlternativeServiceInfo();
 
+  // HTTP/2 Alternative Service.
   AlternativeServiceInfo(const AlternativeService& alternative_service,
-                         base::Time expiration)
-      : alternative_service(alternative_service),
-        expiration(expiration) {}
+                         base::Time expiration);

[Ryan Hamilton, 2017/05/24 19:52:53]

Does anything use this constructor? SetAlternativeServices seems to use the
second, but I didn't look closely enough.

[Zhongyi Shi, 2017/05/24 22:44:54]

SetAlternativeService uses the second, SetAlternativeService*s* takes a
AlternativeServiceInfoVector. 

SpdySession::OnAltSvc() uses this constructor to create AlternativeServicesInfo,
and pushes to a AlternativeServiceInfoVector. 
https://cs.chromium.org/chromium/src/net/spdy/chromium/spdy_session.cc?q=Alte...

[Ryan Hamilton, 2017/05/24 23:12:12]

Can SpdySession create a QUIC Alt-Svc?

 
-  AlternativeServiceInfo(NextProto protocol,
-                         const std::string& host,
-                         uint16_t port,
-                         base::Time expiration)
-      : alternative_service(protocol, host, port), expiration(expiration) {}
+  // QUIC Alternative Service.

[Ryan Hamilton, 2017/05/24 19:52:53]

I'm not sure these two comments are accurate. I don't think that the
constructors check the protocol in |alternative_service|. So it would be
possible to use either one. Also, It's possible to advertise QUIC support
without specifying the list of QUIC versions which might suggest a QUIC alt-svc
being set with the first version.

[Zhongyi Shi, 2017/05/24 22:44:55]

Done.

+  AlternativeServiceInfo(const AlternativeService& alternative_service,
+                         base::Time expiration,
+                         const QuicVersionVector& advertised_versions);
 
   AlternativeServiceInfo(
-      const AlternativeServiceInfo& alternative_service_info) = default;
+      const AlternativeServiceInfo& alternative_service_info);
+
   AlternativeServiceInfo& operator=(
       const AlternativeServiceInfo& alternative_service_info) = default;

[Ryan Hamilton, 2017/05/24 19:52:53]

(I'm surprised that you don't get a compiler complaint about = default here. In
any case, since the copy constructor moved to the .cc file, can you move the
assignment operator there too?

 
+  // TODO(zhongyi): add equality check on |advertised_versions| once it
+  // is persisted to disk.

[Ryan Hamilton, 2017/05/24 19:52:54]

Why not do this now?

[Zhongyi Shi, 2017/05/24 22:44:54]

Ah, apparently I forgot to give a heads-up on this CL: I didn't plan to land
this CL but for review only. A following CL dependent on this one will be sent
for review which adds the logic to persist Alt Svc Info with versions into disk.

[Ryan Hamilton, 2017/05/24 23:12:12]

Heh. OK. Any reason not to land this?

   bool operator==(const AlternativeServiceInfo& other) const {
     return alternative_service == other.alternative_service &&
            expiration == other.expiration;
@@ -146,6 +149,7 @@ struct NET_EXPORT AlternativeServiceInfo {
 
   AlternativeService alternative_service;
   base::Time expiration;
+  QuicVersionVector advertised_versions;
 };
 
 struct NET_EXPORT SupportsQuic {
@@ -251,7 +255,8 @@ class NET_EXPORT HttpServerProperties {
   virtual bool SetAlternativeService(
       const url::SchemeHostPort& origin,
       const AlternativeService& alternative_service,
-      base::Time expiration) = 0;
+      base::Time expiration,
+      const QuicVersionVector& advertised_versions) = 0;
 
   // Set alternative services for |origin|.  Previous alternative services for
   // |origin| are discarded.