Sdch view for net-internals

net/filter/sdch_filter.cc

Index: net/filter/sdch_filter.cc
diff --git a/net/filter/sdch_filter.cc b/net/filter/sdch_filter.cc
index df72367c1dd7729a8ba353ac37b70af4e56089b7..cbf38626604e11f3420e4a6755ddcb55a1862da6 100644
--- a/net/filter/sdch_filter.cc
+++ b/net/filter/sdch_filter.cc
@@ -12,6 +12,7 @@
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "net/base/sdch_manager.h"
+#include "net/base/sdch_net_log_params.h"
 #include "net/url_request/url_request_context.h"
 
 #include "sdch/open-vcdiff/src/google/vcdecoder.h"
@@ -84,7 +85,7 @@ SdchFilter::~SdchFilter() {
   if (vcdiff_streaming_decoder_.get()) {
     if (!vcdiff_streaming_decoder_->FinishDecoding()) {
       decoding_status_ = DECODING_ERROR;
-      SdchManager::SdchErrorRecovery(SdchManager::INCOMPLETE_SDCH_CONTENT);
+      LogSdchProblem(SdchManager::INCOMPLETE_SDCH_CONTENT);
       // Make it possible for the user to hit reload, and get non-sdch content.
       // Note this will "wear off" quickly enough, and is just meant to assure
       // in some rare case that the user is not stuck.
@@ -99,7 +100,7 @@ SdchFilter::~SdchFilter() {
 
   if (!dest_buffer_excess_.empty()) {
     // Filter chaining error, or premature teardown.
-    SdchManager::SdchErrorRecovery(SdchManager::UNFLUSHED_CONTENT);
+    LogSdchProblem(SdchManager::UNFLUSHED_CONTENT);
     UMA_HISTOGRAM_COUNTS("Sdch3.UnflushedBytesIn",
          static_cast<int>(filter_context_.GetByteReadCount()));
     UMA_HISTOGRAM_COUNTS("Sdch3.UnflushedBufferSize",
@@ -111,7 +112,7 @@ SdchFilter::~SdchFilter() {
   if (filter_context_.IsCachedContent()) {
     // Not a real error, but it is useful to have this tally.
     // TODO(jar): Remove this stat after SDCH stability is validated.
-    SdchManager::SdchErrorRecovery(SdchManager::CACHE_DECODED);
+    LogSdchProblem(SdchManager::CACHE_DECODED);
     return;  // We don't need timing stats, and we aready got ratios.
   }
 
@@ -135,15 +136,15 @@ SdchFilter::~SdchFilter() {
       return;
     }
     case DECODING_UNINITIALIZED: {
-      SdchManager::SdchErrorRecovery(SdchManager::UNINITIALIZED);
+      LogSdchProblem(SdchManager::UNINITIALIZED);
       return;
     }
     case WAITING_FOR_DICTIONARY_SELECTION: {
-      SdchManager::SdchErrorRecovery(SdchManager::PRIOR_TO_DICTIONARY);
+      LogSdchProblem(SdchManager::PRIOR_TO_DICTIONARY);
       return;
     }
     case DECODING_ERROR: {
-      SdchManager::SdchErrorRecovery(SdchManager::DECODE_ERROR);
+      LogSdchProblem(SdchManager::DECODE_ERROR);
       return;
     }
     case META_REFRESH_RECOVERY: {
@@ -210,7 +211,7 @@ Filter::FilterStatus SdchFilter::ReadFilteredData(char* dest_buffer,
         // 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);
+        LogSdchProblem(SdchManager::PASS_THROUGH_404_CODE);
         cause = RESPONSE_404;
         decoding_status_ = PASS_THROUGH;
       } else if (filter_context_.GetResponseCode() != 200) {
@@ -220,7 +221,7 @@ Filter::FilterStatus SdchFilter::ReadFilteredData(char* dest_buffer,
                  && !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);
+        LogSdchProblem(SdchManager::PASS_THROUGH_OLD_CACHED);
         decoding_status_ = PASS_THROUGH;
         cause = RESPONSE_OLD_UNENCODED;
       } else if (possible_pass_through_) {
@@ -256,7 +257,7 @@ Filter::FilterStatus SdchFilter::ReadFilteredData(char* dest_buffer,
         // 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);
+        LogSdchProblem(SdchManager::PASSING_THROUGH_NON_SDCH);
         decoding_status_ = PASS_THROUGH;
         // ... but further back-off on advertising SDCH support.
         url_request_context_->sdch_manager()->BlacklistDomain(
@@ -288,7 +289,7 @@ Filter::FilterStatus SdchFilter::ReadFilteredData(char* dest_buffer,
                SdchManager::META_REFRESH_UNSUPPORTED);
           url_request_context_->sdch_manager()->BlacklistDomainForever(
               url_, problem);
-          SdchManager::SdchErrorRecovery(problem);
+          LogSdchProblem(problem);
           return FILTER_ERROR;
         }
         // HTML content means we can issue a meta-refresh, and get the content
@@ -296,14 +297,13 @@ Filter::FilterStatus SdchFilter::ReadFilteredData(char* dest_buffer,
         if (filter_context_.IsCachedContent()) {
           // Cached content is probably a startup tab, so we'll just get fresh
           // content and try again, without disabling sdch.
-          SdchManager::SdchErrorRecovery(
-              SdchManager::META_REFRESH_CACHED_RECOVERY);
+          LogSdchProblem(SdchManager::META_REFRESH_CACHED_RECOVERY);
         } else {
           // Since it wasn't in the cache, we definately need at least some
           // period of blacklisting to get the correct content.
           url_request_context_->sdch_manager()->BlacklistDomain(
               url_, SdchManager::META_REFRESH_RECOVERY);
-          SdchManager::SdchErrorRecovery(SdchManager::META_REFRESH_RECOVERY);
+          LogSdchProblem(SdchManager::META_REFRESH_RECOVERY);
         }
         decoding_status_ = META_REFRESH_RECOVERY;
         // Issue a meta redirect with SDCH disabled.
@@ -357,7 +357,7 @@ Filter::FilterStatus SdchFilter::ReadFilteredData(char* dest_buffer,
   if (!ret) {
     vcdiff_streaming_decoder_.reset(NULL);  // Don't call it again.
     decoding_status_ = DECODING_ERROR;
-    SdchManager::SdchErrorRecovery(SdchManager::DECODE_BODY_ERROR);
+    LogSdchProblem(SdchManager::DECODE_BODY_ERROR);
     return FILTER_ERROR;
   }
 
@@ -396,9 +396,12 @@ Filter::FilterStatus SdchFilter::InitializeDictionary() {
 
   if ('\0' == dictionary_hash_[kServerIdLength - 1]) {
     SdchManager* manager(url_request_context_->sdch_manager());
-    manager->GetVcdiffDictionary(
+    SdchManager::ProblemCodes rv = manager->GetVcdiffDictionary(
         std::string(dictionary_hash_, 0, kServerIdLength - 1),
-        url_, &dictionary_);
+        url_,
+        &dictionary_);
+    if (rv != SdchManager::PROBLEM_CODE_OK)
+      LogSdchProblem(rv);
   } else {
     dictionary_hash_is_plausible_ = false;
   }
@@ -414,9 +417,9 @@ Filter::FilterStatus SdchFilter::InitializeDictionary() {
       }
     }
     if (dictionary_hash_is_plausible_)
-      SdchManager::SdchErrorRecovery(SdchManager::DICTIONARY_HASH_NOT_FOUND);
+      LogSdchProblem(SdchManager::DICTIONARY_HASH_NOT_FOUND);
     else
-      SdchManager::SdchErrorRecovery(SdchManager::DICTIONARY_HASH_MALFORMED);
+      LogSdchProblem(SdchManager::DICTIONARY_HASH_MALFORMED);
     decoding_status_ = DECODING_ERROR;
     return FILTER_ERROR;
   }
@@ -446,4 +449,15 @@ int SdchFilter::OutputBufferExcess(char* const dest_buffer,
   return amount;
 }
 
+void SdchFilter::LogSdchProblem(SdchManager::ProblemCodes problem) {
+  SdchManager::SdchErrorRecovery(problem);
+  // If we were just guessing, we should not display SDCH-decoding problems as
+  // errors.

[Randy Smith (Not in Mondays), 2014/09/18 20:55:51]

Suggestion: Move this comment to right above the !possible_pass_through_; on my
first read through, I wasn't sure what you were referring to.

+  filter_context_.GetNetLog().AddEvent(
+      NetLog::TYPE_SDCH_RESOURCE_ERROR,
+      base::Bind(&NetLogSdchResourceProblemCallback,
+                 problem,
+                 !possible_pass_through_));

[Randy Smith (Not in Mondays), 2014/09/18 20:55:51]

My read of Filter::FixupEncodingTypes() is that we won't use use
FILTER_TYPE_SDCH_POSSIBLE unless we expected an SDCH response (because we
advertised a dictionary and the server didn't say X-SDCH-Encode: 0).  In that
case, I think it's reasonable to call out problems as errors, though it's
probably worthwhile checking with Matt on what level of thing he'd like to be
seen as an "error" in net-internals.  

[baranovich, 2014/09/30 13:16:52]

You're right. I removed this

+}
+
 }  // namespace net