Give ServiceWorkerDiskCache its own implementation

This CL duplicates AppCacheDiskCache{,Entry} into
ServiceWorkerDiskCache{,Entry} to remove the AppCache
dependency from service workers. See [1] for the reasoning of this
duplication.

To avoid risks of behavior changes, this CL only makes minimum tweaks
which includes:
* Address Tricium warnings (use default member initializers, prefer
  emplace_back).
* Follow the current coding style guide.
* Remove `use_simple_cache_` flag. It's always true for service workers.
* Add a sequence checker.

Follow-up CLs will do some refactoring like addressing
crbug.com/586174.

[1] https://docs.google.com/document/d/1UhdZnaeEP8GywZR0Gik6wF6jKmNY3je6aDhPfJ05YhU/

Bug: 1117369
Change-Id: Ia62fe55a23c2c84b569516e97e0059994c7755bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2452029
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#818326}

content/browser/service_worker/service_worker_disk_cache.h

@@ -5,20 +5,161 @@
 #ifndef CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_DISK_CACHE_H_
 #define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_DISK_CACHE_H_
 
-#include "content/browser/appcache/appcache_disk_cache.h"
+#include <stdint.h>
+
+#include <memory>
+#include <set>
+#include <vector>
+
+#include "base/callback_forward.h"
+#include "base/memory/ref_counted.h"
+#include "base/sequence_checker.h"
 #include "content/common/content_export.h"
+#include "net/base/completion_once_callback.h"
+#include "net/disk_cache/disk_cache.h"
 
 namespace content {
 
-// Wholesale reusage of the appcache code for response reading,
-// writing, and storage. See the corresponding class in that
-// library for doc comments and other details.
-// TODO(michaeln): If this reuse sticks, refactor/move the
-// resused classes to a more common location.
+// TODO(crbug.com/586174): Use disk_cache::EntryResult for better lifetime
+// management of disk cache entries. Using EntryResult will eliminate allocating
+// raw pointers and static methods in service worker resource readers/writers.
 
-class CONTENT_EXPORT ServiceWorkerDiskCache : public AppCacheDiskCache {
+class ServiceWorkerDiskCache;
+
+// Thin wrapper around disk_cache::Entry.
+class CONTENT_EXPORT ServiceWorkerDiskCacheEntry {
+ public:
+  // The newly created entry takes ownership of `disk_cache_entry` and closes it
+  // on destruction. |cache| must outlive the newly created entry.
+  ServiceWorkerDiskCacheEntry(disk_cache::Entry* disk_cache_entry,
+                              ServiceWorkerDiskCache* cache);
+
+  // See `disk_cache::Entry::ReadData()`.
+  int Read(int index,
+           int64_t offset,
+           net::IOBuffer* buf,
+           int buf_len,
+           net::CompletionOnceCallback callback);
+
+  // See `disk_cache::Entry::WriteData()`.
+  int Write(int index,
+            int64_t offset,
+            net::IOBuffer* buf,
+            int buf_len,
+            net::CompletionOnceCallback callback);
+  int64_t GetSize(int index);
+  // Closes the disk cache and destroyes the instance.
+  void Close();
+
+  // Should only be called by ServiceWorkerDiskCache.
+  void Abandon();
+
+ private:
+  // Call Close() instead of calling this directly.
+  ~ServiceWorkerDiskCacheEntry();
+
+  // The disk_cache::Entry is owned by this entry and closed on destruction.
+  disk_cache::Entry* disk_cache_entry_;
+
+  // The cache that this entry belongs to.
+  ServiceWorkerDiskCache* cache_;
+};
+
+// net::DiskCache wrapper for the cache used by service worker resources.
+//
+// Provides ways to create/open/doom service worker disk cache entries.
+class CONTENT_EXPORT ServiceWorkerDiskCache {
  public:
   ServiceWorkerDiskCache();
+  ~ServiceWorkerDiskCache();
+
+  // Initializes the object to use disk backed storage.
+  net::Error InitWithDiskBackend(const base::FilePath& disk_cache_directory,
+                                 bool force,
+                                 base::OnceClosure post_cleanup_callback,
+                                 net::CompletionOnceCallback callback);
+
+  // Initializes the object to use memory only storage.
+  // This is used for Chrome's incognito browsing.
+  net::Error InitWithMemBackend(int64_t disk_cache_size,
+                                net::CompletionOnceCallback callback);
+
+  void Disable();
+  bool is_disabled() const { return is_disabled_; }
+
+  net::Error CreateEntry(int64_t key,
+                         ServiceWorkerDiskCacheEntry** entry,
+                         net::CompletionOnceCallback callback);
+  net::Error OpenEntry(int64_t key,
+                       ServiceWorkerDiskCacheEntry** entry,
+                       net::CompletionOnceCallback callback);
+  net::Error DoomEntry(int64_t key, net::CompletionOnceCallback callback);
+
+  base::WeakPtr<ServiceWorkerDiskCache> GetWeakPtr();
+
+  void set_is_waiting_to_initialize(bool is_waiting_to_initialize) {
+    is_waiting_to_initialize_ = is_waiting_to_initialize;
+  }
+
+  disk_cache::Backend* disk_cache() { return disk_cache_.get(); }
+
+ private:
+  class CreateBackendCallbackShim;
+  friend class ServiceWorkerDiskCacheEntry;
+
+  // PendingCalls allow CreateEntry, OpenEntry, and DoomEntry to be called
+  // immediately after construction, without waiting for the
+  // underlying disk_cache::Backend to be fully constructed. Early
+  // calls are queued up and serviced once the disk_cache::Backend is
+  // really ready to go.
+  enum class PendingCallType { kCreate, kOpen, kDoom };
+  struct PendingCall {
+    PendingCall(PendingCallType call_type,
+                int64_t key,
+                ServiceWorkerDiskCacheEntry** entry,
+                net::CompletionOnceCallback callback);
+    PendingCall(PendingCall&& other);
+    PendingCall(const PendingCall&) = delete;
+    PendingCall& operator=(const PendingCall&) = delete;
+    PendingCall& operator=(PendingCall&&) = delete;
+
+    ~PendingCall();
+
+    const PendingCallType call_type;
+    const int64_t key;
+    ServiceWorkerDiskCacheEntry** const entry;
+    net::CompletionOnceCallback callback;
+  };
+
+  bool is_initializing_or_waiting_to_initialize() const {
+    return create_backend_callback_.get() != nullptr ||
+           is_waiting_to_initialize_;
+  }
+
+  net::Error Init(net::CacheType cache_type,
+                  const base::FilePath& directory,
+                  int64_t cache_size,
+                  bool force,
+                  base::OnceClosure post_cleanup_callback,
+                  net::CompletionOnceCallback callback);
+  void OnCreateBackendComplete(int return_value);
+
+  // Called by ServiceWorkerDiskCacheEntry constructor.
+  void AddOpenEntry(ServiceWorkerDiskCacheEntry* entry);
+  // Called by ServiceWorkerDiskCacheEntry destructor.
+  void RemoveOpenEntry(ServiceWorkerDiskCacheEntry* entry);
+
+  bool is_disabled_ = false;
+  bool is_waiting_to_initialize_ = false;
+  net::CompletionOnceCallback init_callback_;
+  scoped_refptr<CreateBackendCallbackShim> create_backend_callback_;
+  std::vector<PendingCall> pending_calls_;
+  std::set<ServiceWorkerDiskCacheEntry*> open_entries_;
+  std::unique_ptr<disk_cache::Backend> disk_cache_;
+
+  SEQUENCE_CHECKER(sequence_checker_);
+
+  base::WeakPtrFactory<ServiceWorkerDiskCache> weak_factory_{this};
 };
 
 }  // namespace content

content/browser/service_worker/service_worker_resource_ops.cc

@@ -124,8 +124,9 @@ mojo_base::BigBuffer BigIOBuffer::TakeBuffer() {
   return std::move(buffer_);
 }
 
-DiskEntryCreator::DiskEntryCreator(int64_t resource_id,
-                                   base::WeakPtr<AppCacheDiskCache> disk_cache)
+DiskEntryCreator::DiskEntryCreator(
+    int64_t resource_id,
+    base::WeakPtr<ServiceWorkerDiskCache> disk_cache)
     : resource_id_(resource_id), disk_cache_(std::move(disk_cache)) {
   DCHECK_NE(resource_id_, blink::mojom::kInvalidServiceWorkerResourceId);
   DCHECK(disk_cache_);
@@ -154,7 +155,7 @@ void DiskEntryCreator::EnsureEntryIsCreated(base::OnceClosure callback) {
     return;
   }
 
-  AppCacheDiskCacheEntry** entry_ptr = new AppCacheDiskCacheEntry*;
+  ServiceWorkerDiskCacheEntry** entry_ptr = new ServiceWorkerDiskCacheEntry*;
   creation_phase_ = CreationPhase::kInitialAttempt;
   int rv = disk_cache_->CreateEntry(
       resource_id_, entry_ptr,
@@ -168,7 +169,7 @@ void DiskEntryCreator::EnsureEntryIsCreated(base::OnceClosure callback) {
 // static
 void DiskEntryCreator::DidCreateEntryForFirstAttempt(
     base::WeakPtr<DiskEntryCreator> entry_creator,
-    AppCacheDiskCacheEntry** entry,
+    ServiceWorkerDiskCacheEntry** entry,
     int rv) {
   if (!entry_creator) {
     delete entry;
@@ -223,7 +224,7 @@ void DiskEntryCreator::DidDoomExistingEntry(
   }
 
   entry_creator->creation_phase_ = CreationPhase::kSecondAttempt;
-  auto** entry_ptr = new AppCacheDiskCacheEntry*;
+  auto** entry_ptr = new ServiceWorkerDiskCacheEntry*;
   rv = entry_creator->disk_cache_->CreateEntry(
       entry_creator->resource_id_, entry_ptr,
       base::BindOnce(&DiskEntryCreator::DidCreateEntryForSecondAttempt,
@@ -236,7 +237,7 @@ void DiskEntryCreator::DidDoomExistingEntry(
 // static
 void DiskEntryCreator::DidCreateEntryForSecondAttempt(
     base::WeakPtr<DiskEntryCreator> entry_creator,
-    AppCacheDiskCacheEntry** entry,
+    ServiceWorkerDiskCacheEntry** entry,
     int rv) {
   if (!entry_creator) {
     delete entry;
@@ -272,8 +273,9 @@ void DiskEntryCreator::RunEnsureEntryIsCreatedCallback() {
   std::move(ensure_entry_is_created_callback_).Run();
 }
 
-DiskEntryOpener::DiskEntryOpener(int64_t resource_id,
-                                 base::WeakPtr<AppCacheDiskCache> disk_cache)
+DiskEntryOpener::DiskEntryOpener(
+    int64_t resource_id,
+    base::WeakPtr<ServiceWorkerDiskCache> disk_cache)
     : resource_id_(resource_id), disk_cache_(std::move(disk_cache)) {
   DCHECK_NE(resource_id_, blink::mojom::kInvalidServiceWorkerResourceId);
   DCHECK(disk_cache_);
@@ -290,13 +292,13 @@ void DiskEntryOpener::EnsureEntryIsOpen(base::OnceClosure callback) {
   ensure_entry_is_opened_callback_ = std::move(callback);
 
   int rv;
-  AppCacheDiskCacheEntry** entry_ptr = nullptr;
+  ServiceWorkerDiskCacheEntry** entry_ptr = nullptr;
   if (entry_) {
     rv = net::OK;
   } else if (!disk_cache_) {
     rv = net::ERR_FAILED;
   } else {
-    entry_ptr = new AppCacheDiskCacheEntry*;
+    entry_ptr = new ServiceWorkerDiskCacheEntry*;
     rv = disk_cache_->OpenEntry(
         resource_id_, entry_ptr,
         base::BindOnce(&DiskEntryOpener::DidOpenEntry,
@@ -310,7 +312,7 @@ void DiskEntryOpener::EnsureEntryIsOpen(base::OnceClosure callback) {
 
 // static
 void DiskEntryOpener::DidOpenEntry(base::WeakPtr<DiskEntryOpener> entry_opener,
-                                   AppCacheDiskCacheEntry** entry,
+                                   ServiceWorkerDiskCacheEntry** entry,
                                    int rv) {
   if (!entry_opener) {
     delete entry;
@@ -500,7 +502,7 @@ class ServiceWorkerResourceReaderImpl::DataReader {
 
 ServiceWorkerResourceReaderImpl::ServiceWorkerResourceReaderImpl(
     int64_t resource_id,
-    base::WeakPtr<AppCacheDiskCache> disk_cache)
+    base::WeakPtr<ServiceWorkerDiskCache> disk_cache)
     : entry_opener_(resource_id, std::move(disk_cache)) {}
 
 ServiceWorkerResourceReaderImpl::~ServiceWorkerResourceReaderImpl() = default;
@@ -691,7 +693,7 @@ void ServiceWorkerResourceReaderImpl::DidReadDataComplete() {
 
 ServiceWorkerResourceWriterImpl::ServiceWorkerResourceWriterImpl(
     int64_t resource_id,
-    base::WeakPtr<AppCacheDiskCache> disk_cache)
+    base::WeakPtr<ServiceWorkerDiskCache> disk_cache)
     : entry_creator_(resource_id, std::move(disk_cache)) {}
 
 ServiceWorkerResourceWriterImpl::~ServiceWorkerResourceWriterImpl() = default;
@@ -807,7 +809,7 @@ void ServiceWorkerResourceWriterImpl::DidWriteData(
 ServiceWorkerResourceMetadataWriterImpl::
     ServiceWorkerResourceMetadataWriterImpl(
         int64_t resource_id,
-        base::WeakPtr<AppCacheDiskCache> disk_cache)
+        base::WeakPtr<ServiceWorkerDiskCache> disk_cache)
     : entry_opener_(resource_id, std::move(disk_cache)) {}
 
 ServiceWorkerResourceMetadataWriterImpl::

content/browser/service_worker/service_worker_resource_ops.h

@@ -17,14 +17,14 @@ class BigIOBuffer;
 class DiskEntryCreator {
  public:
   DiskEntryCreator(int64_t resource_id,
-                   base::WeakPtr<AppCacheDiskCache> disk_cache);
+                   base::WeakPtr<ServiceWorkerDiskCache> disk_cache);
   ~DiskEntryCreator();
 
   DiskEntryCreator(const DiskEntryCreator&) = delete;
   DiskEntryCreator& operator=(const DiskEntryCreator&) = delete;
 
   // Can be nullptr when a disk cache error occurs.
-  AppCacheDiskCacheEntry* entry() {
+  ServiceWorkerDiskCacheEntry* entry() {
     DCHECK_EQ(creation_phase_, CreationPhase::kDone);
     return entry_;
   }
@@ -53,26 +53,26 @@ class DiskEntryCreator {
   };
 
   // Callbacks of EnsureEntryIsCreated(). These are static to manage the
-  // ownership of AppCacheDiskCacheEntry correctly.
+  // ownership of ServiceWorkerDiskCacheEntry correctly.
   // TODO(crbug.com/586174): Refactor service worker's disk cache to use
   // disk_cache::EntryResult to make these callbacks non-static.
   static void DidCreateEntryForFirstAttempt(
       base::WeakPtr<DiskEntryCreator> entry_creator,
-      AppCacheDiskCacheEntry** entry,
+      ServiceWorkerDiskCacheEntry** entry,
       int rv);
   static void DidDoomExistingEntry(
       base::WeakPtr<DiskEntryCreator> entry_creator,
       int rv);
   static void DidCreateEntryForSecondAttempt(
       base::WeakPtr<DiskEntryCreator> entry_creator,
-      AppCacheDiskCacheEntry** entry,
+      ServiceWorkerDiskCacheEntry** entry,
       int rv);
 
   void RunEnsureEntryIsCreatedCallback();
 
   const int64_t resource_id_;
-  base::WeakPtr<AppCacheDiskCache> disk_cache_;
-  AppCacheDiskCacheEntry* entry_ = nullptr;
+  base::WeakPtr<ServiceWorkerDiskCache> disk_cache_;
+  ServiceWorkerDiskCacheEntry* entry_ = nullptr;
 
   CreationPhase creation_phase_ = CreationPhase::kNoAttempt;
 
@@ -86,14 +86,14 @@ class DiskEntryCreator {
 class DiskEntryOpener {
  public:
   DiskEntryOpener(int64_t resource_id,
-                  base::WeakPtr<AppCacheDiskCache> disk_cache);
+                  base::WeakPtr<ServiceWorkerDiskCache> disk_cache);
   ~DiskEntryOpener();
 
   DiskEntryOpener(const DiskEntryOpener&) = delete;
   DiskEntryOpener& operator=(const DiskEntryOpener&) = delete;
 
   // Can be nullptr when a disk cache error occurs.
-  AppCacheDiskCacheEntry* entry() { return entry_; }
+  ServiceWorkerDiskCacheEntry* entry() { return entry_; }
 
   // Calls the callback when entry() is opened and can be used.
   //
@@ -106,12 +106,12 @@ class DiskEntryOpener {
   // TODO(crbug.com/586174): Refactor service worker's disk cache to use
   // disk_cache::EntryResult to make this callback non-static.
   static void DidOpenEntry(base::WeakPtr<DiskEntryOpener> entry_creator,
-                           AppCacheDiskCacheEntry** entry,
+                           ServiceWorkerDiskCacheEntry** entry,
                            int rv);
 
   const int64_t resource_id_;
-  base::WeakPtr<AppCacheDiskCache> disk_cache_;
-  AppCacheDiskCacheEntry* entry_ = nullptr;
+  base::WeakPtr<ServiceWorkerDiskCache> disk_cache_;
+  ServiceWorkerDiskCacheEntry* entry_ = nullptr;
 
   // Stored as a data member to handle //net-style maybe-async methods.
   base::OnceClosure ensure_entry_is_opened_callback_;
@@ -123,8 +123,9 @@ class DiskEntryOpener {
 class ServiceWorkerResourceReaderImpl
     : public storage::mojom::ServiceWorkerResourceReader {
  public:
-  ServiceWorkerResourceReaderImpl(int64_t resource_id,
-                                  base::WeakPtr<AppCacheDiskCache> disk_cache);
+  ServiceWorkerResourceReaderImpl(
+      int64_t resource_id,
+      base::WeakPtr<ServiceWorkerDiskCache> disk_cache);
 
   ServiceWorkerResourceReaderImpl(const ServiceWorkerResourceReaderImpl&) =
       delete;
@@ -189,8 +190,9 @@ class ServiceWorkerResourceReaderImpl
 class ServiceWorkerResourceWriterImpl
     : public storage::mojom::ServiceWorkerResourceWriter {
  public:
-  ServiceWorkerResourceWriterImpl(int64_t resource_id,
-                                  base::WeakPtr<AppCacheDiskCache> disk_cache);
+  ServiceWorkerResourceWriterImpl(
+      int64_t resource_id,
+      base::WeakPtr<ServiceWorkerDiskCache> disk_cache);
 
   ServiceWorkerResourceWriterImpl(const ServiceWorkerResourceWriterImpl&) =
       delete;
@@ -249,7 +251,7 @@ class ServiceWorkerResourceMetadataWriterImpl
  public:
   ServiceWorkerResourceMetadataWriterImpl(
       int64_t resource_id,
-      base::WeakPtr<AppCacheDiskCache> disk_cache);
+      base::WeakPtr<ServiceWorkerDiskCache> disk_cache);
 
   ServiceWorkerResourceMetadataWriterImpl(
       const ServiceWorkerResourceMetadataWriterImpl&) = delete;