ServiceWorker: Migrate the script cache backend from BlockFile to Simple

content/browser/service_worker/service_worker_storage.cc

Index: content/browser/service_worker/service_worker_storage.cc
diff --git a/content/browser/service_worker/service_worker_storage.cc b/content/browser/service_worker/service_worker_storage.cc
index 4f714195ce0ea688daffbebcf25093034b944c16..7b66d027ffdd4d01e8ee28324039085d42ffd838 100644
--- a/content/browser/service_worker/service_worker_storage.cc
+++ b/content/browser/service_worker/service_worker_storage.cc
@@ -14,8 +14,8 @@
 #include "base/trace_event/trace_event.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_disk_cache.h"
+#include "content/browser/service_worker/service_worker_disk_cache_migrator.h"
 #include "content/browser/service_worker/service_worker_info.h"
-#include "content/browser/service_worker/service_worker_metrics.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_utils.h"
 #include "content/browser/service_worker/service_worker_version.h"
@@ -57,10 +57,13 @@ void CompleteFindSoon(
           base::Bind(&CompleteFindNow, registration, status, callback));
 }
 
-const base::FilePath::CharType kDatabaseName[] =
-    FILE_PATH_LITERAL("Database");
-const base::FilePath::CharType kDiskCacheName[] =
-    FILE_PATH_LITERAL("Cache");
+const char kDatabaseName[] = "Database";
+const char kDiskCacheName[] = "ScriptCache";
+const char kOldDiskCacheName[] = "Cache";
+
+// Version 2 means that the diskcache was migrated from BlockFile backend to
+// Simple backend (http://crbug.com/487482).

[michaeln, 2015/06/11 00:52:38]

How are we handling android? It already uses the simple backend. Should the
migrator degenerate into a rename() function call in that case?

[nhiroki, 2015/06/11 21:04:50]

Done in the separate CL:
https://codereview.chromium.org/1181573002/

(The migrator just calls base::Move() in the case)

+const int64 kCurrentDiskCacheVersion = 2;
 
 const int kMaxMemDiskCacheSize = 10 * 1024 * 1024;
 const int kMaxDiskCacheSize = 250 * 1024 * 1024;
@@ -225,6 +228,16 @@ DidDeleteRegistrationParams::~DidDeleteRegistrationParams() {
 ServiceWorkerStorage::~ServiceWorkerStorage() {
   ClearSessionOnlyOrigins();
   weak_factory_.InvalidateWeakPtrs();
+  if (disk_cache_migrator_) {
+    // The migrator will be aborted during the migration. To sweep out partial
+    // results before the next attempt, register the diskcache directory as a
+    // purgeable file.
+    database_task_manager_->GetTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(
+            base::IgnoreResult(&ServiceWorkerDatabase::WritePurgeableFile),
+            base::Unretained(database_.get()), kDiskCacheName));
+  }
   database_task_manager_->GetTaskRunner()->DeleteSoon(FROM_HERE,
                                                       database_.release());
 }
@@ -929,6 +942,7 @@ ServiceWorkerStorage::ServiceWorkerStorage(
       disk_cache_thread_(disk_cache_thread),
       quota_manager_proxy_(quota_manager_proxy),
       special_storage_policy_(special_storage_policy),
+      current_disk_cache_version_(-1),
       is_purge_pending_(false),
       has_checked_for_stale_resources_(false),
       weak_factory_(this) {
@@ -939,14 +953,21 @@ base::FilePath ServiceWorkerStorage::GetDatabasePath() {
   if (path_.empty())
     return base::FilePath();
   return path_.Append(ServiceWorkerContextCore::kServiceWorkerDirectory)
-      .Append(kDatabaseName);
+      .AppendASCII(kDatabaseName);
 }
 
 base::FilePath ServiceWorkerStorage::GetDiskCachePath() {
   if (path_.empty())
     return base::FilePath();
   return path_.Append(ServiceWorkerContextCore::kServiceWorkerDirectory)
-      .Append(kDiskCacheName);
+      .AppendASCII(kDiskCacheName);
+}
+
+base::FilePath ServiceWorkerStorage::GetOldDiskCachePath() {
+  if (path_.empty())
+    return base::FilePath();
+  return path_.Append(ServiceWorkerContextCore::kServiceWorkerDirectory)
+      .AppendASCII(kOldDiskCacheName);
 }
 
 bool ServiceWorkerStorage::LazyInitialize(const base::Closure& callback) {
@@ -969,8 +990,7 @@ bool ServiceWorkerStorage::LazyInitialize(const base::Closure& callback) {
   state_ = INITIALIZING;
   database_task_manager_->GetTaskRunner()->PostTask(
       FROM_HERE,
-      base::Bind(&ReadInitialDataFromDB,
-                 database_.get(),
+      base::Bind(&ReadInitialDataFromDB, database_.get(),
                  base::ThreadTaskRunnerHandle::Get(),
                  base::Bind(&ServiceWorkerStorage::DidReadInitialData,
                             weak_factory_.GetWeakPtr())));
@@ -979,27 +999,52 @@ bool ServiceWorkerStorage::LazyInitialize(const base::Closure& callback) {
 
 void ServiceWorkerStorage::DidReadInitialData(
     InitialData* data,
+    int64 current_disk_cache_version,
+    const std::vector<std::string>& purgeable_files,
     ServiceWorkerDatabase::Status status) {
   DCHECK(data);
   DCHECK_EQ(INITIALIZING, state_);
 
-  if (status == ServiceWorkerDatabase::STATUS_OK) {
-    next_registration_id_ = data->next_registration_id;
-    next_version_id_ = data->next_version_id;
-    next_resource_id_ = data->next_resource_id;
-    registered_origins_.swap(data->origins);
-    state_ = INITIALIZED;
-  } else {
+  if (status != ServiceWorkerDatabase::STATUS_OK) {
     DVLOG(2) << "Failed to initialize: "
              << ServiceWorkerDatabase::StatusToString(status);
     ScheduleDeleteAndStartOver();
-  }
 
-  for (std::vector<base::Closure>::const_iterator it = pending_tasks_.begin();
-       it != pending_tasks_.end(); ++it) {
-    RunSoon(FROM_HERE, *it);
+    // These tasks should run before DeleteAndStartOver is complete, and should
+    // fail due to the disabled storage.
+    for (const base::Closure& pending_task : pending_tasks_)
+      RunSoon(FROM_HERE, pending_task);
+    pending_tasks_.clear();
+    return;
   }
+
+  next_registration_id_ = data->next_registration_id;
+  next_version_id_ = data->next_version_id;
+  next_resource_id_ = data->next_resource_id;
+  registered_origins_.swap(data->origins);
+  state_ = INITIALIZED;
+  current_disk_cache_version_ = current_disk_cache_version;
+
+  for (const base::Closure& pending_task : pending_tasks_)
+    RunSoon(FROM_HERE, pending_task);
   pending_tasks_.clear();
+
+  if (path_.empty()) {
+    // On-memory storage should not have to purge files.
+    DCHECK(purgeable_files.empty());
+    return;
+  }
+
+  const base::FilePath service_worker_directory =
+      path_.Append(ServiceWorkerContextCore::kServiceWorkerDirectory);
+  for (const std::string& filename : purgeable_files) {

[michaeln, 2015/06/11 00:52:38]

Can you defer the start of purging with BrowserThread::PostAfterStartupTask to
reduce IO during startup.

[nhiroki, 2015/06/11 21:04:50]

Done. Purging is deferred until an initial diskcache access.

+    PostTaskAndReplyWithResult(
+        disk_cache_thread_.get(), FROM_HERE,
+        base::Bind(&base::DeleteFile,
+                   service_worker_directory.AppendASCII(filename), true),

[michaeln, 2015/06/11 00:52:38]

Having a general'ish purpose file deletion mechanism spooks me.

What if filename = "../../../Documents"? Should we care about that?

[nhiroki, 2015/06/11 21:04:50]

(This code is gone)

+        base::Bind(&ServiceWorkerStorage::DidPurgeFile,
+                   weak_factory_.GetWeakPtr()));
+  }
 }
 
 void ServiceWorkerStorage::DidFindRegistrationForDocument(
@@ -1105,6 +1150,12 @@ void ServiceWorkerStorage::ReturnFoundRegistration(
   CompleteFindNow(registration, SERVICE_WORKER_OK, callback);
 }
 
+void ServiceWorkerStorage::DidWriteDatabase(
+    ServiceWorkerDatabase::Status status) {
+  if (status != ServiceWorkerDatabase::STATUS_OK)
+    ScheduleDeleteAndStartOver();
+}
+
 void ServiceWorkerStorage::DidGetRegistrations(
     const GetRegistrationsInfosCallback& callback,
     RegistrationList* registrations,
@@ -1368,30 +1419,112 @@ ServiceWorkerStorage::FindInstallingRegistrationForId(
 }
 
 ServiceWorkerDiskCache* ServiceWorkerStorage::disk_cache() {
+  DCHECK_EQ(INITIALIZED, state_);
   if (disk_cache_)
     return disk_cache_.get();
 
-  disk_cache_ = ServiceWorkerDiskCache::CreateWithBlockFileBackend();

[michaeln, 2015/06/11 00:52:38]

Ooops, does this cause the blockfile backend to be used on android too? I think
we may have done that unintentionally in
https://codereview.chromium.org/1155063002

[nhiroki, 2015/06/11 21:04:50]

Fixed in the separate CL:
https://codereview.chromium.org/1181573002/

+  disk_cache_ = ServiceWorkerDiskCache::CreateWithSimpleBackend();
 
-  base::FilePath path = GetDiskCachePath();
-  if (path.empty()) {
+  if (GetDiskCachePath().empty()) {
     int rv = disk_cache_->InitWithMemBackend(kMaxMemDiskCacheSize,
                                              net::CompletionCallback());
     DCHECK_EQ(net::OK, rv);
     return disk_cache_.get();
   }
 
+  if (current_disk_cache_version_ < kCurrentDiskCacheVersion) {
+    // Defer the start of initialization until the migration is complete.
+    disk_cache_->set_is_waiting_to_initialize(true);
+    MigrateDiskCache();
+    return disk_cache_.get();
+  }
+  DCHECK_EQ(kCurrentDiskCacheVersion, current_disk_cache_version_);
+
+  // DiskCache has already been migrated.
+  ServiceWorkerMetrics::RecordDiskCacheMigrationResult(
+      ServiceWorkerMetrics::MIGRATION_NOT_NECESSARY);
+  InitializeDiskCache();
+
+  return disk_cache_.get();
+}
+
+void ServiceWorkerStorage::MigrateDiskCache() {
+  DCHECK(!disk_cache_migrator_);
+  disk_cache_migrator_.reset(new ServiceWorkerDiskCacheMigrator(
+      GetOldDiskCachePath(), GetDiskCachePath(), kMaxDiskCacheSize,
+      disk_cache_thread_));
+  disk_cache_migrator_->Start(base::Bind(
+      &ServiceWorkerStorage::DidMigrateDiskCache, weak_factory_.GetWeakPtr()));
+}
+
+void ServiceWorkerStorage::DidMigrateDiskCache(ServiceWorkerStatusCode status) {
+  disk_cache_migrator_.reset();
+
+  // Add the old diskcache directory to the purgeable file list.
+  PostTaskAndReplyWithResult(
+      database_task_manager_->GetTaskRunner(), FROM_HERE,
+      base::Bind(&ServiceWorkerDatabase::WritePurgeableFile,
+                 base::Unretained(database_.get()), kOldDiskCacheName),
+      base::Bind(&ServiceWorkerStorage::DidWriteDatabase,
+                 weak_factory_.GetWeakPtr()));
+
+  // Delete the old DiskCache directory.
+  PostTaskAndReplyWithResult(
+      disk_cache_thread_.get(), FROM_HERE,
+      base::Bind(&base::DeleteFile, GetOldDiskCachePath(), true),
+      base::Bind(&ServiceWorkerStorage::DidPurgeFile,
+                 weak_factory_.GetWeakPtr()));
+
+  if (status != SERVICE_WORKER_OK) {
+    LOG(ERROR) << "Failed to migrate the diskcache: "
+               << ServiceWorkerStatusToString(status);
+    ServiceWorkerMetrics::RecordDiskCacheMigrationResult(
+        ServiceWorkerMetrics::MIGRATION_ERROR_FAILED);
+
+    // Give up the migration and recreate the whole storage.
+    ScheduleDeleteAndStartOver();
+    return;
+  }
+
+  ServiceWorkerMetrics::RecordDiskCacheMigrationResult(
+      ServiceWorkerMetrics::MIGRATION_OK);
+
+  // Update the disk cache version.

[michaeln, 2015/06/11 00:52:38]

I'm wondering about the order of the operations in here? What if chrome crashes
in between any steps taken here. The steps taken are not in a deterministic
order since tasks are posted to different threads concurrently which makes it
trickier to reason about. I think as coded, we could delete the old dir prior to
having updated the version number in the db, leaving things in an inconsistent
state.

* insert old dir into db's purgeable list
* delete old dir
* delete old dir from db's purgeable list
* update db with new version number


I think we might want to alter the ordering and grouping of operations.

First record our work in the main database
beginTransaction
  update db with new version number
  insert old dir into db's purgeable list
end

Wait for xaction completion, then InitializeDiskCache() so no changes to the
diskcache can be made until we've committed the db updates.

Finally, after browser startup, delete the old directory

[nhiroki, 2015/06/11 21:04:50]

Revised these operations. The new version...

(1) introduces "migration state" instead of "version" and "purgeable list".

(2) groups operations to update a migration state to NEEDS_TO_DELETE, to delete
an old dir, and to update the state to MIGRATED in this order (see
DeleteOldDiskCacheInDB()).
   * The old dir should be deletable on the database thread because any
diskcache doesn't touch it any more.
   * If browser crashes before NEEDS_TO_DELETE is recorded, the migrator retries
from the beginning.
   * If browser crashes after NEEDS_TO_DELETE is recorded. the migrator does not
re-run and this grouped operations are executed concurrently with normal
diskcache operations.

(3) defers the start of diskcache initialization until the migration state is
updated to NEEDS_TO_DELETE or MIGRATED.

+  current_disk_cache_version_ = kCurrentDiskCacheVersion;
+  PostTaskAndReplyWithResult(
+      database_task_manager_->GetTaskRunner(), FROM_HERE,
+      base::Bind(&ServiceWorkerDatabase::WriteDiskCacheVersion,
+                 base::Unretained(database_.get()), kCurrentDiskCacheVersion),
+      base::Bind(&ServiceWorkerStorage::DidWriteDatabase,
+                 weak_factory_.GetWeakPtr()));
+
+  InitializeDiskCache();
+}
+
+void ServiceWorkerStorage::DidPurgeFile(bool deleted) {
+  if (!deleted) {
+    LOG(ERROR) << "Failed to delete the file.";
+    // Retry to delete when the browser restarts.
+    return;
+  }
+
+  // Remove the file from the purgeable file list.
+  PostTaskAndReplyWithResult(
+      database_task_manager_->GetTaskRunner(), FROM_HERE,
+      base::Bind(&ServiceWorkerDatabase::ClearPurgeableFile,
+                 base::Unretained(database_.get()), kOldDiskCacheName),
+      base::Bind(&ServiceWorkerStorage::DidWriteDatabase,
+                 weak_factory_.GetWeakPtr()));
+}
+
+void ServiceWorkerStorage::InitializeDiskCache() {
+  disk_cache_->set_is_waiting_to_initialize(false);
   int rv = disk_cache_->InitWithDiskBackend(
-      path,
-      kMaxDiskCacheSize,
-      false,
-      disk_cache_thread_,
+      GetDiskCachePath(), kMaxDiskCacheSize, false, disk_cache_thread_,
       base::Bind(&ServiceWorkerStorage::OnDiskCacheInitialized,
                  weak_factory_.GetWeakPtr()));
   if (rv != net::ERR_IO_PENDING)
     OnDiskCacheInitialized(rv);
-
-  return disk_cache_.get();
 }
 
 void ServiceWorkerStorage::OnDiskCacheInitialized(int rv) {
@@ -1538,6 +1671,7 @@ void ServiceWorkerStorage::ReadInitialDataFromDB(
   DCHECK(database);
   scoped_ptr<ServiceWorkerStorage::InitialData> data(
       new ServiceWorkerStorage::InitialData());
+  int64 disk_cache_version = -1;
 
   ServiceWorkerDatabase::Status status =
       database->GetNextAvailableIds(&data->next_registration_id,
@@ -1545,13 +1679,35 @@ void ServiceWorkerStorage::ReadInitialDataFromDB(
                                     &data->next_resource_id);
   if (status != ServiceWorkerDatabase::STATUS_OK) {
     original_task_runner->PostTask(
-        FROM_HERE, base::Bind(callback, base::Owned(data.release()), status));
+        FROM_HERE,
+        base::Bind(callback, base::Owned(data.release()), disk_cache_version,
+                   std::vector<std::string>(), status));
+    return;
+  }
+
+  status = database->ReadDiskCacheVersion(&disk_cache_version);
+  if (status != ServiceWorkerDatabase::STATUS_OK) {
+    original_task_runner->PostTask(
+        FROM_HERE,
+        base::Bind(callback, base::Owned(data.release()), disk_cache_version,
+                   std::vector<std::string>(), status));
     return;
   }
 
   status = database->GetOriginsWithRegistrations(&data->origins);
+  if (status != ServiceWorkerDatabase::STATUS_OK) {
+    original_task_runner->PostTask(
+        FROM_HERE,
+        base::Bind(callback, base::Owned(data.release()), disk_cache_version,
+                   std::vector<std::string>(), status));
+    return;
+  }
+
+  std::vector<std::string> purgeable_files;
+  status = database->GetPurgeableFiles(&purgeable_files);
   original_task_runner->PostTask(
-      FROM_HERE, base::Bind(callback, base::Owned(data.release()), status));
+      FROM_HERE, base::Bind(callback, base::Owned(data.release()),
+                            disk_cache_version, purgeable_files, status));
 }
 
 void ServiceWorkerStorage::DeleteRegistrationFromDB(