Add IsAlternativeServiceBroken(), remove is_broken.

net/http/http_server_properties_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_server_properties_impl.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 35 matching lines @@
        it != spdy_servers->rend(); ++it) {
     spdy_servers_map_.Put(*it, support_spdy);
   }
 }
 
 void HttpServerPropertiesImpl::InitializeAlternateProtocolServers(
     AlternateProtocolMap* alternate_protocol_map) {
   // 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);
     }
   }
 
   // Add the entries from persisted data.
   for (AlternateProtocolMap::reverse_iterator it =
            alternate_protocol_map->rbegin();
        it != alternate_protocol_map->rend(); ++it) {
@@ skipping 181 matching lines @@
 
   AlternateProtocolInfo uninitialized_alternate_protocol;
   return uninitialized_alternate_protocol;
 }
 
 void HttpServerPropertiesImpl::SetAlternateProtocol(
     const HostPortPair& server,
     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()
                    << " from [Port: " << existing_alternate.port
                    << ", Protocol: " << existing_alternate.protocol
                    << ", Probability: " << existing_alternate.probability
                    << "] to [Port: " << alternate_port
                    << ", Protocol: " << alternate_protocol
                    << ", Probability: " << alternate_probability
                    << "].";
@@ skipping 28 matching lines @@
   if (it == alternate_protocol_map_.end()) {
     if (alternate.protocol == UNINITIALIZED_ALTERNATE_PROTOCOL) {
       LOG(DFATAL) << "Trying to mark unknown alternate protocol broken.";
       return;
     }
     // 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, alternate);
   }
-  it->second.is_broken = true;
   const AlternativeService alternative_service(alternate.protocol,
                                                server.host(), alternate.port);
   int count = ++recently_broken_alternative_services_[alternative_service];
   base::TimeDelta delay =
       base::TimeDelta::FromSeconds(kBrokenAlternateProtocolDelaySecs);
   base::TimeTicks when = base::TimeTicks::Now() + delay * (1 << (count - 1));
   auto result = broken_alternative_services_.insert(
       std::make_pair(alternative_service, when));
   // Return if alternative service is already in expiration queue.
   if (!result.second) {
     return;
   }
 
   // Do not leave this host as canonical so that we don't infer the other
   // hosts are also broken without testing them first.
   RemoveCanonicalHost(server);
 
   // If this is the only entry in the list, schedule an expiration task.
   // Otherwise it will be rescheduled automatically when the pending task runs.
   if (broken_alternative_services_.size() == 1) {
     ScheduleBrokenAlternateProtocolMappingsExpiration();
   }
 }
 
+bool HttpServerPropertiesImpl::IsAlternativeServiceBroken(
+    const AlternativeService& alternative_service) {
+  return broken_alternative_services_.find(alternative_service) !=

[Ryan Hamilton, 2015/03/10 21:05:46]

Can you do this instead:

return ContainsKey(broken_alternative_services_, alternative_server);?

(I'm not sure if ContainsKey works for linked_hash_map. If it does, let's use
it)

[Bence, 2015/03/10 21:46:13]

Done.

+         broken_alternative_services_.end();
+}
+
 bool HttpServerPropertiesImpl::WasAlternateProtocolRecentlyBroken(
     const HostPortPair& server) {
   const AlternateProtocolInfo alternate_protocol = GetAlternateProtocol(server);
   if (alternate_protocol.protocol == UNINITIALIZED_ALTERNATE_PROTOCOL)
     return false;
   const AlternativeService alternative_service(
       alternate_protocol.protocol, server.host(), alternate_protocol.port);
   return ContainsKey(recently_broken_alternative_services_,
                      alternative_service);
 }
 
 void HttpServerPropertiesImpl::ConfirmAlternateProtocol(
     const HostPortPair& server) {
   const AlternateProtocolInfo alternate_protocol = GetAlternateProtocol(server);
   if (alternate_protocol.protocol == UNINITIALIZED_ALTERNATE_PROTOCOL)
     return;
   const AlternativeService alternative_service(
       alternate_protocol.protocol, server.host(), alternate_protocol.port);
-  recently_broken_alternative_services_.erase(alternative_service);
+  ignore_result(broken_alternative_services_.erase(alternative_service));

[Ryan Hamilton, 2015/03/10 21:05:46]

is ignore_result required?

[Bence, 2015/03/10 21:46:13]

erase(Key) returns size_t, so it makes sense, but it's not marked
WARN_UNUSED_RESULT, so it's not required.   Do you want me to remove
ignore_result?

[Ryan Hamilton, 2015/03/10 22:15:26]

I think I'd remove it since we don't typically do this when we erase things from
containers.

[Bence, 2015/03/10 22:54:41]

Done.

+  ignore_result(
+      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));
 }
 
 const AlternateProtocolMap&
 HttpServerPropertiesImpl::alternate_protocol_map() const {
   return alternate_protocol_map_;
 }
 
 const SettingsMap& HttpServerPropertiesImpl::GetSpdySettings(
     const HostPortPair& host_port_pair) {
   SpdySettingsMap::iterator it = spdy_settings_map_.Get(host_port_pair);
@@ skipping 151 matching lines @@
   base::TimeDelta delay = when > now ? when - now : base::TimeDelta();
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
       base::Bind(
           &HttpServerPropertiesImpl::ExpireBrokenAlternateProtocolMappings,
           weak_ptr_factory_.GetWeakPtr()),
       delay);
 }
 
 }  // namespace net