Add IsAlternativeServiceBroken(), remove is_broken.

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 11a48ab3f183eeb9419bf07c83aab45736210602..b0a0f3882741ddfe3c6876b9c1216fa79c32ba6c 100644
--- a/net/http/http_server_properties_impl.cc
+++ b/net/http/http_server_properties_impl.cc
@@ -53,7 +53,9 @@ void HttpServerPropertiesImpl::InitializeAlternateProtocolServers(
   // Keep all the broken ones since those don't get persisted.
   for (AlternateProtocolMap::iterator it = alternate_protocol_map_.begin();
        it != alternate_protocol_map_.end();) {
-    if (it->second.is_broken) {
+    const AlternativeService alternative_service(
+        it->second.protocol, it->first.host(), it->second.port);
+    if (IsAlternativeServiceBroken(alternative_service)) {
       ++it;
     } else {
       it = alternate_protocol_map_.Erase(it);
@@ -255,20 +257,20 @@ void HttpServerPropertiesImpl::SetAlternateProtocol(
     uint16 alternate_port,
     AlternateProtocol alternate_protocol,
     double alternate_probability) {
+  const AlternativeService alternative_service(alternate_protocol,
+                                               server.host(), alternate_port);
+  if (IsAlternativeServiceBroken(alternative_service)) {
+    DVLOG(1) << "Ignore alternative service since it is known to be broken.";
+    return;
+  }
 
-  AlternateProtocolInfo alternate(alternate_port,
-                                  alternate_protocol,
-                                  alternate_probability);
+  const AlternateProtocolInfo alternate(alternate_port, alternate_protocol,
+                                        alternate_probability);
   AlternateProtocolMap::const_iterator it =
       GetAlternateProtocolIterator(server);
   if (it != alternate_protocol_map_.end()) {
     const AlternateProtocolInfo existing_alternate = it->second;
 
-    if (existing_alternate.is_broken) {
-      DVLOG(1) << "Ignore alternate protocol since it's known to be broken.";
-      return;
-    }
-
     if (!existing_alternate.Equals(alternate)) {
       LOG(WARNING) << "Changing the alternate protocol for: "
                    << server.ToString()
@@ -317,7 +319,6 @@ void HttpServerPropertiesImpl::SetBrokenAlternateProtocol(
     // it can be marked as broken.
     it = alternate_protocol_map_.Put(server, alternate);
   }
-  it->second.is_broken = true;
   const AlternativeService alternative_service(alternate.protocol,
                                                server.host(), alternate.port);
   int count = ++recently_broken_alternative_services_[alternative_service];
@@ -342,6 +343,11 @@ void HttpServerPropertiesImpl::SetBrokenAlternateProtocol(
   }
 }
 
+bool HttpServerPropertiesImpl::IsAlternativeServiceBroken(
+    const AlternativeService& alternative_service) {
+  return ContainsKey(broken_alternative_services_, alternative_service);
+}
+
 bool HttpServerPropertiesImpl::WasAlternateProtocolRecentlyBroken(
     const HostPortPair& server) {
   const AlternateProtocolInfo alternate_protocol = GetAlternateProtocol(server);
@@ -360,16 +366,32 @@ void HttpServerPropertiesImpl::ConfirmAlternateProtocol(
     return;
   const AlternativeService alternative_service(
       alternate_protocol.protocol, server.host(), alternate_protocol.port);
+  broken_alternative_services_.erase(alternative_service);
   recently_broken_alternative_services_.erase(alternative_service);
 }
 
 void HttpServerPropertiesImpl::ClearAlternateProtocol(
     const HostPortPair& server) {
+  RemoveCanonicalHost(server);
+
   AlternateProtocolMap::iterator it = alternate_protocol_map_.Peek(server);
-  if (it != alternate_protocol_map_.end())
-    alternate_protocol_map_.Erase(it);
+  if (it == alternate_protocol_map_.end()) {
+    return;
+  }
+  const AlternativeService alternative_service(
+      it->second.protocol, it->first.host(), it->second.port);
+  alternate_protocol_map_.Erase(it);
 
-  RemoveCanonicalHost(server);
+  // The following is temporary to keep the existing semantics, which is that if
+  // there is a broken alternative service in the mapping, then this method
+  // leaves it in a non-broken, but recently broken state.
+  //
+  // TODO(bnc):
+  //  1. Verify and document the class invariant that no broken alternative
+  //     service can be in the mapping.
+  //  2. Remove the rest of this method as it will be moot.
+  //  3. Provide a SetAlternativeServiceRecentlyBroken if necessary.
+  ignore_result(broken_alternative_services_.erase(alternative_service));

[Ryan Hamilton, 2015/03/12 22:49:05]

As long as you're here, can you remove ignore_result()

[Bence, 2015/03/12 22:54:11]

Done.

 }
 
 const AlternateProtocolMap&
@@ -516,10 +538,10 @@ void HttpServerPropertiesImpl::ExpireBrokenAlternateProtocolMappings() {
       break;
     }
 
-    const AlternativeService& alternative_service = it->first;
+    const AlternativeService alternative_service = it->first;
+    broken_alternative_services_.erase(it);
     ClearAlternateProtocol(
         HostPortPair(alternative_service.host, alternative_service.port));
-    broken_alternative_services_.erase(it);
   }
   ScheduleBrokenAlternateProtocolMappingsExpiration();
 }