Appcache::OnCorruptionDetected handling

webkit/browser/appcache/appcache_service.cc

 // Copyright (c) 2012 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 "webkit/browser/appcache/appcache_service.h"
 
 #include <functional>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 407 matching lines @@
     check_result = AppCacheHistograms::UNEXPECTED_DATA_SIZE;
   else
     check_result = AppCacheHistograms::RESPONSE_OK;
   AppCacheHistograms::CountCheckResponseResult(check_result);
 
   if (check_result != AppCacheHistograms::RESPONSE_OK)
     service_->DeleteAppCacheGroup(manifest_url_, net::CompletionCallback());
   delete this;
 }
 
-
 // AppCacheStorageReference ------
 
 AppCacheStorageReference::AppCacheStorageReference(
     scoped_ptr<AppCacheStorage> storage)
     : storage_(storage.Pass()) {}
 AppCacheStorageReference::~AppCacheStorageReference() {}
 
 // AppCacheService -------
 
 AppCacheService::AppCacheService(quota::QuotaManagerProxy* quota_manager_proxy)
     : appcache_policy_(NULL), quota_client_(NULL), handler_factory_(NULL),
       quota_manager_proxy_(quota_manager_proxy),
       request_context_(NULL),
-      force_keep_session_state_(false),
-      was_reinitialized_(false) {
+      force_keep_session_state_(false) {
   if (quota_manager_proxy_.get()) {
     quota_client_ = new AppCacheQuotaClient(this);
     quota_manager_proxy_->RegisterClient(quota_client_);
   }
 }
 
 AppCacheService::~AppCacheService() {
   DCHECK(backends_.empty());
   std::for_each(pending_helpers_.begin(),
                 pending_helpers_.end(),
@@ skipping 12 matching lines @@
                                  base::MessageLoopProxy* cache_thread) {
   DCHECK(!storage_.get());
   cache_directory_ = cache_directory;
   db_thread_ = db_thread;
   cache_thread_ = cache_thread;
   AppCacheStorageImpl* storage = new AppCacheStorageImpl(this);
   storage->Initialize(cache_directory, db_thread, cache_thread);
   storage_.reset(storage);
 }
 
+void AppCacheService::ScheduleReinitialize() {
+  if (reinit_timer_.IsRunning())
+    return;
+
+  // Reinitialization only happens when corruption has been noticed.
+  // We don't want to thrash the disk but we also don't want to
+  // leave the appcache disabled for an indefinite period of time. Some
+  // users never shutdown the browser.
+
+  const base::TimeDelta kZeroDelta;
+  const base::TimeDelta kOneHour(base::TimeDelta::FromHours(1));
+  const base::TimeDelta k30Seconds(base::TimeDelta::FromSeconds(30));
+
+  // If enough time has past since the last reinit, reset the delay.

[jsbell, 2014/01/24 21:13:44]

This seems set up to handle very frequent corruption detections (e.g. multiple
times an hour). Should we have a counter e.g. "# of reinit requests w/in one
hour" or something, so we can determine if some systems are prone to this
behavior?

+  if (next_reinit_delay_ != kZeroDelta &&
+      base::Time::Now() - last_reinit_time_ > kOneHour) {
+    next_reinit_delay_ = kZeroDelta;
+  }
+  reinit_timer_.Start(FROM_HERE, next_reinit_delay_,
+                      this, &AppCacheService::Reinitialize);
+
+  // Adjust the delay for next time.
+  base::TimeDelta increment = std::max(k30Seconds, next_reinit_delay_);
+  next_reinit_delay_ = std::min(next_reinit_delay_ + increment,  kOneHour);

[jsbell, 2014/01/24 21:13:44]

I don't think this is strictly necessary; if the timer has grown to > 1 hour
then now - last_reinit_time_ should also be > 1 hour, so the above test should
have reset the delay to 0.

Won't hurt, though.

[michaeln, 2014/01/28 22:12:16]

Thnx for questioning, this doesn't do what i want it to do, I'll have a new cut
at it soon.

[michaeln, 2014/01/28 22:21:36]

Actually i think the logic was doing what i want, but i made it a little clearer
what's going on, I hope clear enough to see why this std::min call does need to
be there.

+}
+
 void AppCacheService::Reinitialize() {
-  AppCacheHistograms::CountReinitAttempt(was_reinitialized_);
-
-  // To avoid thrashing, we only do this once.
-  if (was_reinitialized_)
-    return;
-  was_reinitialized_ = true;
+  AppCacheHistograms::CountReinitAttempt(!last_reinit_time_.is_null());
+  last_reinit_time_ = base::Time::Now();
 
   // Inform observers of about this and give them a chance to
   // defer deletion of the old storage object.
   scoped_refptr<AppCacheStorageReference>
       old_storage_ref(new AppCacheStorageReference(storage_.Pass()));
   FOR_EACH_OBSERVER(Observer, observers_,
                     OnServiceReinitialized(old_storage_ref.get()));
 
   Initialize(cache_directory_, db_thread_, cache_thread_);
 }
@@ skipping 47 matching lines @@
   backends_.insert(
       BackendMap::value_type(backend_impl->process_id(), backend_impl));
 }
 
 void AppCacheService::UnregisterBackend(
     AppCacheBackendImpl* backend_impl) {
   backends_.erase(backend_impl->process_id());
 }
 
 }  // namespace appcache