Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1719)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: net/filter/sdch_policy_delegate.cc

Issue 2368433002: Add net::SdchSourceStream and net::SdchPolicyDelegate (Closed)
Patch Set: fix meta refresh Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/filter/sdch_policy_delegate.h ('k') | net/filter/sdch_source_stream.h » ('j') | net/filter/sdch_source_stream.h » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/filter/sdch_policy_delegate.cc
diff --git a/net/filter/sdch_policy_delegate.cc b/net/filter/sdch_policy_delegate.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d48ff1104f3c8ef46ce7306742155035d02f37e0
--- /dev/null
+++ b/net/filter/sdch_policy_delegate.cc
@@ -0,0 +1,189 @@
+// Copyright 2016 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_policy_delegate.h"
+
+#include "base/metrics/histogram_macros.h"
+#include "base/values.h"
+#include "net/base/sdch_net_log_params.h"
+#include "net/log/net_log.h"
+#include "net/log/net_log_event_type.h"
+
+namespace net {
+
+namespace {
+
+const char kRefreshHtml[] =
+ "<head><META HTTP-EQUIV=\"Refresh\" CONTENT=\"0\"></head>";
+
+} // namespace
+
+SdchPolicyDelegate::SdchPolicyDelegate(std::unique_ptr<Context> context)
Randy Smith (Not in Mondays) 2016/10/04 20:06:29 In the original code, there is a data member possi
xunjieli 2016/10/05 13:44:56 Thanks for catching that. I have no idea why that
Randy Smith (Not in Mondays) 2016/10/07 18:46:25 Ok. After a slog through the original code, I've
xunjieli 2016/10/10 19:31:28 Done. The CL is at https://codereview.chromium.org
+ : context_(std::move(context)) {}
+
+SdchPolicyDelegate::~SdchPolicyDelegate() {}
+
+// Dictionary errors are often the first indication that the SDCH stream has
Randy Smith (Not in Mondays) 2016/10/04 20:06:29 Is it reasonable to separate this comment out into
xunjieli 2016/10/05 13:44:56 Done.
+// become corrupt. There are many possible causes: non-200 response codes, a
Randy Smith (Not in Mondays) 2016/10/04 20:06:29 How does a non-200 response code lead here? I wou
xunjieli 2016/10/05 13:44:56 Acknowledged. Not sure what the reason is. I belie
Randy Smith (Not in Mondays) 2016/10/07 18:46:25 TODO/bug?
xunjieli 2016/10/10 19:31:28 Done.
+// cached non-SDCH-ified reply, the server opting not to use SDCH, a cached
+// SDCH-ified reply for which the SdchManager did not have the dictionary, a
+// corrupted response, or a response that claims to be SDCH but isn't actually.
+// These are handled here by issuing a meta-refresh or swapping to passthrough
+// mode if appropriate, or failing the request if the error is unrecoverable.
Randy Smith (Not in Mondays) 2016/10/04 20:06:29 Nice job turning the cascade in SdchFilter::ReadFi
xunjieli 2016/10/05 13:44:56 I'll give credit to Elly :)
+SdchPolicyDelegate::ErrorRecover SdchPolicyDelegate::OnDictionaryIdError(
+ SdchSourceStream* source,
+ std::string* replace_output) {
+ // HTTP 404 might be an unencoded error page, so if decoding failed, pass it
+ // through.
+ // TODO(xunjieli): Figure out whether this special case can be removed.
+ // crbug.com/516773.
+ if (context_->GetResponseCode() == 404) {
+ LogSdchProblem(SDCH_PASS_THROUGH_404_CODE);
+ return PASS_THROUGH;
+ }
+
+ // HTTP !200 gets a meta-refresh for HTML and a hard failure otherwise.
+ if (context_->GetResponseCode() != 200)
+ return IssueMetaRefreshIfPossible(source, replace_output);
+
+ // If this is a cached result and the source hasn't requested a dictionary, it
+ // probably never had a dictionary to begin and is an unencoded response from
+ // earlier.
+ if (context_->IsCachedContent()) {
+ LogSdchProblem(SDCH_PASS_THROUGH_OLD_CACHED);
+ return PASS_THROUGH;
+ }
+
+ // The original request didn't advertise any dictionaries, but the
+ // response claimed to be SDCH. There is no way to repair this situation: the
+ // original request already didn't advertise any dictionaries, and retrying it
+ // would likely have the/ same result. Blacklist the domain and try passing
+ // through.
+ if (!context_->SdchDictionariesAdvertised()) {
+ GURL url;
+ if (context_->GetURL(&url)) {
+ context_->GetSdchManager()->BlacklistDomain(
+ url, SDCH_PASSING_THROUGH_NON_SDCH);
+ return PASS_THROUGH;
+ }
+ }
+ return NONE;
Randy Smith (Not in Mondays) 2016/10/04 20:06:29 Why isn't this meta-refresh? I don't have confide
xunjieli 2016/10/05 13:44:56 Done.
+}
+
+SdchPolicyDelegate::ErrorRecover SdchPolicyDelegate::OnGetDictionaryError(
+ SdchSourceStream* source,
+ std::string* replace_output) {
+ // HTTP 404 might be an unencoded error page, so if decoding failed, pass it
+ // through.
+ // TODO(xunjieli): Figure out whether this special case can be removed.
+ // crbug.com/516773.
+ if (context_->GetResponseCode() == 404) {
+ LogSdchProblem(SDCH_PASS_THROUGH_404_CODE);
+ return PASS_THROUGH;
+ }
+ return IssueMetaRefreshIfPossible(source, replace_output);
+}
+
+SdchPolicyDelegate::ErrorRecover SdchPolicyDelegate::OnDecodingError(
+ SdchSourceStream* source,
+ std::string* replace_output) {
+ // A decoding error, as opposed to a dictionary error, indicates a
+ // decompression failure partway through the payload of the SDCH stream,
+ // which means that the filter already witnessed a valid dictionary ID and
+ // successfully retrieved a dictionary for it. Decoding errors are not
+ // recoverable and it is not appropriate to stop decoding, so there are
+ // relatively few error cases here.
+ //
+ // In particular, a decoding error for an HTML payload is recoverable by
+ // issuing a meta-refresh, but to avoid having that happen too often, this
+ // class also temporarily blacklists the domain. A decoding error for a
+ // non-HTML payload is unrecoverable, so such an error gets a permanent
+ // blacklist entry. If the content was cached, no blacklisting is needed.
Randy Smith (Not in Mondays) 2016/10/04 20:06:29 Hmmm. You can bounce this for this CL if you want
xunjieli 2016/10/05 13:44:56 That's a good point. I think we can include this i
Randy Smith (Not in Mondays) 2016/10/07 18:46:25 Could you record the logic/specific sub-issue abov
xunjieli 2016/10/10 19:31:28 Done. Added in the bug.
+ return IssueMetaRefreshIfPossible(source, replace_output);
+}
+
+bool SdchPolicyDelegate::OnGetDictionary(const std::string& server_id,
+ const std::string** text) {
+ SdchManager::DictionarySet* set = context_->SdchDictionariesAdvertised();
+ if (set) {
+ *text = set->GetDictionaryText(server_id);
+ if (*text)
+ return true;
+ }
+ GURL url;
+ if (!context_->GetURL(&url))
+ return false;
+ // This is a hack. Naively, the dictionaries available for
+ // decoding should be only the ones advertised. However, there are
+ // cases, specifically resources encoded with old dictionaries living
+ // in the cache, that mean the full set of dictionaries should be made
+ // available for decoding. It's not known how often this happens;
+ // if it happens rarely enough, this code can be removed.
+ //
+ // TODO(rdsmith): Long-term, a better solution is necessary, since
+ // an entry in the cache being encoded with the dictionary doesn't
+ // guarantee that the dictionary is present. That solution probably
+ // involves storing unencoded resources in the cache, but might
+ // involve evicting encoded resources on dictionary removal.
+ // See http://crbug.com/383405.
+ SdchProblemCode rv = SDCH_OK;
+ unexpected_dictionary_set_ =
+ context_->GetSdchManager()->GetDictionarySetByHash(url, server_id, &rv);
+ if (unexpected_dictionary_set_) {
+ *text = unexpected_dictionary_set_->GetDictionaryText(server_id);
+ LogSdchProblem(context_->IsCachedContent()
+ ? SDCH_UNADVERTISED_DICTIONARY_USED_CACHED
+ : SDCH_UNADVERTISED_DICTIONARY_USED);
+ if (*text)
+ return true;
+ } else {
+ LogSdchProblem(SDCH_DICTIONARY_HASH_NOT_FOUND);
+ }
+ return false;
+}
+
+// TODO(xunjieli): Remove meta refresh. crbug.com/651821.
+SdchPolicyDelegate::ErrorRecover SdchPolicyDelegate::IssueMetaRefreshIfPossible(
+ SdchSourceStream* source,
Randy Smith (Not in Mondays) 2016/10/04 20:06:29 Why does this function (and its callers) have |sou
xunjieli 2016/10/05 13:44:56 Done.
+ std::string* replace_output) {
+ std::string mime_type;
+ GURL url;
+ if (!context_->GetMimeType(&mime_type) || !context_->GetURL(&url))
+ return NONE;
+
+ SdchManager* manager = context_->GetSdchManager();
+ // Errors for non-HTML payloads are unrecoverable and get the domain
+ // blacklisted indefinitely.
+ if (mime_type.npos == mime_type.find("text/html")) {
+ SdchProblemCode problem =
+ (context_->IsCachedContent() ? SDCH_CACHED_META_REFRESH_UNSUPPORTED
+ : SDCH_META_REFRESH_UNSUPPORTED);
+ manager->BlacklistDomainForever(url, problem);
+ LogSdchProblem(problem);
+ return NONE;
+ }
+
+ if (context_->IsCachedContent()) {
+ // Cached content is a probably startup tab, so just get the fresh content
+ // and try again, without disabling SDCH.
+ LogSdchProblem(SDCH_META_REFRESH_CACHED_RECOVERY);
+ } else {
+ // Since it wasn't in the cache, blacklist for some period to get the
+ // correct content.
+ manager->BlacklistDomain(url, SDCH_META_REFRESH_RECOVERY);
+ LogSdchProblem(SDCH_META_REFRESH_RECOVERY);
+ }
+
+ *replace_output = std::string(kRefreshHtml, strlen(kRefreshHtml));
+ return REPLACE_OUTPUT;
+}
+
+void SdchPolicyDelegate::LogSdchProblem(SdchProblemCode problem) {
+ SdchManager::SdchErrorRecovery(problem);
+ context_->GetNetLog().AddEvent(
+ NetLogEventType::SDCH_DECODING_ERROR,
+ base::Bind(&NetLogSdchResourceProblemCallback, problem));
+}
+
+} // namespace net
« no previous file with comments | « net/filter/sdch_policy_delegate.h ('k') | net/filter/sdch_source_stream.h » ('j') | net/filter/sdch_source_stream.h » ('J')

Powered by Google App Engine
This is Rietveld 408576698