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

Unified Diff: content/browser/appcache/appcache_update_job.cc

Issue 879393002: Expire appcaches that fail to update for "too long". (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 11 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
« content/browser/appcache/appcache.h ('K') | « content/browser/appcache/appcache.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/appcache/appcache_update_job.cc
diff --git a/content/browser/appcache/appcache_update_job.cc b/content/browser/appcache/appcache_update_job.cc
index 4848c9a5f15019ae07ff5a5c0dc7d26e7239e3b0..74c4dbbd70422239cca6ac93ead0b15b359963d4 100644
--- a/content/browser/appcache/appcache_update_job.cc
+++ b/content/browser/appcache/appcache_update_job.cc
@@ -251,6 +251,13 @@ void AppCacheUpdateJob::URLFetcher::AddConditionalHeaders(
DCHECK(request_.get() && headers);
net::HttpRequestHeaders extra_headers;
+ // TODO(michaeln): skip condition headers on manifest fetches if its been 24 hours
+ // since the last update check
+ // TODO: store date of last update check even that busted the cache
+ // if (firstManifestFetch &&
+ // now > newest_successful_unconditional_update_check_time + 24hours)
+ // performing_unconditional_update_check_ = true;
+
// Add If-Modified-Since header if response info has Last-Modified header.
const std::string last_modified = "Last-Modified";
std::string last_modified_value;
@@ -478,6 +485,17 @@ void AppCacheUpdateJob::HandleCacheFailure(
DiscardInprogressCache();
internal_state_ = COMPLETED;
DeleteSoon(); // To unwind the stack prior to deletion.
+
+ // TODO(michaeln): Evict the cache under other cicrumstances too
+ // if we've been getting srvr errors for "too long"
+ // if we've been getting srvr redirects for "too long"
+ // if we've been getting an invalid manifest for "too long"
+ // maybe "too long" for http is less than it is for https?
palmer 2015/04/21 19:56:34 Good idea.
+ // TODO: store date of first evictable error since last successfull update check
+ // if (is_an_evictable_error &&
+ // now > first_evictable_error_since_successfull_check + kTooLong)
+ // evict();
+ // else if (!first_evictable_error_since_successfull_check) set it to now
}
void AppCacheUpdateJob::FetchManifest(bool is_first_fetch) {
@@ -541,6 +559,8 @@ void AppCacheUpdateJob::HandleManifestFetchCompleted(
update_type_ == UPGRADE_ATTEMPT) {
storage_->MakeGroupObsolete(group_, this, response_code); // async
} else {
+ // TODO(michaeln): Evict the cache under other cicrumstances too.
+ // APPCACHE_MANIFEST_ERROR + networkresponsecde is an evictable error
const char* kFormatString = "Manifest fetch failed (%d) %s";
std::string message = FormatUrlErrorMessage(
kFormatString, manifest_url_, fetcher->result(), response_code);
@@ -587,6 +607,7 @@ void AppCacheUpdateJob::ContinueHandleManifestFetchCompleted(bool changed) {
DCHECK(internal_state_ == FETCH_MANIFEST);
if (!changed) {
+ // if unconditional_check, set time to now
DCHECK(update_type_ == UPGRADE_ATTEMPT);
internal_state_ = NO_UPDATE;
@@ -603,6 +624,8 @@ void AppCacheUpdateJob::ContinueHandleManifestFetchCompleted(bool changed) {
PARSE_MANIFEST_ALLOWING_INTERCEPTS :
PARSE_MANIFEST_PER_STANDARD,
manifest)) {
+ // TODO(michaeln): Evict the cache under other cicrumstances too.
+ // APPCACHE_SIGNATURE_ERROR is an evictable error
const char* kFormatString = "Failed to parse manifest %s";
const std::string message = base::StringPrintf(kFormatString,
manifest_url_.spec().c_str());
« content/browser/appcache/appcache.h ('K') | « content/browser/appcache/appcache.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698