Add AlternateProtocol to broken_altproto_list_ and map_.

net/http/http_server_properties_impl.cc

Index: net/http/http_server_properties_impl.cc
diff --git a/net/http/http_server_properties_impl.cc b/net/http/http_server_properties_impl.cc
index a53e73fded1f969e45a5361f0d7d009d4fd57c87..3f685212e94d941033bcb395aefb6e5de065441b 100644
--- a/net/http/http_server_properties_impl.cc
+++ b/net/http/http_server_properties_impl.cc
@@ -321,6 +321,7 @@ void HttpServerPropertiesImpl::SetAlternateProtocol(
 
 void HttpServerPropertiesImpl::SetBrokenAlternateProtocol(
     const HostPortPair& server) {
+  const AlternateProtocolInfo alternate = GetAlternateProtocol(server);
   AlternateProtocolMap::iterator it = alternate_protocol_map_.Get(server);
   if (it == alternate_protocol_map_.end()) {
     if (!HasAlternateProtocol(server)) {
@@ -330,16 +331,17 @@ void HttpServerPropertiesImpl::SetBrokenAlternateProtocol(
     // This server's alternate protocol information is coming from a canonical
     // server. Add an entry in the map for this server explicitly so that
     // it can be marked as broken.
-    it = alternate_protocol_map_.Put(server, GetAlternateProtocol(server));
+    it = alternate_protocol_map_.Put(server, alternate);
   }
   it->second.is_broken = true;
-  int count = ++broken_alternate_protocol_map_[server];
+  const BrokenAlternateProtocolEntry entry(server, alternate.port,
+                                           alternate.protocol);
+  int count = ++broken_alternate_protocol_map_[entry];
   base::TimeDelta delay =
       base::TimeDelta::FromSeconds(kBrokenAlternateProtocolDelaySecs);
-  BrokenAlternateProtocolEntry entry;
-  entry.server = server;
-  entry.when = base::TimeTicks::Now() + delay * (1 << (count - 1));
-  broken_alternate_protocol_list_.push_back(entry);
+  base::TimeTicks when = base::TimeTicks::Now() + delay * (1 << (count - 1));
+  broken_alternate_protocol_list_.push_back(
+      BrokenAlternateProtocolEntryWithTime(entry, when));
 
   // Do not leave this host as canonical so that we don't infer the other
   // hosts are also broken without testing them first.
@@ -354,12 +356,22 @@ void HttpServerPropertiesImpl::SetBrokenAlternateProtocol(
 
 bool HttpServerPropertiesImpl::WasAlternateProtocolRecentlyBroken(
     const HostPortPair& server) {
-  return ContainsKey(broken_alternate_protocol_map_, server);
+  if (!HasAlternateProtocol(server))

[Ryan Hamilton, 2015/02/03 22:39:35]

Looks like this needs to be rebased on top of your other CL.

[Bence, 2015/02/04 14:25:33]

Done.

+    return false;
+  const AlternateProtocolInfo alternate_protocol = GetAlternateProtocol(server);
+  const BrokenAlternateProtocolEntry entry(server, alternate_protocol.port,
+                                           alternate_protocol.protocol);
+  return ContainsKey(broken_alternate_protocol_map_, entry);
 }
 
 void HttpServerPropertiesImpl::ConfirmAlternateProtocol(
     const HostPortPair& server) {
-  broken_alternate_protocol_map_.erase(server);
+  if (!HasAlternateProtocol(server))
+    return;
+  const AlternateProtocolInfo alternate_protocol = GetAlternateProtocol(server);
+  const BrokenAlternateProtocolEntry entry(server, alternate_protocol.port,
+                                           alternate_protocol.protocol);

[Ryan Hamilton, 2015/02/03 22:39:35]

might consider adding a 2 arg constructor(server, alternate protocol info) since
that's basically what's used each time.

[Bence, 2015/02/04 14:25:33]

I was thinking a lot about it, but it turns out AlternateProtocolInfo includes
probability, which we do not want to have in BrokenAlternateProtocolEntry. 
Therefore I decided against it.

I might define an AlternativeService struct down the road with (protocol, host,
port) triple, and might make AlternateServiceInfo have one of those as a member.
 We'll need to see if GetAlternativeServices, SetAlternativeServiceBroken etc.
should take the (protocol, host, port) triple as a single struct, or as three
separate arguments.  But that's gonna be another CL...

+  broken_alternate_protocol_map_.erase(entry);
 }
 
 void HttpServerPropertiesImpl::ClearAlternateProtocol(
@@ -513,12 +525,14 @@ void HttpServerPropertiesImpl::RemoveCanonicalHost(
 void HttpServerPropertiesImpl::ExpireBrokenAlternateProtocolMappings() {
   base::TimeTicks now = base::TimeTicks::Now();
   while (!broken_alternate_protocol_list_.empty()) {
-    BrokenAlternateProtocolEntry entry =
+    BrokenAlternateProtocolEntryWithTime entry_with_time =
         broken_alternate_protocol_list_.front();
-    if (now < entry.when) {
+    if (now < entry_with_time.when) {
       break;
     }
 
+    const BrokenAlternateProtocolEntry& entry =
+        entry_with_time.broken_alternate_protocol_entry;
     ClearAlternateProtocol(entry.server);

[Ryan Hamilton, 2015/02/03 22:39:35]

I think this method should now that a BrokenAPEntry instead of just a server,
right?

[Bence, 2015/02/04 14:25:33]

A server can still only have one AlternateProtocolInfo, so I think the change
you propose does not belong to this CL quite yet.  But you are right, it will
come as soon as I introduce multiple alternate protocols per server.

[Ryan Hamilton, 2015/02/05 17:32:24]

Ah,yes, good point.

     broken_alternate_protocol_list_.pop_front();
   }