Sdch view for net-internals

net/base/sdch_manager.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/base/sdch_manager.h"
 
 #include "base/base64.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
+#include "base/values.h"
 #include "crypto/sha2.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
+#include "net/base/sdch_net_log_params.h"

[Randy Smith (Not in Mondays), 2014/10/02 21:28:45]

By the compartmentalization I'm reaching for, this shouldn't be needed anymore?

 #include "net/url_request/url_request_http_job.h"
 
 namespace {
 
 void StripTrailingDot(GURL* gurl) {
   std::string host(gurl->host());
 
   if (host.empty())
     return;
 
@@ skipping 46 matching lines @@
       url_(gurl),
       domain_(domain),
       path_(path),
       expiration_(expiration),
       ports_(ports) {
 }
 
 SdchManager::Dictionary::~Dictionary() {
 }
 
-bool SdchManager::Dictionary::CanAdvertise(const GURL& target_url) {
+SdchProblemCodes SdchManager::Dictionary::CanAdvertise(
+    const GURL& target_url) const {
   /* The specific rules of when a dictionary should be advertised in an
      Avail-Dictionary header are modeled after the rules for cookie scoping. The
      terms "domain-match" and "pathmatch" are defined in RFC 2965 [6]. A
      dictionary may be advertised in the Avail-Dictionaries header exactly when
      all of the following are true:
       1. The server's effective host name domain-matches the Domain attribute of
          the dictionary.
       2. If the dictionary has a Port attribute, the request port is one of the
          ports listed in the Port attribute.
       3. The request URI path-matches the path header of the dictionary.
       4. The request is not an HTTPS request.
      We can override (ignore) item (4) only when we have explicitly enabled
      HTTPS support AND the dictionary acquisition scheme matches the target
      url scheme.
     */
   if (!DomainMatch(target_url, domain_))
-    return false;
+    return SDCH_DICTIONARY_FOUND_HAS_WRONG_DOMAIN;
   if (!ports_.empty() && 0 == ports_.count(target_url.EffectiveIntPort()))
-    return false;
+    return SDCH_DICTIONARY_FOUND_HAS_WRONG_PORT_LIST;
   if (path_.size() && !PathMatch(target_url.path(), path_))
-    return false;
+    return SDCH_DICTIONARY_FOUND_HAS_WRONG_PATH;
   if (!SdchManager::secure_scheme_supported() && target_url.SchemeIsSecure())
-    return false;
+    return SDCH_DICTIONARY_FOUND_HAS_WRONG_SCHEME;
   if (target_url.SchemeIsSecure() != url_.SchemeIsSecure())
-    return false;
+    return SDCH_DICTIONARY_FOUND_HAS_WRONG_SCHEME;
   if (base::Time::Now() > expiration_)
-    return false;
-  return true;
+    return SDCH_DICTIONARY_FOUND_EXPIRED;
+  return SDCH_OK;
 }
 
 //------------------------------------------------------------------------------
 // Security functions restricting loads and use of dictionaries.
 
 // static
-bool SdchManager::Dictionary::CanSet(const std::string& domain,
-                                     const std::string& path,
-                                     const std::set<int>& ports,
-                                     const GURL& dictionary_url) {
+SdchProblemCodes SdchManager::Dictionary::CanSet(const std::string& domain,
+                                                 const std::string& path,
+                                                 const std::set<int>& ports,
+                                                 const GURL& dictionary_url) {
   /*
   A dictionary is invalid and must not be stored if any of the following are
   true:
     1. The dictionary has no Domain attribute.
     2. The effective host name that derives from the referer URL host name does
       not domain-match the Domain attribute.
     3. The Domain attribute is a top level domain.
     4. The referer URL host is a host domain name (not IP address) and has the
       form HD, where D is the value of the Domain attribute, and H is a string
       that contains one or more dots.
     5. If the dictionary has a Port attribute and the referer URL's port was not
       in the list.
   */
 
   // TODO(jar): Redirects in dictionary fetches might plausibly be problematic,
   // and hence the conservative approach is to not allow any redirects (if there
   // were any... then don't allow the dictionary to be set).
 
-  if (domain.empty()) {
-    SdchErrorRecovery(DICTIONARY_MISSING_DOMAIN_SPECIFIER);
-    return false;  // Domain is required.
-  }
+  if (domain.empty())
+    return SDCH_DICTIONARY_MISSING_DOMAIN_SPECIFIER;  // Domain is required.
+
   if (registry_controlled_domains::GetDomainAndRegistry(
-        domain,
-        registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES).empty()) {
-    SdchErrorRecovery(DICTIONARY_SPECIFIES_TOP_LEVEL_DOMAIN);
-    return false;  // domain was a TLD.
-  }
-  if (!Dictionary::DomainMatch(dictionary_url, domain)) {
-    SdchErrorRecovery(DICTIONARY_DOMAIN_NOT_MATCHING_SOURCE_URL);
-    return false;
-  }
+          domain, registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)
+          .empty())
+    return SDCH_DICTIONARY_SPECIFIES_TOP_LEVEL_DOMAIN;  // domain was a TLD.

[mmenke, 2014/10/03 18:59:27]

nit:  Should use braces when the conditional takes up multiple lines.

[baranovich, 2014/10/27 20:49:24]

Done.

+
+  if (!Dictionary::DomainMatch(dictionary_url, domain))
+    return SDCH_DICTIONARY_DOMAIN_NOT_MATCHING_SOURCE_URL;
 
   std::string referrer_url_host = dictionary_url.host();
   size_t postfix_domain_index = referrer_url_host.rfind(domain);
   // See if it is indeed a postfix, or just an internal string.
   if (referrer_url_host.size() == postfix_domain_index + domain.size()) {
     // It is a postfix... so check to see if there's a dot in the prefix.
     size_t end_of_host_index = referrer_url_host.find_first_of('.');
-    if (referrer_url_host.npos != end_of_host_index  &&
-        end_of_host_index < postfix_domain_index) {
-      SdchErrorRecovery(DICTIONARY_REFERER_URL_HAS_DOT_IN_PREFIX);
-      return false;
-    }
+    if (referrer_url_host.npos != end_of_host_index &&
+        end_of_host_index < postfix_domain_index)
+      return SDCH_DICTIONARY_REFERER_URL_HAS_DOT_IN_PREFIX;

[mmenke, 2014/10/03 18:59:27]

nit:  Use braces

[baranovich, 2014/10/27 20:49:24]

Done.

   }
 
-  if (!ports.empty()
-      && 0 == ports.count(dictionary_url.EffectiveIntPort())) {
-    SdchErrorRecovery(DICTIONARY_PORT_NOT_MATCHING_SOURCE_URL);
-    return false;
-  }
-  return true;
+  if (!ports.empty() && 0 == ports.count(dictionary_url.EffectiveIntPort()))
+    return SDCH_DICTIONARY_PORT_NOT_MATCHING_SOURCE_URL;
+
+  return SDCH_OK;
 }
 
-// static
-bool SdchManager::Dictionary::CanUse(const GURL& referring_url) {
+SdchProblemCodes SdchManager::Dictionary::CanUse(
+    const GURL& referring_url) const {
   /*
     1. The request URL's host name domain-matches the Domain attribute of the
       dictionary.
     2. If the dictionary has a Port attribute, the request port is one of the
       ports listed in the Port attribute.
     3. The request URL path-matches the path attribute of the dictionary.
     4. The request is not an HTTPS request.
     We can override (ignore) item (4) only when we have explicitly enabled
     HTTPS support AND the dictionary acquisition scheme matches the target
      url scheme.
   */
-  if (!DomainMatch(referring_url, domain_)) {
-    SdchErrorRecovery(DICTIONARY_FOUND_HAS_WRONG_DOMAIN);
-    return false;
-  }
-  if (!ports_.empty()
-      && 0 == ports_.count(referring_url.EffectiveIntPort())) {
-    SdchErrorRecovery(DICTIONARY_FOUND_HAS_WRONG_PORT_LIST);
-    return false;
-  }
-  if (path_.size() && !PathMatch(referring_url.path(), path_)) {
-    SdchErrorRecovery(DICTIONARY_FOUND_HAS_WRONG_PATH);
-    return false;
-  }
-  if (!SdchManager::secure_scheme_supported() &&
-      referring_url.SchemeIsSecure()) {
-    SdchErrorRecovery(DICTIONARY_FOUND_HAS_WRONG_SCHEME);
-    return false;
-  }
-  if (referring_url.SchemeIsSecure() != url_.SchemeIsSecure()) {
-    SdchErrorRecovery(DICTIONARY_FOUND_HAS_WRONG_SCHEME);
-    return false;
-  }
+  if (!DomainMatch(referring_url, domain_))
+    return SDCH_DICTIONARY_FOUND_HAS_WRONG_DOMAIN;
+
+  if (!ports_.empty() && 0 == ports_.count(referring_url.EffectiveIntPort()))
+    return SDCH_DICTIONARY_FOUND_HAS_WRONG_PORT_LIST;
+
+  if (path_.size() && !PathMatch(referring_url.path(), path_))
+    return SDCH_DICTIONARY_FOUND_HAS_WRONG_PATH;
+
+  if (!SdchManager::secure_scheme_supported() && referring_url.SchemeIsSecure())
+    return SDCH_DICTIONARY_FOUND_HAS_WRONG_SCHEME;
+
+  if (referring_url.SchemeIsSecure() != url_.SchemeIsSecure())
+    return SDCH_DICTIONARY_FOUND_HAS_WRONG_SCHEME;
 
   // TODO(jar): Remove overly restrictive failsafe test (added per security
   // review) when we have a need to be more general.
-  if (!referring_url.SchemeIsHTTPOrHTTPS()) {
-    SdchErrorRecovery(ATTEMPT_TO_DECODE_NON_HTTP_DATA);
-    return false;
-  }
+  if (!referring_url.SchemeIsHTTPOrHTTPS())
+    return SDCH_ATTEMPT_TO_DECODE_NON_HTTP_DATA;
 
-  return true;
+  return SDCH_OK;
 }
 
+// static
 bool SdchManager::Dictionary::PathMatch(const std::string& path,
                                         const std::string& restriction) {
   /*  Must be either:
   1. P2 is equal to P1
   2. P2 is a prefix of P1 and either the final character in P2 is "/" or the
       character following P2 in P1 is "/".
       */
   if (path == restriction)
     return true;
   size_t prefix_length = restriction.size();
@@ skipping 32 matching lines @@
     fetcher_->Cancel();
 
   // Note that this may result in not having dictionaries we've advertised
   // for incoming responses.  The window is relatively small (as ClearData()
   // is not expected to be called frequently), so we rely on meta-refresh
   // to handle this case.
   dictionaries_.clear();
 }
 
 // static
-void SdchManager::SdchErrorRecovery(ProblemCodes problem) {
-  UMA_HISTOGRAM_ENUMERATION("Sdch3.ProblemCodes_4", problem, MAX_PROBLEM_CODE);
+void SdchManager::SdchErrorRecovery(SdchProblemCodes problem) {
+  UMA_HISTOGRAM_ENUMERATION(
+      "Sdch3.ProblemCodes_4", problem, SDCH_MAX_PROBLEM_CODE);
 }
 
 void SdchManager::set_sdch_fetcher(scoped_ptr<SdchFetcher> fetcher) {
   DCHECK(CalledOnValidThread());
   fetcher_ = fetcher.Pass();
 }
 
 // static
 void SdchManager::EnableSdchSupport(bool enabled) {
   g_sdch_enabled_ = enabled;
 }
 
 // static
 void SdchManager::EnableSecureSchemeSupport(bool enabled) {
   g_secure_scheme_supported_ = enabled;
 }
 
 void SdchManager::BlacklistDomain(const GURL& url,
-                                  ProblemCodes blacklist_reason) {
+                                  SdchProblemCodes blacklist_reason) {
   SetAllowLatencyExperiment(url, false);
 
   BlacklistInfo* blacklist_info =
       &blacklisted_domains_[base::StringToLowerASCII(url.host())];
 
   if (blacklist_info->count > 0)
     return;  // Domain is already blacklisted.
 
   if (blacklist_info->exponential_count > (INT_MAX - 1) / 2) {
     blacklist_info->exponential_count = INT_MAX;
   } else {
     blacklist_info->exponential_count =
         blacklist_info->exponential_count * 2 + 1;
   }
 
   blacklist_info->count = blacklist_info->exponential_count;
   blacklist_info->reason = blacklist_reason;
 }
 
 void SdchManager::BlacklistDomainForever(const GURL& url,
-                                         ProblemCodes blacklist_reason) {
+                                         SdchProblemCodes blacklist_reason) {
   SetAllowLatencyExperiment(url, false);
 
   BlacklistInfo* blacklist_info =
       &blacklisted_domains_[base::StringToLowerASCII(url.host())];
   blacklist_info->count = INT_MAX;
   blacklist_info->exponential_count = INT_MAX;
   blacklist_info->reason = blacklist_reason;
 }
 
 void SdchManager::ClearBlacklistings() {
   blacklisted_domains_.clear();
 }
 
 void SdchManager::ClearDomainBlacklisting(const std::string& domain) {
   BlacklistInfo* blacklist_info = &blacklisted_domains_[
       base::StringToLowerASCII(domain)];
   blacklist_info->count = 0;
-  blacklist_info->reason = MIN_PROBLEM_CODE;
+  blacklist_info->reason = SDCH_OK;
 }
 
 int SdchManager::BlackListDomainCount(const std::string& domain) {
   std::string domain_lower(base::StringToLowerASCII(domain));
 
   if (blacklisted_domains_.end() == blacklisted_domains_.find(domain_lower))
     return 0;
   return blacklisted_domains_[domain_lower].count;
 }
 
 int SdchManager::BlacklistDomainExponential(const std::string& domain) {
   std::string domain_lower(base::StringToLowerASCII(domain));
 
   if (blacklisted_domains_.end() == blacklisted_domains_.find(domain_lower))
     return 0;
   return blacklisted_domains_[domain_lower].exponential_count;
 }
 
-bool SdchManager::IsInSupportedDomain(const GURL& url) {
+SdchProblemCodes SdchManager::IsInSupportedDomain(const GURL& url) {
   DCHECK(CalledOnValidThread());
   if (!g_sdch_enabled_ )
-    return false;
+    return SDCH_DISABLED;
 
   if (!secure_scheme_supported() && url.SchemeIsSecure())
-    return false;
+    return SDCH_SECURE_SCHEME_NOT_SUPPORTED;
 
   if (blacklisted_domains_.empty())
-    return true;
+    return SDCH_OK;
 
   DomainBlacklistInfo::iterator it =
       blacklisted_domains_.find(base::StringToLowerASCII(url.host()));
   if (blacklisted_domains_.end() == it || it->second.count == 0)
-    return true;
+    return SDCH_OK;
 
-  UMA_HISTOGRAM_ENUMERATION("Sdch3.BlacklistReason", it->second.reason,
-                            MAX_PROBLEM_CODE);
-  SdchErrorRecovery(DOMAIN_BLACKLIST_INCLUDES_TARGET);

[Randy Smith (Not in Mondays), 2014/10/02 21:28:45]

So I'm still confused and concerned by this refactor.  It looks as if you've
pulled the logging into some but not all of the calling tree (.e.g
URLRHJ::AddExtraHeaders ignores the return code from GetAvailDictionaryList,
which calls this, and SDF::DoCompleteHeaders logs everything except a subset of
return codes, and it calls AddSdchDictionary, which calls this).  So I don't see
how the BLACKLIST_INCLUDES_TARGET logging is unchanged across this CL, which is
what I thought we were going for with the most recent change.  And I also think
we're going to be changing the contexts in which the other error codes from this
routine get passed to SdchErrorRecovery().  

This is the only place that returns SDCH_BLACKLIST_INCLUDES_TARGET.  Given that,
I think the minimal code change (and hence lowest likelyhood of bugs) is to
leave this call to SdchErrorRecovery here, and make sure not to call
SdchErrorRecovery based on the return code from this function in the calling
tree.  I realize that this may be an almost impossible thing to do.

Alternatively, you could rev the Sdch3.ProblemCodes_4 histogram and go to town,
which might actually end up being the smallest code change.  I'm not ecstatic
about that just because I'll probably do another rev at some point in the
future, but truth to tell, it's not actually a horrible thing (either abstractly
or in terms of amount of coding), and I'm not coming up with a clean solution
other than that which fufills both your goals for this CL and my goal of not
having the histogram distribution change across this CL.

[baranovich, 2014/10/02 23:01:50]

No. that's possible, but I was not sure if it makes the code cleaner, given that
netlog event should still be logged. So I decided to do as I did. But I'll try
that approach.
I'm afraid I didn't got what you suggest here, my English sometimes is too bad(


Yes, unfortunately the code became pretty ugly =(
Below are some clarifications:
- means what happens now
+ means what this CL does

- URLRHJ::AddExtraHeaders may write one blacklist histogram by calling to
SM::IsInSupportedDomain.
+ Now this histogram is added by URLRHJ::AddExtraHeaders when it checks retval
of SM::IsInSupportedDomain. Also netlog event is logged. 

- URLRHJ::AddExtraHeaders may write lots of them by calling to
SM::GetAvailDictionaryList which calls SM::IsInSupportedDomain many times
+ Now these histograms are written by SM::GetAvailDictionaryList when it check
retvals from SM::IsInSupportedDomain. netlog events are not written. I guess
they won't be useful:)

- URLRHJ::NotifyHeadersComplete may write one blacklist histogram by calling to
SM::IsInSupportedDomain
+ Now this histogram is added by URLRHJ::NotifyHeadersComplete when it checks
retval of SM::IsInSupportedDomain. Also netlog event is logged.

- SDCHFilter::InitializeDictionary calls SM::GetVcdiffDictionary which calls to
SM::IsInSupportedDomain and may add one blacklist histogram
+SDCHFilter::InitializeDictionary will log this histogram after checking retval
of SM::GetVcdiffDictionary and add netlog event

- at last SM::AddSdchDictionary may write one blacklist histogram when calling
to SM::IsInSupportedDomain
+ Now SM::AddSdchDictionary returns error to SDF::DoCompleteRequest, which
writes blacklist histogram among other problem codes and adds netlog event.

[Randy Smith (Not in Mondays), 2014/10/06 19:24:57]

No, my apologies; I was writing too quickly.  What I mean is:
* Create a new histogram, Sdch3.ProblemCodes_5, and mark the old one obsolete
(tools/metrics/histograms/histograms.xml)
* Change the code to write to that histogram instead of the old one.
* Log to that histogram as makes sense for your new code, without worrying about
whether you're going to have the logging counts for different errors as the old
code did--since it's a new histogram, it can have different shapes.  

I think that'll be easier than what you've done, and produce cleaner code.  Does
that makes sense?
I have a hard time visualizing this without seeing the new code, but it doesn't
sound a lot cleaner.  What do you think of creating the new histogram as
sketched above?

+  UMA_HISTOGRAM_ENUMERATION(
+      "Sdch3.BlacklistReason", it->second.reason, SDCH_MAX_PROBLEM_CODE);
 
   int count = it->second.count - 1;
   if (count > 0) {
     it->second.count = count;
   } else {
     it->second.count = 0;
-    it->second.reason = MIN_PROBLEM_CODE;
+    it->second.reason = SDCH_OK;
   }
 
-  return false;
+  return SDCH_DOMAIN_BLACKLIST_INCLUDES_TARGET;
 }
 
-void SdchManager::FetchDictionary(const GURL& request_url,
-                                  const GURL& dictionary_url) {
+SdchProblemCodes SdchManager::FetchDictionary(const GURL& request_url,
+                                              const GURL& dictionary_url) {
   DCHECK(CalledOnValidThread());
-  if (CanFetchDictionary(request_url, dictionary_url) && fetcher_.get()) {
+  SdchProblemCodes rv = CanFetchDictionary(request_url, dictionary_url);
+  if (rv != SDCH_OK)
+    return rv;
+
+  if (fetcher_.get()) {
     ++fetches_count_for_testing_;
-    fetcher_->Schedule(dictionary_url);
+    if (fetcher_->Schedule(dictionary_url))
+      return SDCH_DICTIONARY_PREVIOUSLY_SCHEDULED_TO_DOWNLOAD;
   }
+
+  return SDCH_OK;
 }
 
-bool SdchManager::CanFetchDictionary(const GURL& referring_url,
-                                     const GURL& dictionary_url) const {
+SdchProblemCodes SdchManager::CanFetchDictionary(
+    const GURL& referring_url,
+    const GURL& dictionary_url) const {
   DCHECK(CalledOnValidThread());
   /* The user agent may retrieve a dictionary from the dictionary URL if all of
      the following are true:
        1 The dictionary URL host name matches the referrer URL host name and
            scheme.
        2 The dictionary URL host name domain matches the parent domain of the
            referrer URL host name
        3 The parent domain of the referrer URL host name is not a top level
            domain
        4 The dictionary URL is not an HTTPS URL.
    */
   // Item (1) above implies item (2).  Spec should be updated.
   // I take "host name match" to be "is identical to"
   if (referring_url.host() != dictionary_url.host() ||
-      referring_url.scheme() != dictionary_url.scheme()) {
-    SdchErrorRecovery(DICTIONARY_LOAD_ATTEMPT_FROM_DIFFERENT_HOST);
-    return false;
-  }
-  if (!secure_scheme_supported() && referring_url.SchemeIsSecure()) {
-    SdchErrorRecovery(DICTIONARY_SELECTED_FOR_SSL);
-    return false;
-  }
+      referring_url.scheme() != dictionary_url.scheme())
+    return SDCH_DICTIONARY_LOAD_ATTEMPT_FROM_DIFFERENT_HOST;
+
+  if (!secure_scheme_supported() && referring_url.SchemeIsSecure())
+    return SDCH_DICTIONARY_SELECTED_FOR_SSL;
 
   // TODO(jar): Remove this failsafe conservative hack which is more restrictive
   // than current SDCH spec when needed, and justified by security audit.
-  if (!referring_url.SchemeIsHTTPOrHTTPS()) {
-    SdchErrorRecovery(DICTIONARY_SELECTED_FROM_NON_HTTP);
-    return false;
-  }
+  if (!referring_url.SchemeIsHTTPOrHTTPS())
+    return SDCH_DICTIONARY_SELECTED_FROM_NON_HTTP;
 
-  return true;
+  return SDCH_OK;
 }
 
-void SdchManager::GetVcdiffDictionary(
+SdchProblemCodes SdchManager::GetVcdiffDictionary(
     const std::string& server_hash,
     const GURL& referring_url,
     scoped_refptr<Dictionary>* dictionary) {
   DCHECK(CalledOnValidThread());
   *dictionary = NULL;
   DictionaryMap::iterator it = dictionaries_.find(server_hash);
-  if (it == dictionaries_.end()) {
-    return;
-  }
+  if (it == dictionaries_.end())
+    return SDCH_OK;  // not a problem. Just no dictionary.
+
   scoped_refptr<Dictionary> matching_dictionary = it->second;
-  if (!IsInSupportedDomain(referring_url))
-    return;
-  if (!matching_dictionary->CanUse(referring_url))
-    return;
-  *dictionary = matching_dictionary;
+
+  SdchProblemCodes rv = IsInSupportedDomain(referring_url);
+  if (rv != SDCH_OK)
+    return rv;
+
+  rv = matching_dictionary->CanUse(referring_url);
+  if (rv == SDCH_OK)
+    *dictionary = matching_dictionary;
+  return rv;
 }
 
 // TODO(jar): If we have evictions from the dictionaries_, then we need to
 // change this interface to return a list of reference counted Dictionary
 // instances that can be used if/when a server specifies one.
 void SdchManager::GetAvailDictionaryList(const GURL& target_url,
                                          std::string* list) {
   DCHECK(CalledOnValidThread());
   int count = 0;
   for (DictionaryMap::iterator it = dictionaries_.begin();
        it != dictionaries_.end(); ++it) {
-    if (!IsInSupportedDomain(target_url))
+    SdchProblemCodes rv = IsInSupportedDomain(target_url);
+    if (rv != SDCH_OK) {
+      if (rv == SDCH_DOMAIN_BLACKLIST_INCLUDES_TARGET)
+        SdchErrorRecovery(rv);
       continue;
-    if (!it->second->CanAdvertise(target_url))
+    }
+    if (it->second->CanAdvertise(target_url) != SDCH_OK)
       continue;
     ++count;
     if (!list->empty())
       list->append(",");
     list->append(it->second->client_hash());
   }
   // Watch to see if we have corrupt or numerous dictionaries.
   if (count > 0)
     UMA_HISTOGRAM_COUNTS("Sdch3.Advertisement_Count", count);
 }
@@ skipping 24 matching lines @@
 
 void SdchManager::SetAllowLatencyExperiment(const GURL& url, bool enable) {
   DCHECK(CalledOnValidThread());
   if (enable) {
     allow_latency_experiment_.insert(url.host());
     return;
   }
   ExperimentSet::iterator it = allow_latency_experiment_.find(url.host());
   if (allow_latency_experiment_.end() == it)
     return;  // It was already erased, or never allowed.
-  SdchErrorRecovery(LATENCY_TEST_DISALLOWED);
+  SdchErrorRecovery(SDCH_LATENCY_TEST_DISALLOWED);
   allow_latency_experiment_.erase(it);
 }
 
-void SdchManager::AddSdchDictionary(const std::string& dictionary_text,
+SdchProblemCodes SdchManager::AddSdchDictionary(
+    const std::string& dictionary_text,
     const GURL& dictionary_url) {
   DCHECK(CalledOnValidThread());
   std::string client_hash;
   std::string server_hash;
   GenerateHash(dictionary_text, &client_hash, &server_hash);
-  if (dictionaries_.find(server_hash) != dictionaries_.end()) {
-    SdchErrorRecovery(DICTIONARY_ALREADY_LOADED);
-    return;                             // Already loaded.
-  }
+  if (dictionaries_.find(server_hash) != dictionaries_.end())
+    return SDCH_DICTIONARY_ALREADY_LOADED;  // Already loaded.
 
   std::string domain, path;
   std::set<int> ports;
   base::Time expiration(base::Time::Now() + base::TimeDelta::FromDays(30));
 
-  if (dictionary_text.empty()) {
-    SdchErrorRecovery(DICTIONARY_HAS_NO_TEXT);
-    return;                             // Missing header.
-  }
+  if (dictionary_text.empty())
+    return SDCH_DICTIONARY_HAS_NO_TEXT;  // Missing header.
 
   size_t header_end = dictionary_text.find("\n\n");
-  if (std::string::npos == header_end) {
-    SdchErrorRecovery(DICTIONARY_HAS_NO_HEADER);
-    return;                             // Missing header.
-  }
+  if (std::string::npos == header_end)
+    return SDCH_DICTIONARY_HAS_NO_HEADER;  // Missing header.
+
   size_t line_start = 0;  // Start of line being parsed.
   while (1) {
     size_t line_end = dictionary_text.find('\n', line_start);
     DCHECK(std::string::npos != line_end);
     DCHECK_LE(line_end, header_end);
 
     size_t colon_index = dictionary_text.find(':', line_start);
-    if (std::string::npos == colon_index) {
-      SdchErrorRecovery(DICTIONARY_HEADER_LINE_MISSING_COLON);
-      return;                         // Illegal line missing a colon.
-    }
+    if (std::string::npos == colon_index)
+      return SDCH_DICTIONARY_HEADER_LINE_MISSING_COLON;  // Illegal line missing
+                                                         // a colon.
 
     if (colon_index > line_end)
       break;
 
     size_t value_start = dictionary_text.find_first_not_of(" \t",
                                                            colon_index + 1);
     if (std::string::npos != value_start) {
       if (value_start >= line_end)
         break;
       std::string name(dictionary_text, line_start, colon_index - line_start);
       std::string value(dictionary_text, value_start, line_end - value_start);
       name = base::StringToLowerASCII(name);
       if (name == "domain") {
         domain = value;
       } else if (name == "path") {
         path = value;
       } else if (name == "format-version") {
         if (value != "1.0")
-          return;
+          return SDCH_UNSUPPORTED_VERSION;
       } else if (name == "max-age") {
         int64 seconds;
         base::StringToInt64(value, &seconds);
         expiration = base::Time::Now() + base::TimeDelta::FromSeconds(seconds);
       } else if (name == "port") {
         int port;
         base::StringToInt(value, &port);
         if (port >= 0)
           ports.insert(port);
       }
     }
 
     if (line_end >= header_end)
       break;
     line_start = line_end + 1;
   }
 
   // Narrow fix for http://crbug.com/389451.
   GURL dictionary_url_normalized(dictionary_url);
   StripTrailingDot(&dictionary_url_normalized);
 
-  if (!IsInSupportedDomain(dictionary_url_normalized))
-    return;
+  SdchProblemCodes rv = IsInSupportedDomain(dictionary_url_normalized);
+  if (rv != SDCH_OK)
+    return rv;
 
-  if (!Dictionary::CanSet(domain, path, ports, dictionary_url_normalized))
-    return;
+  rv = Dictionary::CanSet(domain, path, ports, dictionary_url_normalized);
+  if (rv != SDCH_OK)
+    return rv;
 
   // TODO(jar): Remove these hacks to preclude a DOS attack involving piles of
   // useless dictionaries.  We should probably have a cache eviction plan,
   // instead of just blocking additions.  For now, with the spec in flux, it
   // is probably not worth doing eviction handling.
-  if (kMaxDictionarySize < dictionary_text.size()) {
-    SdchErrorRecovery(DICTIONARY_IS_TOO_LARGE);
-    return;
-  }
-  if (kMaxDictionaryCount <= dictionaries_.size()) {
-    SdchErrorRecovery(DICTIONARY_COUNT_EXCEEDED);
-    return;
-  }
+  if (kMaxDictionarySize < dictionary_text.size())
+    return SDCH_DICTIONARY_IS_TOO_LARGE;
+
+  if (kMaxDictionaryCount <= dictionaries_.size())
+    return SDCH_DICTIONARY_COUNT_EXCEEDED;
 
   UMA_HISTOGRAM_COUNTS("Sdch3.Dictionary size loaded", dictionary_text.size());
   DVLOG(1) << "Loaded dictionary with client hash " << client_hash
            << " and server hash " << server_hash;
   Dictionary* dictionary =
       new Dictionary(dictionary_text, header_end + 2, client_hash,
                      dictionary_url_normalized, domain,
                      path, expiration, ports);
   dictionaries_[server_hash] = dictionary;
-  return;
+  return SDCH_OK;
 }
 
 // static
 void SdchManager::UrlSafeBase64Encode(const std::string& input,
                                       std::string* output) {
   // Since this is only done during a dictionary load, and hashes are only 8
   // characters, we just do the simple fixup, rather than rewriting the encoder.
   base::Base64Encode(input, output);
   std::replace(output->begin(), output->end(), '+', '-');
   std::replace(output->begin(), output->end(), '/', '_');
 }
 
+base::Value* SdchManager::SdchInfoToValue() const {
+  base::DictionaryValue* value = new base::DictionaryValue();
+
+  value->SetBoolean("sdch_enabled", sdch_enabled());
+  value->SetBoolean("secure_scheme_support", secure_scheme_supported());
+
+  base::ListValue* entry_list = new base::ListValue();
+  for (DictionaryMap::const_iterator it = dictionaries_.begin();
+       it != dictionaries_.end();
+       ++it) {
+    base::DictionaryValue* entry_dict = new base::DictionaryValue();
+    entry_dict->SetString("url", it->second->url().spec());
+    entry_dict->SetString("client_hash", it->second->client_hash());
+    entry_dict->SetString("domain", it->second->domain());
+    entry_dict->SetString("path", it->second->path());
+    base::ListValue* port_list = new base::ListValue();
+    for (std::set<int>::const_iterator port_it = it->second->ports().begin();
+         port_it != it->second->ports().end();
+         ++port_it) {
+      port_list->AppendInteger(*port_it);
+    }
+    entry_dict->Set("ports", port_list);
+    entry_dict->SetString("server_hash", it->first);
+    entry_list->Append(entry_dict);
+  }
+  value->Set("dictionaries", entry_list);
+
+  entry_list = new base::ListValue();
+  for (DomainBlacklistInfo::const_iterator it = blacklisted_domains_.begin();
+       it != blacklisted_domains_.end();
+       ++it) {
+    if (it->second.count == 0)
+      continue;
+    base::DictionaryValue* entry_dict = new base::DictionaryValue();
+    entry_dict->SetString("domain", it->first);
+    if (it->second.count != INT_MAX)
+      entry_dict->SetInteger("tries", it->second.count);
+    entry_dict->SetInteger("reason", it->second.reason);
+    entry_list->Append(entry_dict);
+  }
+  value->Set("blacklisted", entry_list);
+
+  return value;
+}
+
 }  // namespace net