Appcache::OnCorruptionDetected handling

webkit/browser/appcache/appcache_storage_impl.cc

Index: webkit/browser/appcache/appcache_storage_impl.cc
diff --git a/webkit/browser/appcache/appcache_storage_impl.cc b/webkit/browser/appcache/appcache_storage_impl.cc
index f762a1647b793d80906169a3b44e98a340fb9004..6e7b9143e823453cf67b2daf1208f444f44ec941 100644
--- a/webkit/browser/appcache/appcache_storage_impl.cc
+++ b/webkit/browser/appcache/appcache_storage_impl.cc
@@ -173,7 +173,8 @@ class AppCacheStorageImpl::DatabaseTask
  private:
   void CallRun(base::TimeTicks schedule_time);
   void CallRunCompleted(base::TimeTicks schedule_time);
-  void CallDisableStorage();
+  void OnDisableStorage();
+  void OnCorruptionDetected();
 
   scoped_refptr<base::MessageLoopProxy> io_thread_;
 };
@@ -211,7 +212,12 @@ void AppCacheStorageImpl::DatabaseTask::CallRun(
     if (database_->is_disabled()) {
       io_thread_->PostTask(
           FROM_HERE,
-          base::Bind(&DatabaseTask::CallDisableStorage, this));
+          base::Bind(&DatabaseTask::OnDisableStorage, this));
+    } else if (database_->was_corruption_dectected()) {
+      database_->Disable();
+      io_thread_->PostTask(
+          FROM_HERE,
+          base::Bind(&DatabaseTask::OnCorruptionDetected, this));
     }
   }
   io_thread_->PostTask(
@@ -236,13 +242,22 @@ void AppCacheStorageImpl::DatabaseTask::CallRunCompleted(
   }
 }
 
-void AppCacheStorageImpl::DatabaseTask::CallDisableStorage() {
+void AppCacheStorageImpl::DatabaseTask::OnDisableStorage() {
   if (storage_) {
     DCHECK(io_thread_->BelongsToCurrentThread());
     storage_->Disable();
   }
 }
 
+void AppCacheStorageImpl::DatabaseTask::OnCorruptionDetected() {
+  if (storage_) {
+    DCHECK(io_thread_->BelongsToCurrentThread());
+    AppCacheHistograms::CountCorruptionDetected();
+    storage_->Disable();
+    storage_->DeleteAndStartOver();
+  }
+}
+
 // InitTask -------
 
 class AppCacheStorageImpl::InitTask : public DatabaseTask {
@@ -250,7 +265,14 @@ class AppCacheStorageImpl::InitTask : public DatabaseTask {
   explicit InitTask(AppCacheStorageImpl* storage)
       : DatabaseTask(storage), last_group_id_(0),
         last_cache_id_(0), last_response_id_(0),
-        last_deletable_response_rowid_(0) {}
+        last_deletable_response_rowid_(0) {
+    if (!storage->is_incognito_) {
+      db_file_path_ =
+          storage->cache_directory_.Append(kAppCacheDatabaseName);
+      disk_cache_directory_ =
+          storage->cache_directory_.Append(kDiskCacheDirectoryName);
+    }
+  }
 
   // DatabaseTask:
   virtual void Run() OVERRIDE;
@@ -260,6 +282,8 @@ class AppCacheStorageImpl::InitTask : public DatabaseTask {
   virtual ~InitTask() {}
 
  private:
+  base::FilePath db_file_path_;
+  base::FilePath disk_cache_directory_;
   int64 last_group_id_;
   int64 last_cache_id_;
   int64 last_response_id_;
@@ -268,6 +292,16 @@ class AppCacheStorageImpl::InitTask : public DatabaseTask {
 };
 
 void AppCacheStorageImpl::InitTask::Run() {
+  if (!db_file_path_.empty()) {
+    // If there is no sql database, ensure there is no disk cache either.
+    if (!base::PathExists(db_file_path_)) {
+      if (base::DirectoryExists(disk_cache_directory_)) {
+        base::DeleteFile(disk_cache_directory_, true);
+        DCHECK(!base::DirectoryExists(disk_cache_directory_));
+      }
+    }
+  }
+
   database_->FindLastStorageIds(
       &last_group_id_, &last_cache_id_, &last_response_id_,
       &last_deletable_response_rowid_);
@@ -912,8 +946,6 @@ class AppCacheStorageImpl::FindMainResponseTask : public DatabaseTask {
   GURL manifest_url_;
 };
 
-
-
 void AppCacheStorageImpl::FindMainResponseTask::Run() {
   // NOTE: The heuristics around choosing amoungst multiple candidates
   // is underspecified, and just plain not fully understood. This needs
@@ -1792,9 +1824,6 @@ AppCacheDiskCache* AppCacheStorageImpl::disk_cache() {
                      base::Unretained(this)));
     }
 
-    // We should not keep this reference around.
-    cache_thread_ = NULL;
-
     if (rv != net::ERR_IO_PENDING)
       OnDiskCacheInitialized(rv);
   }
@@ -1810,21 +1839,52 @@ void AppCacheStorageImpl::OnDiskCacheInitialized(int rv) {
     // really recover from. We handle it by temporarily disabling the appcache
     // deleting the directory on disk and reinitializing the appcache system.
     Disable();
-    if (!is_incognito_ && rv != net::ERR_ABORTED) {
-      VLOG(1) << "Deleting existing appcache data and starting over.";
-      db_thread_->PostTaskAndReply(
-          FROM_HERE,
-          base::Bind(base::IgnoreResult(&base::DeleteFile),
-                     cache_directory_, true),
-          base::Bind(&AppCacheStorageImpl::CallReinitialize,
-                     weak_factory_.GetWeakPtr()));
-    }
+    if (rv != net::ERR_ABORTED)
+      DeleteAndStartOver();
   }
 }
 
-void AppCacheStorageImpl::CallReinitialize() {
-  service_->Reinitialize();
-  // note: 'this' may be deleted during reinit.
+namespace {
+
+void HopThruCacheThread() {
+  // Function body is intentionally empty,
+  // all diskcache file handles should be closed by the
+  // time this function runs.
+}
+
+void DeleteDirectoryHelper(const base::FilePath& dir) {
+  base::DeleteFile(dir, true);
+  DCHECK(!base::DirectoryExists(dir));
+}
+
+}
+
+void AppCacheStorageImpl::DeleteAndStartOver() {
+  DCHECK(is_disabled_);
+  if (!is_incognito_) {
+    VLOG(1) << "Deleting existing appcache data and starting over.";
+    // We can have tasks in flight to close file handles on both the db
+    // and cache threads, we need to allow those tasks to cycle thru
+    // prior to deleting the files and calling reinit.
+    cache_thread_->PostTaskAndReply(
+        FROM_HERE,
+        base::Bind(&HopThruCacheThread),
+        base::Bind(&AppCacheStorageImpl::DeleteAndStartOverPart2,
+                   weak_factory_.GetWeakPtr()));
+  }
+}
+
+void AppCacheStorageImpl::DeleteAndStartOverPart2() {
+  db_thread_->PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&DeleteDirectoryHelper, cache_directory_),
+      base::Bind(&AppCacheStorageImpl::CallScheduleReinitialize,
+                  weak_factory_.GetWeakPtr()));
+}
+
+void AppCacheStorageImpl::CallScheduleReinitialize() {
+  service_->ScheduleReinitialize();
+  // note: 'this' may be deleted at this point.
 }
 
 }  // namespace appcache