Added new histogram for analyzing SDCH corruption detection cases.

net/filter/sdch_filter.cc

 // Copyright 2014 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/filter/sdch_filter.h"
 
 #include <ctype.h>
 #include <limits.h>
 
 #include <algorithm>
 
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "net/base/sdch_manager.h"
 #include "net/url_request/url_request_context.h"
 
 #include "sdch/open-vcdiff/src/google/vcdecoder.h"
 
 namespace net {
 
+namespace {
+
+// Disambiguate various types of responses that trigger a meta-refresh,
+// failure, or fallback to pass-through.
+enum ResponseCorruptionDetectionCause {
+  RESPONSE_NONE,
+
+  // 404 Http Response Code
+  RESPONSE_404 = 1,
+
+  // Not a 200 Http Response Code
+  RESPONSE_NOT_200 = 2,
+
+  // Cached before dictionary retrieved.
+  RESPONSE_OLD_UNENCODED = 3,
+
+  // Speculative but incorrect SDCH filtering was added added.
+  RESPONSE_TENTATIVE_SDCH = 4,
+
+  // Missing correct dict for decoding.
+  RESPONSE_NO_DICTIONARY = 5,
+
+  // Not an SDCH response but should be.
+  RESPONSE_CORRUPT_SDCH = 6,
+
+  // No dictionary was advertised with the request, the server claims
+  // to have encoded with SDCH anyway, but it isn't an SDCH response.
+  RESPONSE_ENCODING_LIE = 7,
+
+  RESPONSE_MAX,
+};
+
+}  // namespace
+
 SdchFilter::SdchFilter(const FilterContext& filter_context)
     : filter_context_(filter_context),
       decoding_status_(DECODING_UNINITIALIZED),
       dictionary_hash_(),
       dictionary_hash_is_plausible_(false),
       dictionary_(NULL),
       url_request_context_(filter_context.GetURLRequestContext()),
       dest_buffer_excess_(),
       dest_buffer_excess_index_(0),
       source_bytes_(0),
@@ skipping 133 matching lines @@
       DCHECK(dest_buffer_excess_.empty());
       // This is where we try very hard to do error recovery, and make this
       // protocol robust in the face of proxies that do many different things.
       // If we decide that things are looking very bad (too hard to recover),
       // we may even issue a "meta-refresh" to reload the page without an SDCH
       // advertisement (so that we are sure we're not hurting anything).
       //
       // Watch out for an error page inserted by the proxy as part of a 40x
       // error response.  When we see such content molestation, we certainly
       // need to fall into the meta-refresh case.
+      ResponseCorruptionDetectionCause cause = RESPONSE_NONE;
       if (filter_context_.GetResponseCode() == 404) {
         // We could be more generous, but for now, only a "NOT FOUND" code will
         // cause a pass through.  All other bad codes will fall into a
         // meta-refresh.
         SdchManager::SdchErrorRecovery(SdchManager::PASS_THROUGH_404_CODE);
+        cause = RESPONSE_404;
         decoding_status_ = PASS_THROUGH;
       } else if (filter_context_.GetResponseCode() != 200) {
         // We need to meta-refresh, with SDCH disabled.
+        cause = RESPONSE_NOT_200;
       } else if (filter_context_.IsCachedContent()
                  && !dictionary_hash_is_plausible_) {
         // We must have hit the back button, and gotten content that was fetched
         // before we *really* advertised SDCH and a dictionary.
         SdchManager::SdchErrorRecovery(SdchManager::PASS_THROUGH_OLD_CACHED);
         decoding_status_ = PASS_THROUGH;
+        cause = RESPONSE_OLD_UNENCODED;
       } else if (possible_pass_through_) {
         // This is the potentially most graceful response. There really was no
         // error. We were just overly cautious when we added a TENTATIVE_SDCH.
         // We added the sdch coding tag, and it should not have been added.
         // This can happen in server experiments, where the server decides
         // not to use sdch, even though there is a dictionary.  To be
         // conservative, we locally added the tentative sdch (fearing that a
         // proxy stripped it!) and we must now recant (pass through).
-        SdchManager::SdchErrorRecovery(SdchManager::DISCARD_TENTATIVE_SDCH);
+        //
         // However.... just to be sure we don't get burned by proxies that
         // re-compress with gzip or other system, we can sniff to see if this
         // is compressed data etc.  For now, we do nothing, which gets us into
         // the meta-refresh result.
         // TODO(jar): Improve robustness by sniffing for valid text that we can
         // actual use re: decoding_status_ = PASS_THROUGH;
+        cause = RESPONSE_TENTATIVE_SDCH;
       } else if (dictionary_hash_is_plausible_) {
         // We need a meta-refresh since we don't have the dictionary.
         // The common cause is a restart of the browser, where we try to render
         // cached content that was saved when we had a dictionary.
-      } else if (filter_context_.IsSdchResponse()) {
+        cause = RESPONSE_NO_DICTIONARY;
+      } else if (filter_context_.SdchResponseExpected()) {
         // This is a very corrupt SDCH request response.  We can't decode it.
         // We'll use a meta-refresh, and get content without asking for SDCH.
         // This will also progressively disable SDCH for this domain.
+        cause = RESPONSE_CORRUPT_SDCH;
       } else {
         // One of the first 9 bytes precluded consideration as a hash.
         // This can't be an SDCH payload, even though the server said it was.
         // This is a major error, as the server or proxy tagged this SDCH even
         // though it is not!
         // Meta-refresh won't help, as we didn't advertise an SDCH dictionary!!
         // Worse yet, meta-refresh could lead to an infinite refresh loop.
         SdchManager::SdchErrorRecovery(SdchManager::PASSING_THROUGH_NON_SDCH);
         decoding_status_ = PASS_THROUGH;
         // ... but further back-off on advertising SDCH support.
         url_request_context_->sdch_manager()->BlacklistDomain(
             url_, SdchManager::PASSING_THROUGH_NON_SDCH);
+        cause = RESPONSE_ENCODING_LIE;
       }
+      DCHECK_NE(RESPONSE_NONE, cause);
+      UMA_HISTOGRAM_ENUMERATION(
+          (filter_context_.IsCachedContent() ?
+           "Sdch3.ResponseCorruptionDetectionCached" :
+           "Sdch3.ResponseCorruptionDetectionUncached"),

[Alexei Svitkine (slow), 2014/09/09 18:12:01]

This is incorrect to do. The first invocation of the macro will cache the
Histogram object at the call site. So you shouldn't use a variable-value string
for the first param.

Please make it an else if and maybe add a comment about it.

[Randy Smith (Not in Mondays), 2014/09/09 18:27:06]

Whoops; thank you.  

I'm not sure what you wanted the comment about.  I put in a comment about the
caching of the histogram name; let me know if that's not what you meant.

+          cause, RESPONSE_MAX);
 
       if (decoding_status_ == PASS_THROUGH) {
         dest_buffer_excess_ = dictionary_hash_;  // Send what we scanned.
       } else {
         // This is where we try to do the expensive meta-refresh.
         if (std::string::npos == mime_type_.find("text/html")) {
           // Since we can't do a meta-refresh (along with an exponential
           // backoff), we'll just make sure this NEVER happens again.
           SdchManager::ProblemCodes problem =
               (filter_context_.IsCachedContent() ?
@@ skipping 153 matching lines @@
   dest_buffer_excess_index_ += amount;
   if (dest_buffer_excess_.size() <= dest_buffer_excess_index_) {
     DCHECK(dest_buffer_excess_.size() == dest_buffer_excess_index_);
     dest_buffer_excess_.clear();
     dest_buffer_excess_index_ = 0;
   }
   return amount;
 }
 
 }  // namespace net