ABANDONED Handle multiple AlternateProtocols for each HostPortPair.

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 edf598ff029c4e6f79119c702fb001c2d33913c4..ef728ce3d6418afcb184007d90f02094e91bca24 100644
--- a/net/http/http_server_properties.h
+++ b/net/http/http_server_properties.h
@@ -7,6 +7,8 @@
 
 #include <map>
 #include <string>
+#include <vector>
+
 #include "base/basictypes.h"
 #include "base/containers/mru_cache.h"
 #include "base/memory/weak_ptr.h"
@@ -76,13 +78,11 @@ enum AlternateProtocolSize {
     ALTERNATE_PROTOCOL_MINIMUM_VALID_VERSION + 1,
 };
 
-NET_EXPORT const char* AlternateProtocolToString(AlternateProtocol protocol);
-NET_EXPORT AlternateProtocol AlternateProtocolFromString(
-    const std::string& str);
-NET_EXPORT_PRIVATE AlternateProtocol AlternateProtocolFromNextProto(
-    NextProto next_proto);
-
+// TODO(bnc):  Rename to AlternateServiceInfo.
 struct NET_EXPORT AlternateProtocolInfo {
+  AlternateProtocolInfo()
+      : port(), protocol(UNINITIALIZED_ALTERNATE_PROTOCOL), probability() {}
+
   AlternateProtocolInfo(uint16 port,
                         AlternateProtocol protocol,
                         double probability)
@@ -96,6 +96,11 @@ struct NET_EXPORT AlternateProtocolInfo {
         probability == other.probability;
   }
 
+  bool EqualsModuloProbabilityAndProtocol(
+      const AlternateProtocolInfo& other) const {
+    return port == other.port;

[Ryan Hamilton, 2014/11/03 19:00:30]

I'm surprised that this only checks the port. Is this method actually used? 

[Bence, 2014/11/03 22:32:25]

Yes, I use it in AlternateProtocols to identify duplicates.  Once we start
saving host in AlternateProtocolInfo, this method will compare that too. 
(Currently host is implied to be the same as the first one.)

[Ryan Hamilton, 2014/11/04 18:56:22]

Ah right! There are still no hosts. Gotcha.

+  }
+
   std::string ToString() const;
 
   uint16 port;
@@ -117,13 +122,23 @@ struct NET_EXPORT SupportsQuic {
   std::string address;
 };
 
-typedef base::MRUCache<
-    HostPortPair, AlternateProtocolInfo> AlternateProtocolMap;
+// TODO(bnc):  Rename to AlternateServices.
+typedef std::vector<AlternateProtocolInfo> AlternateProtocols;
+// TODO(bnc):  Rename to AlternateServicesMap.
+typedef base::MRUCache<HostPortPair, AlternateProtocols> AlternateProtocolMap;
 typedef base::MRUCache<HostPortPair, SettingsMap> SpdySettingsMap;
 typedef std::map<HostPortPair, SupportsQuic> SupportsQuicMap;
 
 extern const char kAlternateProtocolHeader[];
 
+NET_EXPORT const char* AlternateProtocolToString(AlternateProtocol protocol);
+// TODO(bnc):  Rename to AlternateServicesToString.
+NET_EXPORT std::string AlternateProtocolsToString(AlternateProtocols services);
+NET_EXPORT AlternateProtocol
+    AlternateProtocolFromString(const std::string& str);
+NET_EXPORT_PRIVATE AlternateProtocol
+    AlternateProtocolFromNextProto(NextProto next_proto);
+
 // The interface for setting/retrieving the HTTP server properties.
 // Currently, this class manages servers':
 // * SPDY support (based on NPN results)
@@ -154,33 +169,48 @@ class NET_EXPORT HttpServerProperties {
                                bool support_spdy) = 0;
 
   // Returns true if |server| has an Alternate-Protocol header.
+  // TODO(bnc):  Rename to HasAlternateService.
   virtual bool HasAlternateProtocol(const HostPortPair& server) = 0;
 
-  // Returns the Alternate-Protocol and port for |server|.
+  // Returns the Alternate-Protocols for |server|.
   // HasAlternateProtocol(server) must be true.
-  virtual AlternateProtocolInfo GetAlternateProtocol(
+  virtual AlternateProtocols GetAlternateProtocol(

[Ryan Hamilton, 2014/11/03 19:00:30]

Seems like it might be a good idea for this to return a const ref since this
struct is (potentially bigish). 

[Ryan Hamilton, 2014/11/03 19:00:30]

Since this returns multiple protocols, should the method be named
GetAlternateProtocols()?

[Bence, 2014/11/03 22:32:25]

Done.

[Bence, 2014/11/03 22:32:25]

Done.

       const HostPortPair& server) = 0;
 
-  // Sets the Alternate-Protocol for |server|.
-  virtual void SetAlternateProtocol(const HostPortPair& server,
+  // Adds an Alternate-Protocol for |server|.
+  // TODO(bnc):  Rename to AddAlternateService.
+  virtual void AddAlternateProtocol(const HostPortPair& server,
                                     uint16 alternate_port,
                                     AlternateProtocol alternate_protocol,
                                     double probability) = 0;
 
-  // Sets the Alternate-Protocol for |server| to be BROKEN.
-  virtual void SetBrokenAlternateProtocol(const HostPortPair& server) = 0;
+  // Sets a given Alternate-Protocol for |server| to be BROKEN.
+  virtual void SetBrokenAlternateProtocol(
+      const HostPortPair& server,
+      const AlternateProtocolInfo& broken_alternate_protocol) = 0;

[Ryan Hamilton, 2014/11/03 19:00:30]

Does this only need to take the protocol, or does it also need to take the
host:port for the alternative as well? (Same goes for below)

[Bence, 2014/11/03 22:32:26]

Since a server might advertise multiple alternate protocols (services) using the
same protocol but different ports (and hosts), we need the entire
AlternateProtocolInfo structure except for the probability to know which one to
set as broken/query if was broken/confirm as not broken/remove.

[Ryan Hamilton, 2014/11/04 18:56:22]

Argh! I misread AlternateProtocolInfo as the protocol enum again. Once it
becomes AlternateServiceInfo, I think I'll stop making this mistake.

 
-  // Returns true if Alternate-Protocol for |server| was recently BROKEN.
+  // Returns true if given Alternate-Protocol for |server| was recently BROKEN.
   virtual bool WasAlternateProtocolRecentlyBroken(
-      const HostPortPair& server) = 0;
+      const HostPortPair& server,
+      const AlternateProtocolInfo& alternate_protocol) = 0;
 
-  // Confirms that Alternate-Protocol for |server| is working.
-  virtual void ConfirmAlternateProtocol(const HostPortPair& server) = 0;
+  // Confirms that given Alternate-Protocol for |server| is working.
+  virtual void ConfirmAlternateProtocol(
+      const HostPortPair& server,
+      const AlternateProtocolInfo& alternate_protocol) = 0;
 
-  // Clears the Alternate-Protocol for |server|.
+  // Clears all Alternate-Protocols for |server|.
+  // TODO(bnc):  Rename to ClearAlternateServices.
   virtual void ClearAlternateProtocol(const HostPortPair& server) = 0;
 
+  // Removes an Alternate-Protocol for |server|.
+  // TODO(bnc):  Rename to RemoveAlternateService.
+  virtual void RemoveAlternateProtocol(
+      const HostPortPair& server,
+      const AlternateProtocolInfo& alternate_protocol) = 0;
+
   // Returns all Alternate-Protocol mappings.
+  // TODO(bnc):  Rename to alternate_services_map.
   virtual const AlternateProtocolMap& alternate_protocol_map() const = 0;
 
   // Sets the threshold to be used when evaluating Alternate-Protocol