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

Unified Diff: net/filter/sdch_filter.cc

Issue 711753003: Pin dictionaries from being deleted while request is outstanding. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Incorporated comments; fixed try job errors. Created 6 years, 1 month 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
Index: net/filter/sdch_filter.cc
diff --git a/net/filter/sdch_filter.cc b/net/filter/sdch_filter.cc
index df72367c1dd7729a8ba353ac37b70af4e56089b7..4e223078f36a6ffac59586c7c8bfc49d38f87441 100644
--- a/net/filter/sdch_filter.cc
+++ b/net/filter/sdch_filter.cc
@@ -244,7 +244,7 @@ Filter::FilterStatus SdchFilter::ReadFilteredData(char* dest_buffer,
// The common cause is a restart of the browser, where we try to render
// cached content that was saved when we had a dictionary.
cause = RESPONSE_NO_DICTIONARY;
- } else if (filter_context_.SdchResponseExpected()) {
+ } else if (filter_context_.SdchDictionariesAdvertised()) {
// 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.
@@ -391,19 +391,44 @@ Filter::FilterStatus SdchFilter::InitializeDictionary() {
else
next_stream_data_ = NULL;
- DCHECK(!dictionary_.get());
+ DCHECK(!dictionary_);
dictionary_hash_is_plausible_ = true; // Assume plausible, but check.
if ('\0' == dictionary_hash_[kServerIdLength - 1]) {
- SdchManager* manager(url_request_context_->sdch_manager());
- manager->GetVcdiffDictionary(
- std::string(dictionary_hash_, 0, kServerIdLength - 1),
- url_, &dictionary_);
+ std::string server_hash(dictionary_hash_, 0, kServerIdLength - 1);
+ SdchManager::DictionarySet* handle =
+ filter_context_.SdchDictionariesAdvertised();
+ if (handle)
+ dictionary_ = handle->Dictionary(server_hash);
+ if (!dictionary_) {
+ // 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.
Ryan Sleevi 2014/11/14 03:21:25 Whenever someone adds a TODO like this, I like to
Randy Smith (Not in Mondays) 2014/11/17 17:04:22 I've added a pointer to the relevant bug in the TO
+ unexpected_dictionary_handle_ =
+ url_request_context_->sdch_manager()->GetDictionarySetByHash(
+ url_, server_hash);
+ if (unexpected_dictionary_handle_.get()) {
+ dictionary_ = unexpected_dictionary_handle_->Dictionary(server_hash);
+ SdchManager::SdchErrorRecovery(
+ filter_context_.IsCachedContent() ?
+ SdchManager::UNADVERTISED_SDCH_DICTIONARY_USED_CACHED :
+ SdchManager::UNADVERTISED_SDCH_DICTIONARY_USED);
+ }
+ }
} else {
dictionary_hash_is_plausible_ = false;
}
- if (!dictionary_.get()) {
+ if (!dictionary_) {
DCHECK(dictionary_hash_.size() == kServerIdLength);
// Since dictionary was not found, check to see if hash was even plausible.
for (size_t i = 0; i < kServerIdLength - 1; ++i) {

Powered by Google App Engine
This is Rietveld 408576698