Lazy RemoveCanonicalHost().

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 307 matching lines @@
   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(origin);
-
   // 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();
   }
 }
 
 void HttpServerPropertiesImpl::MarkAlternativeServiceRecentlyBroken(
     const AlternativeService& alternative_service) {
   if (!ContainsKey(recently_broken_alternative_services_, alternative_service))
@@ skipping 141 matching lines @@
 void HttpServerPropertiesImpl::SetAlternateProtocolProbabilityThreshold(
     double threshold) {
   alternate_protocol_probability_threshold_ = threshold;
 }
 
 AlternateProtocolMap::const_iterator
 HttpServerPropertiesImpl::GetAlternateProtocolIterator(
     const HostPortPair& server) {
   AlternateProtocolMap::const_iterator it = alternate_protocol_map_.Get(server);
   if (it != alternate_protocol_map_.end())
     return it;

[Ryan Hamilton, 2015/03/18 22:44:36]

Do you need to (possibly) do the removal here too?

[Bence, 2015/03/19 12:44:02]

It's not necessary.  Consider foo being the canonical host for bar.  If foo has
an alternative service, and we detect it to be broken, and later we query the
alternative service of foo, we might or might not RemoveCanonicalHost(foo), but
we get back the broken alternative service in either case.  When we query the
alternative service of bar, the codepath below gets executed, foo is removed
from the canonical host list, and no alternative service is returned,
irrespectively from whether foo was removed from the canonical host list earlier
or not.

Since most hosts are not canonical hosts, I think it's better not to add a call
to RemoveCanonicalHost here.

[Ryan Hamilton, 2015/03/19 20:05:18]

Ah, I see. Ok, do we have a test which covers this case? foo is canonical for
bar, foo is broken. GetAlternateProtocol for bar does not return the broken
entry?

[Bence, 2015/03/19 21:43:09]

Yes, AlternateProtocolServerPropertiesTest.CanonicalBroken does.

 
   CanonicalHostMap::const_iterator canonical = GetCanonicalHost(server);
-  if (canonical != canonical_host_to_origin_map_.end())
-    return alternate_protocol_map_.Get(canonical->second);
+  if (canonical == canonical_host_to_origin_map_.end()) {
+    return alternate_protocol_map_.end();
+  }
 
+  const HostPortPair canonical_host_port = canonical->second;
+  it = alternate_protocol_map_.Get(canonical_host_port);
+  if (it == alternate_protocol_map_.end()) {
+    return alternate_protocol_map_.end();
+  }
+
+  const AlternativeService alternative_service(
+      it->second.protocol, canonical_host_port.host(), it->second.port);
+  if (!IsAlternativeServiceBroken(alternative_service)) {
+    return it;
+  }
+
+  RemoveCanonicalHost(canonical_host_port);
   return alternate_protocol_map_.end();
 }
 
 HttpServerPropertiesImpl::CanonicalHostMap::const_iterator
 HttpServerPropertiesImpl::GetCanonicalHost(HostPortPair server) const {
   for (size_t i = 0; i < canonical_suffixes_.size(); ++i) {
     std::string canonical_suffix = canonical_suffixes_[i];
     if (EndsWith(server.host(), canonical_suffixes_[i], false)) {
       HostPortPair canonical_host(canonical_suffix, server.port());
       return canonical_host_to_origin_map_.find(canonical_host);
@@ skipping 42 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