Add Storage Buckets to Cache Storage CDP

Adds support to the chrome devtools protocol to have an optional bucket
parameter for some Cache Storage API commands. If not specified, the
default bucket is assumed for backwards compatibility.

Bug: 1406017
Change-Id: I4a737dbecd0abfd324135902d47729fad9fbbac3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4297980
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Reviewed-by: Chris Bookholt <bookholt@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Nathan Memmott <memmott@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1146818}

components/services/storage/public/mojom/cache_storage_control.mojom

@@ -19,16 +19,14 @@ enum CacheStorageOwner {
   kBackgroundFetch,
 };
 
-// Observer interface for receiving callbacks after cache storage changes.
+// Observer interface for receiving callbacks after cache storage changes. This
+// is used by Devtools.
 interface CacheStorageObserver {
   // Called when caches are created or deleted.
-  // TODO(crbug.com/1218097): Pass bucket information also.
-  OnCacheListChanged(blink.mojom.StorageKey storage_key);
+  OnCacheListChanged(BucketLocator bucket_locator);
 
   // Called when the content of a cache has been modified.
-  // TODO(crbug.com/1218097): Pass bucket information also.
-  OnCacheContentChanged(blink.mojom.StorageKey storage_key,
-                        string cache_name);
+  OnCacheContentChanged(BucketLocator bucket_locator, string cache_name);
 };
 
 // Controls the state of CacheStorage within a partition. This is a privileged

content/browser/buckets/bucket_host.cc

@@ -47,6 +47,13 @@ BucketHost::CreateStorageBucketBinding(
   return remote;
 }
 
+void BucketHost::PassStorageBucketBinding(
+    base::WeakPtr<BucketContext> bucket_context,
+    mojo::PendingReceiver<blink::mojom::BucketHost> receiver) {
+  DCHECK(bucket_context);
+  receivers_.Add(this, std::move(receiver), bucket_context);
+}
+
 void BucketHost::Persist(PersistCallback callback) {
   if (bucket_info_.is_null() || !receivers_.current_context()) {
     std::move(callback).Run(false, /*success=*/false);
@@ -150,17 +157,19 @@ void BucketHost::Expires(ExpiresCallback callback) {
 void BucketHost::DidValidateForExpires(ExpiresCallback callback,
                                        bool bucket_exists) {
   absl::optional<base::Time> expires;
-  if (bucket_exists && !bucket_info_.expiration.is_null())
+  if (bucket_exists && !bucket_info_.expiration.is_null()) {
     expires = bucket_info_.expiration;
+  }
 
   std::move(callback).Run(expires, bucket_exists);
 }
 
 void BucketHost::GetIdbFactory(
     mojo::PendingReceiver<blink::mojom::IDBFactory> receiver) {
   auto bucket_context = receivers_.current_context();
-  if (!bucket_context)
+  if (!bucket_context) {
     return;
+  }
 
   GlobalRenderFrameHostId rfh_id =
       bucket_context->GetAssociatedRenderFrameHostId();
@@ -177,16 +186,18 @@ void BucketHost::GetIdbFactory(
 void BucketHost::GetCaches(
     mojo::PendingReceiver<blink::mojom::CacheStorage> caches) {
   auto bucket_context = receivers_.current_context();
-  if (!bucket_context)
+  if (!bucket_context) {
     return;
+  }
 
   bucket_context->BindCacheStorageForBucket(bucket_info_, std::move(caches));
 }
 
 void BucketHost::GetDirectory(GetDirectoryCallback callback) {
   auto bucket_context = receivers_.current_context();
-  if (!bucket_context)
+  if (!bucket_context) {
     return;
+  }
 
   bucket_context->GetSandboxedFileSystemForBucket(bucket_info_,
                                                   std::move(callback));
@@ -199,8 +210,9 @@ void BucketHost::GetLockManager(
 }
 
 void BucketHost::OnReceiverDisconnected() {
-  if (!receivers_.empty())
+  if (!receivers_.empty()) {
     return;
+  }
   // Destroys `this`.
   bucket_manager_host_->RemoveBucketHost(bucket_id_);
 }

content/browser/buckets/bucket_host.h

@@ -43,6 +43,10 @@ class BucketHost : public blink::mojom::BucketHost {
   // for the StorageBucket object.
   mojo::PendingRemote<blink::mojom::BucketHost> CreateStorageBucketBinding(
       base::WeakPtr<BucketContext> context);
+  // Pass a mojo data pipe sent from the renderer for the StorageBucket object.
+  void PassStorageBucketBinding(
+      base::WeakPtr<BucketContext> bucket_context,
+      mojo::PendingReceiver<blink::mojom::BucketHost> receiver);
 
   // blink::mojom::BucketHost
   void Persist(PersistCallback callback) override;

content/browser/buckets/bucket_manager_host.cc

@@ -11,6 +11,7 @@
 #include "base/task/sequenced_task_runner.h"
 #include "base/types/pass_key.h"
 #include "components/services/storage/public/cpp/buckets/bucket_info.h"
+#include "content/browser/buckets/bucket_host.h"
 #include "content/browser/buckets/bucket_manager.h"
 #include "content/browser/buckets/bucket_utils.h"
 #include "content/browser/storage_partition_impl.h"
@@ -131,13 +132,13 @@ void BucketManagerHost::OpenBucket(const std::string& name,
 
 void BucketManagerHost::GetBucketForDevtools(
     const std::string& name,
-    GetBucketForDevtoolsCallback callback) {
+    mojo::PendingReceiver<blink::mojom::BucketHost> receiver) {
   GetQuotaManagerProxy()->GetBucketByNameUnsafe(
       storage_key_, name, blink::mojom::StorageType::kTemporary,
       base::SequencedTaskRunner::GetCurrentDefault(),
-      base::BindOnce(&BucketManagerHost::DidGetBucket,
+      base::BindOnce(&BucketManagerHost::DidGetBucketForDevtools,
                      weak_factory_.GetWeakPtr(), receivers_.current_context(),
-                     std::move(callback)));
+                     std::move(receiver)));
 }
 
 void BucketManagerHost::Keys(KeysCallback callback) {
@@ -226,6 +227,27 @@ void BucketManagerHost::DidGetBucket(
                           blink::mojom::BucketError::kUnknown);
 }
 
+void BucketManagerHost::DidGetBucketForDevtools(
+    base::WeakPtr<BucketContext> bucket_context,
+    mojo::PendingReceiver<blink::mojom::BucketHost> receiver,
+    storage::QuotaErrorOr<storage::BucketInfo> result) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+  if (!bucket_context || !result.has_value()) {
+    return;
+  }
+
+  const auto& bucket = result.value();
+  auto it = bucket_map_.find(bucket.id);
+  if (it == bucket_map_.end()) {
+    it = bucket_map_
+             .emplace(bucket.id, std::make_unique<BucketHost>(this, bucket))
+             .first;
+  }
+
+  it->second->PassStorageBucketBinding(bucket_context, std::move(receiver));
+}
+
 void BucketManagerHost::DidGetBuckets(
     KeysCallback callback,
     storage::QuotaErrorOr<std::set<storage::BucketInfo>> buckets) {

content/browser/buckets/bucket_manager_host.h

@@ -66,8 +66,9 @@ class BucketManagerHost : public blink::mojom::BucketManagerHost {
                   OpenBucketCallback callback) override;
   // Gets the bucket with the given name. Doesn't create the bucket if it
   // doesn't exist.
-  void GetBucketForDevtools(const std::string& name,
-                            GetBucketForDevtoolsCallback callback) override;
+  void GetBucketForDevtools(
+      const std::string& name,
+      mojo::PendingReceiver<blink::mojom::BucketHost> receiver) override;
   void Keys(KeysCallback callback) override;
   void DeleteBucket(const std::string& name,
                     DeleteBucketCallback callback) override;
@@ -95,7 +96,7 @@ class BucketManagerHost : public blink::mojom::BucketManagerHost {
 
   void DidGetBucketForDevtools(
       base::WeakPtr<BucketContext> bucket_context,
-      GetBucketForDevtoolsCallback callback,
+      mojo::PendingReceiver<blink::mojom::BucketHost> receiver,
       storage::QuotaErrorOr<storage::BucketInfo> result);
 
   SEQUENCE_CHECKER(sequence_checker_);

content/browser/cache_storage/cache_storage_manager.cc

@@ -134,12 +134,14 @@ enum class IndexResult {
 };
 
 IndexResult ValidateIndex(proto::CacheStorageIndex index) {
-  if (!index.has_origin())
+  if (!index.has_origin()) {
     return IndexResult::kMissingOrigin;
+  }
 
   GURL url(index.origin());
-  if (url.is_empty())
+  if (url.is_empty()) {
     return IndexResult::kEmptyOriginUrl;
+  }
 
   // TODO(https://crbug.com/1199077): Consider adding a
   // 'index.has_storage_key()' check here once we've ensured that a
@@ -183,8 +185,9 @@ void ValidateAndAddUsageFromPath(
       index_file_directory_path.AppendASCII(CacheStorage::kIndexFileName);
   base::File::Info file_info;
   base::Time index_last_modified;
-  if (GetFileInfo(index_path, &file_info))
+  if (GetFileInfo(index_path, &file_info)) {
     index_last_modified = file_info.last_modified;
+  }
   std::string protobuf;
   base::ReadFileToString(index_path, &protobuf);
 
@@ -265,8 +268,9 @@ void ValidateAndAddUsageFromPath(
               : storage::mojom::CacheStorageOwner::kCacheAPI;
       auto other_owner_path = CacheStorageManager::ConstructBucketPath(
           profile_path, bucket_locator, other_owner);
-      if (index_file_directory_path == other_owner_path)
+      if (index_file_directory_path == other_owner_path) {
         return;
+      }
     }
     RecordIndexValidationResult(IndexResult::kPathMismatch);
     return;
@@ -617,16 +621,18 @@ CacheStorageHandle CacheStorageManager::OpenCacheStorage(
 void CacheStorageManager::NotifyCacheListChanged(
     const storage::BucketLocator& bucket_locator) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  for (const auto& observer : observers_)
-    observer->OnCacheListChanged(bucket_locator.storage_key);
+  for (const auto& observer : observers_) {
+    observer->OnCacheListChanged(bucket_locator);
+  }
 }
 
 void CacheStorageManager::NotifyCacheContentChanged(
     const storage::BucketLocator& bucket_locator,
     const std::string& name) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  for (const auto& observer : observers_)
-    observer->OnCacheContentChanged(bucket_locator.storage_key, name);
+  for (const auto& observer : observers_) {
+    observer->OnCacheContentChanged(bucket_locator, name);
+  }
 }
 
 void CacheStorageManager::CacheStorageUnreferenced(
@@ -657,8 +663,9 @@ void CacheStorageManager::GetAllStorageKeysUsage(
 
   if (IsMemoryBacked()) {
     for (const auto& bucket_details : cache_storage_map_) {
-      if (bucket_details.first.second != owner)
+      if (bucket_details.first.second != owner) {
         continue;
+      }
       const storage::BucketLocator& bucket_locator = bucket_details.first.first;
       usages.emplace_back(bucket_locator, storage::mojom::StorageUsageInfo::New(
                                               bucket_locator.storage_key,
@@ -790,12 +797,14 @@ void CacheStorageManager::GetStorageKeys(
   if (IsMemoryBacked()) {
     std::vector<blink::StorageKey> storage_keys;
     for (const auto& key_value : cache_storage_map_) {
-      if (key_value.first.second != owner)
+      if (key_value.first.second != owner) {
         continue;
+      }
 
       const storage::BucketLocator& bucket_locator = key_value.first.first;
-      if (!bucket_locator.is_default)
+      if (!bucket_locator.is_default) {
         continue;
+      }
 
       storage_keys.push_back(bucket_locator.storage_key);
     }
@@ -926,8 +935,9 @@ void CacheStorageManager::DeleteOriginData(
     // search for the corresponding `storage::BucketLocator` keys, given a
     // `blink::StorageKey`.
     for (const auto& key_value : cache_storage_map_) {
-      if (key_value.first.second != owner)
+      if (key_value.first.second != owner) {
         continue;
+      }
       const storage::BucketLocator& bucket_locator = key_value.first.first;
       if (!BucketMatchesOriginsForDeletion(bucket_locator, origins)) {
         continue;
@@ -1055,8 +1065,9 @@ void CacheStorageManager::DeleteBucketDidClose(
       -bucket_size, base::Time::Now(),
       base::SequencedTaskRunner::GetCurrentDefault(), base::DoNothing());
 
-  if (owner == storage::mojom::CacheStorageOwner::kCacheAPI)
+  if (owner == storage::mojom::CacheStorageOwner::kCacheAPI) {
     NotifyCacheListChanged(bucket_locator);
+  }
 
   if (IsMemoryBacked()) {
     scheduler_task_runner_->PostTask(
@@ -1132,8 +1143,9 @@ bool CacheStorageManager::IsValidQuotaStorageKey(
 void CacheStorageManager::OnMemoryPressure(
     base::MemoryPressureListener::MemoryPressureLevel level) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (level != base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL)
+  if (level != base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL) {
     return;
+  }
 
   for (auto& entry : cache_storage_map_) {
     entry.second->ReleaseUnreferencedCaches();