ServiceWorker: Introduce ServiceWorkerDiskCacheMigrator

content/browser/service_worker/service_worker_disk_cache_migrator.cc

Index: content/browser/service_worker/service_worker_disk_cache_migrator.cc
diff --git a/content/browser/service_worker/service_worker_disk_cache_migrator.cc b/content/browser/service_worker/service_worker_disk_cache_migrator.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f08da847a042243df80c944b0848a6f00a36641d
--- /dev/null
+++ b/content/browser/service_worker/service_worker_disk_cache_migrator.cc
@@ -0,0 +1,269 @@
+// Copyright 2015 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 "content/browser/service_worker/service_worker_disk_cache_migrator.h"
+
+#include "base/memory/ref_counted.h"
+#include "base/strings/string_number_conversions.h"
+#include "content/common/service_worker/service_worker_types.h"
+#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
+#include "net/disk_cache/disk_cache.h"
+
+namespace content {
+
+// A task to move a cached resource from the src DiskCache to the dest
+// DiskCache. This is owned by ServiceWorkerDiskCacheMigrator.
+class ServiceWorkerDiskCacheMigrator::Task {
+ public:
+  Task(InflightTaskMap::KeyType task_id,
+       int64 resource_id,
+       disk_cache::Entry* entry,
+       ServiceWorkerDiskCache* src,
+       ServiceWorkerDiskCache* dest,
+       const base::WeakPtr<ServiceWorkerDiskCacheMigrator>& owner);
+  ~Task();
+
+  void Run();
+
+ private:
+  void ReadResponseInfo();
+  void OnReadResponseInfo(
+      const scoped_refptr<HttpResponseInfoIOBuffer>& info_buffer,
+      int result);
+  void OnWriteResponseInfo(
+      const scoped_refptr<HttpResponseInfoIOBuffer>& info_buffer,
+      int result);
+  void WriteResponseMetadata(
+      const scoped_refptr<HttpResponseInfoIOBuffer>& info_buffer);
+  void OnWriteResponseMetadata(int result);
+  void ReadResponseData();
+  void OnReadResponseData(const scoped_refptr<net::IOBuffer>& buffer,
+                          int result);
+  void OnWriteResponseData(int result);
+  void Finish(ServiceWorkerStatusCode status);
+
+  InflightTaskMap::KeyType task_id_;
+  int64 resource_id_;
+  disk_cache::Entry* entry_;
+  base::WeakPtr<ServiceWorkerDiskCacheMigrator> owner_;
+
+  scoped_ptr<ServiceWorkerResponseReader> reader_;
+  scoped_ptr<ServiceWorkerResponseWriter> writer_;
+  scoped_ptr<ServiceWorkerResponseMetadataWriter> metadata_writer_;
+
+  base::WeakPtrFactory<Task> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(Task);
+};
+
+ServiceWorkerDiskCacheMigrator::Task::Task(
+    InflightTaskMap::KeyType task_id,
+    int64 resource_id,
+    disk_cache::Entry* entry,
+    ServiceWorkerDiskCache* src,
+    ServiceWorkerDiskCache* dest,
+    const base::WeakPtr<ServiceWorkerDiskCacheMigrator>& owner)
+    : task_id_(task_id),
+      resource_id_(resource_id),
+      entry_(entry),
+      owner_(owner),
+      weak_factory_(this) {
+  DCHECK(entry_);
+  reader_.reset(new ServiceWorkerResponseReader(resource_id, src));
+  writer_.reset(new ServiceWorkerResponseWriter(resource_id, dest));
+  metadata_writer_.reset(
+      new ServiceWorkerResponseMetadataWriter(resource_id, dest));
+}
+
+ServiceWorkerDiskCacheMigrator::Task::~Task() {
+  entry_->Close();
+}
+
+void ServiceWorkerDiskCacheMigrator::Task::Run() {
+  ReadResponseInfo();
+}
+
+void ServiceWorkerDiskCacheMigrator::Task::ReadResponseInfo() {
+  scoped_refptr<HttpResponseInfoIOBuffer> info_buffer(
+      new HttpResponseInfoIOBuffer);
+  reader_->ReadInfo(info_buffer.get(),
+                    base::Bind(&Task::OnReadResponseInfo,
+                               weak_factory_.GetWeakPtr(), info_buffer));
+}
+
+void ServiceWorkerDiskCacheMigrator::Task::OnReadResponseInfo(
+    const scoped_refptr<HttpResponseInfoIOBuffer>& info_buffer,
+    int result) {
+  if (result < 0) {
+    LOG(ERROR) << "Failed to read the response info";
+    Finish(SERVICE_WORKER_ERROR_FAILED);
+    return;
+  }
+  writer_->WriteInfo(info_buffer.get(),
+                     base::Bind(&Task::OnWriteResponseInfo,
+                                weak_factory_.GetWeakPtr(), info_buffer));
+}
+
+void ServiceWorkerDiskCacheMigrator::Task::OnWriteResponseInfo(
+    const scoped_refptr<HttpResponseInfoIOBuffer>& buffer,
+    int result) {
+  if (result < 0) {
+    LOG(ERROR) << "Failed to write the response info";
+    Finish(SERVICE_WORKER_ERROR_FAILED);
+    return;
+  }
+
+  const net::HttpResponseInfo* http_info = buffer->http_info.get();
+  if (http_info->metadata) {
+    WriteResponseMetadata(buffer);
+    return;
+  }
+  ReadResponseData();
+}
+
+void ServiceWorkerDiskCacheMigrator::Task::WriteResponseMetadata(
+    const scoped_refptr<HttpResponseInfoIOBuffer>& info_buffer) {
+  const net::HttpResponseInfo* http_info = info_buffer->http_info.get();
+  const int data_size = http_info->metadata->size();
+
+  scoped_refptr<net::IOBuffer> buffer = new net::IOBuffer(data_size);
+  if (data_size)
+    memmove(buffer->data(), http_info->metadata->data(), data_size);

[michaeln, 2015/05/27 23:09:23]

This copy is unfortunate, i think there's an IOBuffer derivative than can wrap
an caller supplied ptr, WrappedIOBuffer. I think a derivative of that could hold
on to a reference to the refcounted info_buffer that owns the metadata.

[nhiroki, 2015/05/28 07:47:11]

I see. I replaced memmove with WrapppedIOBuffer, and made
OnWriteResponseMetadata callback to keep |info_buffer|.

+  metadata_writer_->WriteMetadata(
+      buffer.get(), data_size,
+      base::Bind(&Task::OnWriteResponseMetadata, weak_factory_.GetWeakPtr()));
+}
+
+void ServiceWorkerDiskCacheMigrator::Task::OnWriteResponseMetadata(int result) {
+  if (result < 0) {
+    LOG(ERROR) << "Failed to write the response metadata";
+    Finish(SERVICE_WORKER_ERROR_FAILED);
+    return;
+  }
+  ReadResponseData();
+}
+
+void ServiceWorkerDiskCacheMigrator::Task::ReadResponseData() {
+  scoped_refptr<net::IOBuffer> buffer =
+      new net::IOBuffer(entry_->GetDataSize(0));
+  reader_->ReadData(buffer.get(), entry_->GetDataSize(0),
+                    base::Bind(&Task::OnReadResponseData,
+                               weak_factory_.GetWeakPtr(), buffer));
+}
+
+void ServiceWorkerDiskCacheMigrator::Task::OnReadResponseData(
+    const scoped_refptr<net::IOBuffer>& buffer,
+    int result) {
+  if (result < 0) {
+    LOG(ERROR) << "Failed to read the response data";
+    Finish(SERVICE_WORKER_ERROR_FAILED);
+    return;
+  }
+  writer_->WriteData(
+      buffer.get(), result,
+      base::Bind(&Task::OnWriteResponseData, weak_factory_.GetWeakPtr()));
+}
+
+void ServiceWorkerDiskCacheMigrator::Task::OnWriteResponseData(int result) {
+  if (result < 0) {
+    LOG(ERROR) << "Failed to write the response data";
+    Finish(SERVICE_WORKER_ERROR_FAILED);
+    return;
+  }
+  Finish(SERVICE_WORKER_OK);
+}
+
+void ServiceWorkerDiskCacheMigrator::Task::Finish(
+    ServiceWorkerStatusCode status) {
+  DCHECK(owner_);
+  owner_->OnResourceMigrated(task_id_, status);
+}
+
+ServiceWorkerDiskCacheMigrator::ServiceWorkerDiskCacheMigrator(
+    ServiceWorkerDiskCache* src,
+    ServiceWorkerDiskCache* dest)
+    : src_(src), dest_(dest), weak_factory_(this) {
+  DCHECK(!src_->is_disabled());
+  DCHECK(!dest_->is_disabled());
+}
+
+ServiceWorkerDiskCacheMigrator::~ServiceWorkerDiskCacheMigrator() {
+}
+
+void ServiceWorkerDiskCacheMigrator::Start(const StatusCallback& callback) {
+  callback_ = callback;
+  iterator_ = src_->disk_cache()->CreateIterator();
+  ContinueMigratingResources();
+}
+
+void ServiceWorkerDiskCacheMigrator::ContinueMigratingResources() {
+  disk_cache::Entry** entry = new disk_cache::Entry*();
+  net::CompletionCallback callback =
+      base::Bind(&ServiceWorkerDiskCacheMigrator::OnOpenNextEntry,
+                 weak_factory_.GetWeakPtr(), base::Owned(entry));
+  int result = iterator_->OpenNextEntry(entry, callback);
+  if (result == net::ERR_IO_PENDING)
+    return;
+  callback.Run(result);
+}
+
+void ServiceWorkerDiskCacheMigrator::OnOpenNextEntry(disk_cache::Entry** entry,
+                                                     int result) {
+  if (result == net::ERR_FAILED) {
+    // ERR_FAILED means the iterator reaches the end of the enumeration.
+    if (inflight_tasks_.IsEmpty())
+      Complete(SERVICE_WORKER_OK);
+    return;
+  }
+
+  if (result != net::OK) {
+    LOG(ERROR) << "Failed to open the next entry";
+    inflight_tasks_.Clear();
+    Complete(SERVICE_WORKER_ERROR_FAILED);
+    return;
+  }
+
+  int64 resource_id = kInvalidServiceWorkerResourceId;
+  if (!base::StringToInt64((*entry)->GetKey(), &resource_id)) {

[michaeln, 2015/05/27 23:09:23]

Does anything call (*entry)->Close() in this case or in the case of
OnOpenNextEntry not being called due to the weakptr becoming invalid?

[nhiroki, 2015/05/28 07:47:11]

Good point. To fix this, I introduced WrappedEntry to hold an open entry and
ensure that the entry gets closed on the dtor.

+    LOG(ERROR) << "Failed to read the resource id";
+    inflight_tasks_.Clear();
+    Complete(SERVICE_WORKER_ERROR_FAILED);
+    return;
+  }
+
+  InflightTaskMap::KeyType task_id = next_task_id_++;
+  scoped_ptr<Task> next_task(new Task(task_id, resource_id, *entry, src_, dest_,
+                                      weak_factory_.GetWeakPtr()));
+  next_task->Run();

[michaeln, 2015/05/27 23:09:23]

Looks like this will create and start tasks to migrate all entries in the cache
in parallel. Where each migrate tasks reads into memory all of the
header+complied script, then all of the script source. Is that right? Is there
anything that would prevent this from exceeding memory constraints on a low end
device given a large enough cache?

I know it's not in the CL yet, but I'm wondering how this is to be used?
- does regular browsing (minus sw registration/update) happen in parallel with
this migration occurring?
- does migration happen at startup and block first page load until completion?
- something in between?

[nhiroki, 2015/05/28 00:21:09]

In the current patchset, there is no mechanism to throttle the number of
inflight tasks and the migrator runs them greedily. We could throttle them
between OnOpenNextEntry and ContinueMigratingResources. I'll work on it.
2nd one, that is, the migration will happen during
ServiceWorkerStorage::LazyInitialize() like this:
https://codereview.chromium.org/1152543002/#ps180001

I hope this is acceptable because the migration happens only one time and
probably most of users don't have so many resources in the cache... (I'm going
to measure migration time by UMA in canary)

[nhiroki, 2015/05/28 07:47:11]

Done.

+  inflight_tasks_.AddWithID(next_task.release(), task_id);
+
+  ContinueMigratingResources();
+}
+
+void ServiceWorkerDiskCacheMigrator::OnResourceMigrated(
+    InflightTaskMap::KeyType task_id,
+    ServiceWorkerStatusCode status) {
+  DCHECK(inflight_tasks_.Lookup(task_id));
+  inflight_tasks_.Remove(task_id);
+
+  if (status != SERVICE_WORKER_OK) {
+    inflight_tasks_.Clear();
+    Complete(status);
+    return;
+  }
+
+  if (inflight_tasks_.IsEmpty()) {
+    Complete(SERVICE_WORKER_OK);
+    return;
+  }
+}
+
+void ServiceWorkerDiskCacheMigrator::Complete(ServiceWorkerStatusCode status) {
+  DCHECK(inflight_tasks_.IsEmpty());
+  // TODO(nhiroki): Add UMA for the result of migration.
+  callback_.Run(status);
+}
+
+}  // namespace content