AppCaches which belong to hosted apps are not protected from deletion

webkit/appcache/appcache_storage_impl.cc

Index: webkit/appcache/appcache_storage_impl.cc
diff --git a/webkit/appcache/appcache_storage_impl.cc b/webkit/appcache/appcache_storage_impl.cc
index 6b7d04d148979f8ba367ba50f4d508eb264ae61f..a6b561ffa219d7969fb19d0d891d37e1a5852053 100644
--- a/webkit/appcache/appcache_storage_impl.cc
+++ b/webkit/appcache/appcache_storage_impl.cc
@@ -34,8 +34,66 @@ namespace {
 void DeleteDirectory(const FilePath& path) {
   file_util::Delete(path, true);
 }
+
+void ClearOnExit(
+    appcache::AppCacheDatabase* database,
+    scoped_refptr<quota::SpecialStoragePolicy> special_storage_policy) {
+  LOG(WARNING) << "ClearOnExit";
+  std::set<GURL> origins;
+  database->FindOriginsWithGroups(&origins);
+  if (origins.empty())
+    return;  // nothing to delete
+
+  sql::Connection* connection = database->db_connection();
+  if (!connection) {
+    NOTREACHED() << "Missing database connection.";
+    return;
+  }
+
+  std::set<GURL>::const_iterator origin;
+  for (origin = origins.begin(); origin != origins.end(); ++origin) {
+    if (special_storage_policy &&
+        special_storage_policy->IsStorageProtected(*origin))
+      continue;
+
+    std::vector<appcache::AppCacheDatabase::GroupRecord> groups;
+    database->FindGroupsForOrigin(*origin, &groups);
+    std::vector<appcache::AppCacheDatabase::GroupRecord>::const_iterator group;
+    for (group = groups.begin(); group != groups.end(); ++group) {
+      sql::Transaction transaction(connection);
+      if (!transaction.Begin()) {
+        NOTREACHED() << "Failed to start transaction";
+        return;
+      }
+
+      bool success = false;
+
+      // Logic lifted from MakeGroupObsoleteTask::Run, could be shared.

[michaeln, 2011/07/14 22:53:16]

Can you take a stab at sharing this logic with MakeGroupObsoleteTask::Run? A
helper function in the anon namespace could work...

bool DeleteGroupAndRelatedRecords(db, group, opt_deletable_response_ids);

...that's used by both MakeGroupObsoleteTask::Run and this function.

[marja(google), 2011/07/15 11:03:42]

Done.

Btw, how about AppCacheGroup::AddNewlyDeletableResponseIds;
MakeGroupObsoleteTask does that but the ClearOnExit doesn't. Maybe it doesn't
matter in the current situation, since the Profiles are deleted only on exit.
But what if the Profiles are deleted sooner? Should the ClearOnExit also do
AddNewlyDeletableResponseIds?

[michaeln, 2011/07/15 19:15:46]

By the time our ClearOnExit function is run, there are no AppCacheGroup
instances in memory anymore. That stuff has to do with lazily scavenging
responses from the DiskCache, the AppCacheStorageImpl class is what conducts
that scavenging... but the service and the storage class are gone by ClearOnExit
time.

+      appcache::AppCacheDatabase::CacheRecord cache_record;
+      if (database->FindCacheForGroup(group->group_id, &cache_record)) {
+        std::vector<int64> response_ids;
+        database->FindResponseIdsForCacheAsVector(cache_record.cache_id,
+                                                  &response_ids);
+        success =
+            database->DeleteGroup(group->group_id) &&
+            database->DeleteCache(cache_record.cache_id) &&
+            database->DeleteEntriesForCache(cache_record.cache_id) &&
+            database->DeleteFallbackNameSpacesForCache(cache_record.cache_id) &&
+            database->DeleteOnlineWhiteListForCache(cache_record.cache_id) &&
+            database->InsertDeletableResponseIds(response_ids);
+      } else {
+        NOTREACHED() << "A existing group without a cache is unexpected";
+        success = database->DeleteGroup(group->group_id);
+      }
+
+      success = success && transaction.Commit();
+      DCHECK(success);
+    }  // for each group
+  }  // for each origin
 }
 
+}  // namespace
+
 namespace appcache {
 
 // Hard coded default when not using quota management.
@@ -1088,6 +1146,16 @@ AppCacheStorageImpl::~AppCacheStorageImpl() {
                 scheduled_database_tasks_.end(),
                 std::mem_fun(&DatabaseTask::CancelCompletion));
 
+  if (service()->clear_local_state_on_exit() && database_) {

[michaeln, 2011/07/14 22:53:16]

nit: maybe test if (database_) once and {
  if (clear_on_exit())
    PostTask(ClearOnExit);
  DeleteSoon(database_);
}

[marja(google), 2011/07/15 11:03:42]

Done.

+    AppCacheThread::PostTask(
+        AppCacheThread::db(),
+        FROM_HERE,
+        NewRunnableFunction(
+            ClearOnExit,
+            database_,
+            make_scoped_refptr(service_->special_storage_policy())));
+
+  }
   if (database_)
     AppCacheThread::DeleteSoon(AppCacheThread::db(), FROM_HERE, database_);
 }