Added stats on why blacklisted requests were blacklisted.

net/base/sdch_manager.cc

Index: net/base/sdch_manager.cc
diff --git a/net/base/sdch_manager.cc b/net/base/sdch_manager.cc
index 8892c13a7c46013467b1b249515a8157bab7ed9f..104d2e69146dc648c1c211d8c5ca0b5e8d4f8042 100644
--- a/net/base/sdch_manager.cc
+++ b/net/base/sdch_manager.cc
@@ -232,7 +232,6 @@ SdchManager::~SdchManager() {
 
 void SdchManager::ClearData() {
   blacklisted_domains_.clear();
-  exponential_blacklist_count_.clear();
   allow_latency_experiment_.clear();
   if (fetcher_.get())
     fetcher_->Cancel();
@@ -264,51 +263,62 @@ void SdchManager::EnableSecureSchemeSupport(bool enabled) {
   g_secure_scheme_supported_ = enabled;
 }
 
-void SdchManager::BlacklistDomain(const GURL& url) {
+void SdchManager::BlacklistDomain(const GURL& url,
+                                  ProblemCodes blacklist_reason) {
   SetAllowLatencyExperiment(url, false);
 
   std::string domain(StringToLowerASCII(url.host()));
-  int count = blacklisted_domains_[domain];
-  if (count > 0)
+  BlacklistInfo* blacklist_info = &blacklisted_domains_[domain];
+
+  if (blacklist_info->count > 0)
     return;  // Domain is already blacklisted.
 
-  count = 1 + 2 * exponential_blacklist_count_[domain];
+  int count = 1 + 2 * blacklist_info->exponential_count;
   if (count > 0)
-    exponential_blacklist_count_[domain] = count;
+    blacklist_info->exponential_count = count;
   else
-    count = INT_MAX;
+    blacklist_info->exponential_count = INT_MAX;
 
-  blacklisted_domains_[domain] = count;
+  blacklist_info->count = blacklist_info->exponential_count;
+  blacklist_info->reason = blacklist_reason;
 }
 
-void SdchManager::BlacklistDomainForever(const GURL& url) {
+void SdchManager::BlacklistDomainForever(const GURL& url,
+                                         ProblemCodes blacklist_reason) {
   SetAllowLatencyExperiment(url, false);
 
   std::string domain(StringToLowerASCII(url.host()));
-  exponential_blacklist_count_[domain] = INT_MAX;
-  blacklisted_domains_[domain] = INT_MAX;
+  BlacklistInfo* blacklist_info = &blacklisted_domains_[domain];
+  blacklist_info->count = INT_MAX;
+  blacklist_info->exponential_count = INT_MAX;
+  blacklist_info->reason = blacklist_reason;
 }
 
 void SdchManager::ClearBlacklistings() {
   blacklisted_domains_.clear();
-  exponential_blacklist_count_.clear();
 }
 
 void SdchManager::ClearDomainBlacklisting(const std::string& domain) {
-  blacklisted_domains_.erase(StringToLowerASCII(domain));
+  BlacklistInfo* blacklist_info = &blacklisted_domains_[
+      StringToLowerASCII(domain)];
+  blacklist_info->count = 0;
+  blacklist_info->reason = MIN_PROBLEM_CODE;

[jar (doing other things), 2014/08/05 23:33:30]

Why did you change to leaving the entry in the map, rather than erasing it??

[Randy Smith (Not in Mondays), 2014/08/11 20:46:39]

Because the map now contains the exponential backoff information, which is
needed for the next time the domain is blacklisted.

 }
 
 int SdchManager::BlackListDomainCount(const std::string& domain) {
-  if (blacklisted_domains_.end() == blacklisted_domains_.find(domain))
+  std::string domain_lower(StringToLowerASCII(domain));

[jar (doing other things), 2014/08/05 23:33:30]

Good bug fix! Old code assumed it was already lower.

nit: Perhaps you should rename |domain| to |domain_lower| in other methods
before use as an index?

[Randy Smith (Not in Mondays), 2014/08/11 20:46:39]

Done (or got rid of any variable at all), everywhere but in the middle of
AddSdchDictionary, where it's used in a couple of different ways, not just as an
index.

+
+  if (blacklisted_domains_.end() == blacklisted_domains_.find(domain_lower))
     return 0;
-  return blacklisted_domains_[StringToLowerASCII(domain)];
+  return blacklisted_domains_[domain_lower].count;
 }
 
 int SdchManager::BlacklistDomainExponential(const std::string& domain) {
-  if (exponential_blacklist_count_.end() ==
-      exponential_blacklist_count_.find(domain))
+  std::string domain_lower(StringToLowerASCII(domain));
+
+  if (blacklisted_domains_.end() == blacklisted_domains_.find(domain_lower))
     return 0;
-  return exponential_blacklist_count_[StringToLowerASCII(domain)];
+  return blacklisted_domains_[domain_lower].exponential_count;
 }
 
 bool SdchManager::IsInSupportedDomain(const GURL& url) {
@@ -323,15 +333,20 @@ bool SdchManager::IsInSupportedDomain(const GURL& url) {
     return true;
 
   std::string domain(StringToLowerASCII(url.host()));
-  DomainCounter::iterator it = blacklisted_domains_.find(domain);
-  if (blacklisted_domains_.end() == it)
+  DomainBlacklistInfo::iterator it = blacklisted_domains_.find(domain);
+  if (blacklisted_domains_.end() == it || it->second.count == 0)
     return true;
 
-  int count = it->second - 1;
-  if (count > 0)
-    blacklisted_domains_[domain] = count;
-  else
-    blacklisted_domains_.erase(domain);
+  int count = it->second.count - 1;
+  if (count > 0) {
+    it->second.count = count;
+  } else {
+    it->second.count = 0;
+    it->second.reason = MIN_PROBLEM_CODE;
+  }
+
+  UMA_HISTOGRAM_ENUMERATION("Sdch3.BlacklistReason", it->second.reason,

[jar (doing other things), 2014/08/05 23:33:30]

Maybe this should go before line 343, since it may otherwise be cleared in line
345.  ...or is this useful in that context?

[Randy Smith (Not in Mondays), 2014/08/11 20:46:39]

Ooops; thanks for the catch.  No, I think we still want the error code on the
last rejection.

+                            MAX_PROBLEM_CODE);
   SdchErrorRecovery(DOMAIN_BLACKLIST_INCLUDES_TARGET);
   return false;
 }