Limit exponential backoff for broken alternative services.

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 f0489c8698671be4b62107d75724afec2fc16a8d..ceffabb2c457c82c861f1265a3ca3fff5c4bcc7d 100644
--- a/net/http/http_server_properties_impl.cc
+++ b/net/http/http_server_properties_impl.cc
@@ -23,7 +23,9 @@ namespace net {
 
 namespace {
 
+// kBrokenAlternativeProtocolDelaySecs << kBrokenDelayMaxShift ~ 2 days.

[Ryan Hamilton, 2016/11/03 16:16:22]

nit: I'm not sure what this comment means? Perhaps a static_assert?

[Bence, 2016/11/07 20:17:24]

Done.

 const uint64_t kBrokenAlternativeProtocolDelaySecs = 300;
+const int kBrokenDelayMaxShift = 9;
 
 }  // namespace
 
@@ -472,10 +474,13 @@ void HttpServerPropertiesImpl::MarkAlternativeServiceBroken(
     LOG(DFATAL) << "Trying to mark unknown alternate protocol broken.";
     return;
   }
-  int count = ++recently_broken_alternative_services_[alternative_service];
+  ++recently_broken_alternative_services_[alternative_service];
+  int shift = recently_broken_alternative_services_[alternative_service] - 1;

[Zhongyi Shi, 2016/11/03 15:59:37]

typo here?

Since you're directly retrieving shift, I think what you need here is:
int shift = recently_broken_alternative_services_[alternative_service] ; 
as in the old code, the prefix increment happens before the assignment. Sorry
for not catching this in the first place. 

[Bence, 2016/11/07 20:17:24]

Indeed prefix increment returns new value.  And |count| has the new value in the
old code.  And |shift = count - 1|.  There's many different ways of writing it. 
For example, I can write:

int count = ++r[a];
int shift = count - 1;

Or I can write

int shift = r[a]++;

Or I can write

int shift = r[a];
++r[a];

Or I can write

++r[a];
int shift = r[a] - 1;

These are all equivalent, and I think this is the functionality that want.  Do
you have any preference?  Or I can leave everything as is, and not introduce
|shift|, but then it's not very clear how |count| interacts with
|kBrokenDelayMaxShift| (one is count, the other is shift, and they are off by
one).

+  if (shift > kBrokenDelayMaxShift)
+    shift = kBrokenDelayMaxShift;
   base::TimeDelta delay =
       base::TimeDelta::FromSeconds(kBrokenAlternativeProtocolDelaySecs);
-  base::TimeTicks when = base::TimeTicks::Now() + delay * (1 << (count - 1));
+  base::TimeTicks when = base::TimeTicks::Now() + delay * (1 << shift);
   auto result = broken_alternative_services_.insert(
       std::make_pair(alternative_service, when));
   // Return if alternative service is already in expiration queue.