Storage buckets: partial implementation of keys()

TODO: delete expired buckets when invoked.

Bug: 1335170
Change-Id: Icaf94c44117965db6532f0592bf3f1bfaf1d30dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3696094
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1013635}

components/services/storage/public/cpp/buckets/bucket_info.cc

@@ -42,4 +42,13 @@ bool operator<(const BucketInfo& lhs, const BucketInfo& rhs) {
   return lhs.id < rhs.id;
 }
 
+std::set<BucketLocator> COMPONENT_EXPORT(STORAGE_SERVICE_BUCKETS_SUPPORT)
+    BucketInfosToBucketLocators(const std::set<BucketInfo>& bucket_infos) {
+  std::set<BucketLocator> result;
+  std::transform(bucket_infos.begin(), bucket_infos.end(),
+                 std::inserter(result, result.begin()),
+                 [](const BucketInfo& info) { return info.ToBucketLocator(); });
+  return result;
+}
+
 }  // namespace storage

components/services/storage/public/cpp/buckets/bucket_info.h

@@ -64,6 +64,9 @@ struct COMPONENT_EXPORT(STORAGE_SERVICE_BUCKETS_SUPPORT) BucketInfo {
   blink::mojom::BucketDurability durability;
 };
 
+std::set<BucketLocator> COMPONENT_EXPORT(STORAGE_SERVICE_BUCKETS_SUPPORT)
+    BucketInfosToBucketLocators(const std::set<BucketInfo>& bucket_infos);
+
 }  // namespace storage
 
 #endif  // COMPONENTS_SERVICES_STORAGE_PUBLIC_CPP_BUCKETS_BUCKET_INFO_H_

content/browser/buckets/bucket_manager_host.cc

@@ -104,11 +104,11 @@ void BucketManagerHost::OpenBucket(const std::string& name,
 }
 
 void BucketManagerHost::Keys(KeysCallback callback) {
-  std::vector<std::string> keys;
-  for (auto& bucket : bucket_map_)
-    keys.push_back(bucket.first);
-  // TODO(ayui): Update to retrieve from QuotaManager.
-  std::move(callback).Run(keys, true);
+  manager_->quota_manager_proxy()->GetBucketsForStorageKey(
+      blink::StorageKey(origin_), blink::mojom::StorageType::kTemporary,
+      base::SequencedTaskRunnerHandle::Get(),
+      base::BindOnce(&BucketManagerHost::DidGetBuckets,
+                     weak_factory_.GetWeakPtr(), std::move(callback)));
 }
 
 void BucketManagerHost::DeleteBucket(const std::string& name,
@@ -167,6 +167,24 @@ void BucketManagerHost::DidGetBucket(
   std::move(callback).Run(std::move(pending_remote));
 }
 
+void BucketManagerHost::DidGetBuckets(
+    KeysCallback callback,
+    storage::QuotaErrorOr<std::set<storage::BucketInfo>> buckets) {
+  if (!buckets.ok()) {
+    std::move(callback).Run({}, false);
+    return;
+  }
+
+  std::vector<std::string> keys;
+  for (auto& bucket : buckets.value()) {
+    if (!bucket.is_default())
+      keys.push_back(bucket.name);
+  }
+  std::sort(keys.begin(), keys.end());
+
+  std::move(callback).Run(keys, true);
+}
+
 void BucketManagerHost::DidDeleteBucket(const std::string& bucket_name,
                                         DeleteBucketCallback callback,
                                         blink::mojom::QuotaStatusCode status) {

content/browser/buckets/bucket_manager_host.h

@@ -6,6 +6,7 @@
 #define CONTENT_BROWSER_BUCKETS_BUCKET_MANAGER_HOST_H_
 
 #include <map>
+#include <set>
 
 #include "base/memory/raw_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -80,6 +81,10 @@ class BucketManagerHost : public blink::mojom::BucketManagerHost {
                     OpenBucketCallback callback,
                     storage::QuotaErrorOr<storage::BucketInfo> result);
 
+  void DidGetBuckets(
+      KeysCallback callback,
+      storage::QuotaErrorOr<std::set<storage::BucketInfo>> result);
+
   void DidDeleteBucket(const std::string& bucket_name,
                        DeleteBucketCallback callback,
                        blink::mojom::QuotaStatusCode status);

storage/browser/quota/quota_database.cc

@@ -127,6 +127,16 @@ QuotaErrorOr<BucketInfo> BucketInfoFromSqlStatement(sql::Statement& statement) {
       static_cast<blink::mojom::BucketDurability>(statement.ColumnInt(7)));
 }
 
+std::set<BucketInfo> BucketInfosFromSqlStatement(sql::Statement& statement) {
+  std::set<BucketInfo> result;
+  QuotaErrorOr<BucketInfo> bucket;
+  while ((bucket = BucketInfoFromSqlStatement(statement)).ok()) {
+    result.insert(bucket.value());
+  }
+
+  return result;
+}
+
 }  // anonymous namespace
 
 const QuotaDatabase::TableSchema QuotaDatabase::kTables[] = {
@@ -410,7 +420,7 @@ QuotaErrorOr<BucketInfo> QuotaDatabase::GetBucketById(BucketId bucket_id) {
   return BucketInfoFromSqlStatement(statement);
 }
 
-QuotaErrorOr<std::set<BucketLocator>> QuotaDatabase::GetBucketsForType(
+QuotaErrorOr<std::set<BucketInfo>> QuotaDatabase::GetBucketsForType(
     StorageType type) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   QuotaError open_error = EnsureOpened();
@@ -418,53 +428,39 @@ QuotaErrorOr<std::set<BucketLocator>> QuotaDatabase::GetBucketsForType(
     return open_error;
 
   static constexpr char kSql[] =
-      "SELECT id, storage_key, name FROM buckets WHERE type = ?";
-
+      // clang-format off
+      "SELECT " BUCKET_INFO_FIELDS_SELECTOR
+        "FROM buckets "
+        "WHERE type = ?";
+  // clang-format on
   sql::Statement statement(db_->GetCachedStatement(SQL_FROM_HERE, kSql));
   statement.BindInt(0, static_cast<int>(type));
 
-  std::set<BucketLocator> buckets;
-  while (statement.Step()) {
-    absl::optional<StorageKey> read_storage_key =
-        StorageKey::Deserialize(statement.ColumnString(1));
-    if (!read_storage_key.has_value())
-      continue;
-    buckets.emplace(BucketId(statement.ColumnInt64(0)),
-                    read_storage_key.value(), type,
-                    statement.ColumnString(2) == kDefaultBucketName);
-  }
-  return buckets;
+  return BucketInfosFromSqlStatement(statement);
 }
 
-QuotaErrorOr<std::set<BucketLocator>> QuotaDatabase::GetBucketsForHost(
+QuotaErrorOr<std::set<BucketInfo>> QuotaDatabase::GetBucketsForHost(
     const std::string& host,
-    blink::mojom::StorageType storage_type) {
+    StorageType type) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   QuotaError open_error = EnsureOpened();
   if (open_error != QuotaError::kNone)
     return open_error;
 
   static constexpr char kSql[] =
-      "SELECT id, storage_key, name FROM buckets WHERE host = ? AND type = ?";
-
+      // clang-format off
+      "SELECT " BUCKET_INFO_FIELDS_SELECTOR
+        "FROM buckets "
+        "WHERE host = ? AND type = ?";
+  // clang-format on
   sql::Statement statement(db_->GetCachedStatement(SQL_FROM_HERE, kSql));
   statement.BindString(0, host);
-  statement.BindInt(1, static_cast<int>(storage_type));
+  statement.BindInt(1, static_cast<int>(type));
 
-  std::set<BucketLocator> buckets;
-  while (statement.Step()) {
-    absl::optional<StorageKey> read_storage_key =
-        StorageKey::Deserialize(statement.ColumnString(1));
-    if (!read_storage_key.has_value())
-      continue;
-    buckets.emplace(BucketId(statement.ColumnInt64(0)),
-                    read_storage_key.value(), storage_type,
-                    statement.ColumnString(2) == kDefaultBucketName);
-  }
-  return buckets;
+  return BucketInfosFromSqlStatement(statement);
 }
 
-QuotaErrorOr<std::set<BucketLocator>> QuotaDatabase::GetBucketsForStorageKey(
+QuotaErrorOr<std::set<BucketInfo>> QuotaDatabase::GetBucketsForStorageKey(
     const StorageKey& storage_key,
     StorageType type) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -473,18 +469,16 @@ QuotaErrorOr<std::set<BucketLocator>> QuotaDatabase::GetBucketsForStorageKey(
     return open_error;
 
   static constexpr char kSql[] =
-      "SELECT id, name FROM buckets WHERE storage_key = ? AND type = ?";
-
+      // clang-format off
+      "SELECT " BUCKET_INFO_FIELDS_SELECTOR
+        "FROM buckets "
+        "WHERE storage_key = ? AND type = ?";
+  // clang-format on
   sql::Statement statement(db_->GetCachedStatement(SQL_FROM_HERE, kSql));
   statement.BindString(0, storage_key.Serialize());
   statement.BindInt(1, static_cast<int>(type));
 
-  std::set<BucketLocator> buckets;
-  while (statement.Step()) {
-    buckets.emplace(BucketId(statement.ColumnInt64(0)), storage_key, type,
-                    statement.ColumnString(1) == kDefaultBucketName);
-  }
-  return buckets;
+  return BucketInfosFromSqlStatement(statement);
 }
 
 QuotaError QuotaDatabase::SetStorageKeyLastAccessTime(

storage/browser/quota/quota_database.h

@@ -153,18 +153,18 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) QuotaDatabase {
 
   // Returns all buckets for `type` in the buckets table. Returns a QuotaError
   // if the operation has failed.
-  QuotaErrorOr<std::set<BucketLocator>> GetBucketsForType(
+  QuotaErrorOr<std::set<BucketInfo>> GetBucketsForType(
       blink::mojom::StorageType type);
 
   // Retrieves all buckets for `host` and `type`. Returns a QuotaError if the
   // operation has failed.
-  QuotaErrorOr<std::set<BucketLocator>> GetBucketsForHost(
+  QuotaErrorOr<std::set<BucketInfo>> GetBucketsForHost(
       const std::string& host,
       blink::mojom::StorageType type);
 
   // Returns all buckets for `storage_key` in the buckets table. Returns a
   // QuotaError if the operation has failed.
-  QuotaErrorOr<std::set<BucketLocator>> GetBucketsForStorageKey(
+  QuotaErrorOr<std::set<BucketInfo>> GetBucketsForStorageKey(
       const blink::StorageKey& storage_key,
       blink::mojom::StorageType type);
 

storage/browser/quota/quota_database_unittest.cc

@@ -459,16 +459,16 @@ TEST_P(QuotaDatabaseTest, GetBucketsForType) {
   ASSERT_TRUE(bucket_result.ok());
   BucketInfo perm_bucket2 = bucket_result.value();
 
-  QuotaErrorOr<std::set<BucketLocator>> result = db->GetBucketsForType(kTemp);
+  QuotaErrorOr<std::set<BucketInfo>> result = db->GetBucketsForType(kTemp);
   ASSERT_TRUE(result.ok());
-  std::set<BucketLocator> buckets = result.value();
+  std::set<BucketLocator> buckets = BucketInfosToBucketLocators(result.value());
   ASSERT_EQ(2U, buckets.size());
   EXPECT_TRUE(ContainsBucket(buckets, temp_bucket1));
   EXPECT_TRUE(ContainsBucket(buckets, temp_bucket2));
 
   result = db->GetBucketsForType(kPerm);
   ASSERT_TRUE(result.ok());
-  buckets = result.value();
+  buckets = BucketInfosToBucketLocators(result.value());
   ASSERT_EQ(2U, buckets.size());
   EXPECT_TRUE(ContainsBucket(buckets, perm_bucket1));
   EXPECT_TRUE(ContainsBucket(buckets, perm_bucket2));
@@ -491,12 +491,12 @@ TEST_P(QuotaDatabaseTest, GetBucketsForHost) {
       StorageKey::CreateFromStringForTesting("http://google.com:123/"),
       "default", kTemp);
 
-  QuotaErrorOr<std::set<BucketLocator>> result =
+  QuotaErrorOr<std::set<BucketInfo>> result =
       db->GetBucketsForHost("example.com", kTemp);
   ASSERT_TRUE(result.ok());
   ASSERT_EQ(result->size(), 2U);
-  EXPECT_TRUE(ContainsBucket(result.value(), temp_example_bucket1.value()));
-  EXPECT_TRUE(ContainsBucket(result.value(), temp_example_bucket2.value()));
+  EXPECT_TRUE(base::Contains(result.value(), temp_example_bucket1.value()));
+  EXPECT_TRUE(base::Contains(result.value(), temp_example_bucket2.value()));
 
   result = db->GetBucketsForHost("example.com", kPerm);
   ASSERT_TRUE(result.ok());
@@ -505,12 +505,12 @@ TEST_P(QuotaDatabaseTest, GetBucketsForHost) {
   result = db->GetBucketsForHost("google.com", kPerm);
   ASSERT_TRUE(result.ok());
   ASSERT_EQ(result->size(), 1U);
-  EXPECT_TRUE(ContainsBucket(result.value(), perm_google_bucket1.value()));
+  EXPECT_TRUE(base::Contains(result.value(), perm_google_bucket1.value()));
 
   result = db->GetBucketsForHost("google.com", kTemp);
   ASSERT_TRUE(result.ok());
   ASSERT_EQ(result->size(), 1U);
-  EXPECT_TRUE(ContainsBucket(result.value(), temp_google_bucket2.value()));
+  EXPECT_TRUE(base::Contains(result.value(), temp_google_bucket2.value()));
 }
 
 TEST_P(QuotaDatabaseTest, GetBucketsForStorageKey) {
@@ -539,17 +539,17 @@ TEST_P(QuotaDatabaseTest, GetBucketsForStorageKey) {
   ASSERT_TRUE(bucket_result.ok());
   BucketInfo perm_bucket2 = bucket_result.value();
 
-  QuotaErrorOr<std::set<BucketLocator>> result =
+  QuotaErrorOr<std::set<BucketInfo>> result =
       db->GetBucketsForStorageKey(storage_key1, kTemp);
   ASSERT_TRUE(result.ok());
-  std::set<BucketLocator> buckets = result.value();
+  std::set<BucketLocator> buckets = BucketInfosToBucketLocators(result.value());
   ASSERT_EQ(2U, buckets.size());
   EXPECT_TRUE(ContainsBucket(buckets, temp_bucket1));
   EXPECT_TRUE(ContainsBucket(buckets, temp_bucket2));
 
   result = db->GetBucketsForStorageKey(storage_key2, kPerm);
   ASSERT_TRUE(result.ok());
-  buckets = result.value();
+  buckets = BucketInfosToBucketLocators(result.value());
   ASSERT_EQ(1U, buckets.size());
   EXPECT_TRUE(ContainsBucket(buckets, perm_bucket2));
 }

storage/browser/quota/quota_manager_impl.cc

@@ -848,14 +848,14 @@ class QuotaManagerImpl::HostDataDeleter {
   }
 
  private:
-  void DidGetBucketsForHost(QuotaErrorOr<std::set<BucketLocator>> result) {
+  void DidGetBucketsForHost(QuotaErrorOr<std::set<BucketInfo>> result) {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     if (!result.ok()) {
       Complete(/*success=*/false);
       return;
     }
 
-    buckets_ = result.value();
+    buckets_ = BucketInfosToBucketLocators(result.value());
     if (!buckets_.empty()) {
       ScheduleBucketsDeletion();
       return;
@@ -1209,7 +1209,7 @@ void QuotaManagerImpl::GetStorageKeysForType(blink::mojom::StorageType type,
 
 void QuotaManagerImpl::GetBucketsForType(
     blink::mojom::StorageType type,
-    base::OnceCallback<void(QuotaErrorOr<std::set<BucketLocator>>)> callback) {
+    base::OnceCallback<void(QuotaErrorOr<std::set<BucketInfo>>)> callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(callback);
   EnsureDatabaseOpened();
@@ -1232,7 +1232,7 @@ void QuotaManagerImpl::GetBucketsForType(
 void QuotaManagerImpl::GetBucketsForHost(
     const std::string& host,
     blink::mojom::StorageType type,
-    base::OnceCallback<void(QuotaErrorOr<std::set<BucketLocator>>)> callback) {
+    base::OnceCallback<void(QuotaErrorOr<std::set<BucketInfo>>)> callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(callback);
   EnsureDatabaseOpened();
@@ -1256,7 +1256,7 @@ void QuotaManagerImpl::GetBucketsForHost(
 void QuotaManagerImpl::GetBucketsForStorageKey(
     const StorageKey& storage_key,
     blink::mojom::StorageType type,
-    base::OnceCallback<void(QuotaErrorOr<std::set<BucketLocator>>)> callback) {
+    base::OnceCallback<void(QuotaErrorOr<std::set<BucketInfo>>)> callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(callback);
   EnsureDatabaseOpened();
@@ -2947,8 +2947,8 @@ void QuotaManagerImpl::DidGetStorageKeys(
 }
 
 void QuotaManagerImpl::DidGetBuckets(
-    base::OnceCallback<void(QuotaErrorOr<std::set<BucketLocator>>)> callback,
-    QuotaErrorOr<std::set<BucketLocator>> result) {
+    base::OnceCallback<void(QuotaErrorOr<std::set<BucketInfo>>)> callback,
+    QuotaErrorOr<std::set<BucketInfo>> result) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(callback);
 

storage/browser/quota/quota_manager_impl.h

@@ -235,21 +235,21 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) QuotaManagerImpl
   // Used for retrieving global usage data in the UsageTracker.
   void GetBucketsForType(
       blink::mojom::StorageType type,
-      base::OnceCallback<void(QuotaErrorOr<std::set<BucketLocator>>)> callback);
+      base::OnceCallback<void(QuotaErrorOr<std::set<BucketInfo>>)> callback);
 
   // Retrieves all buckets for `host` and `type` that are in the buckets table.
   // Used for retrieving host usage data in the UsageTracker.
   void GetBucketsForHost(
       const std::string& host,
       blink::mojom::StorageType type,
-      base::OnceCallback<void(QuotaErrorOr<std::set<BucketLocator>>)> callback);
+      base::OnceCallback<void(QuotaErrorOr<std::set<BucketInfo>>)> callback);
 
   // Retrieves all buckets for `storage_key` and `type` that are in the buckets
   // table. Used for retrieving storage key usage data in the UsageTracker.
   void GetBucketsForStorageKey(
       const blink::StorageKey& storage_key,
       blink::mojom::StorageType type,
-      base::OnceCallback<void(QuotaErrorOr<std::set<BucketLocator>>)> callback);
+      base::OnceCallback<void(QuotaErrorOr<std::set<BucketInfo>>)> callback);
 
   // Called by clients or webapps. Returns usage per host.
   void GetUsageInfo(GetUsageInfoCallback callback);
@@ -713,8 +713,8 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) QuotaManagerImpl
   void DidGetStorageKeys(GetStorageKeysCallback callback,
                          QuotaErrorOr<std::set<blink::StorageKey>> result);
   void DidGetBuckets(
-      base::OnceCallback<void(QuotaErrorOr<std::set<BucketLocator>>)> callback,
-      QuotaErrorOr<std::set<BucketLocator>> result);
+      base::OnceCallback<void(QuotaErrorOr<std::set<BucketInfo>>)> callback,
+      QuotaErrorOr<std::set<BucketInfo>> result);
   void DidGetModifiedBetween(GetBucketsCallback callback,
                              blink::mojom::StorageType type,
                              QuotaErrorOr<std::set<BucketLocator>> result);

storage/browser/quota/quota_manager_proxy.cc

@@ -252,6 +252,36 @@ void QuotaManagerProxy::GetBucket(
                                  std::move(respond));
 }
 
+void QuotaManagerProxy::GetBucketsForStorageKey(
+    const blink::StorageKey& storage_key,
+    blink::mojom::StorageType type,
+    scoped_refptr<base::SequencedTaskRunner> callback_task_runner,
+    base::OnceCallback<void(QuotaErrorOr<std::set<BucketInfo>>)> callback) {
+  DCHECK(callback_task_runner);
+  DCHECK(callback);
+
+  if (!quota_manager_impl_task_runner_->RunsTasksInCurrentSequence()) {
+    quota_manager_impl_task_runner_->PostTask(
+        FROM_HERE,
+        base::BindOnce(&QuotaManagerProxy::GetBucketsForStorageKey, this,
+                       storage_key, type, std::move(callback_task_runner),
+                       std::move(callback)));
+    return;
+  }
+
+  DCHECK_CALLED_ON_VALID_SEQUENCE(quota_manager_impl_sequence_checker_);
+
+  auto respond =
+      base::BindPostTask(std::move(callback_task_runner), std::move(callback));
+  if (!quota_manager_impl_) {
+    std::move(respond).Run(QuotaError::kUnknownError);
+    return;
+  }
+
+  quota_manager_impl_->GetBucketsForStorageKey(storage_key, type,
+                                               std::move(respond));
+}
+
 void QuotaManagerProxy::GetBucketById(
     const BucketId& bucket_id,
     scoped_refptr<base::SequencedTaskRunner> callback_task_runner,

storage/browser/quota/quota_manager_proxy.h

@@ -145,6 +145,14 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) QuotaManagerProxy
       scoped_refptr<base::SequencedTaskRunner> callback_task_runner,
       base::OnceCallback<void(QuotaErrorOr<BucketInfo>)> callback);
 
+  // Retrieves all buckets for `storage_key` and `type` that are in the buckets
+  // table.
+  virtual void GetBucketsForStorageKey(
+      const blink::StorageKey& storage_key,
+      blink::mojom::StorageType type,
+      scoped_refptr<base::SequencedTaskRunner> callback_task_runner,
+      base::OnceCallback<void(QuotaErrorOr<std::set<BucketInfo>>)> callback);
+
   // Deletes bucket with `bucket_name` for `storage_key` for
   // StorageType::kTemporary for all registered QuotaClients if a bucket exists.
   // Will return QuotaStatusCode to the callback. Called by Storage Buckets API

storage/browser/quota/quota_manager_unittest.cc

@@ -257,26 +257,26 @@ class QuotaManagerImplTest : public testing::Test {
     return future.Take();
   }
 
-  QuotaErrorOr<std::set<BucketLocator>> GetBucketsForType(
+  QuotaErrorOr<std::set<BucketInfo>> GetBucketsForType(
       blink::mojom::StorageType storage_type) {
-    base::test::TestFuture<QuotaErrorOr<std::set<BucketLocator>>> future;
+    base::test::TestFuture<QuotaErrorOr<std::set<BucketInfo>>> future;
     quota_manager_impl_->GetBucketsForType(storage_type, future.GetCallback());
     return future.Take();
   }
 
-  QuotaErrorOr<std::set<BucketLocator>> GetBucketsForHost(
+  QuotaErrorOr<std::set<BucketInfo>> GetBucketsForHost(
       const std::string& host,
       blink::mojom::StorageType storage_type) {
-    base::test::TestFuture<QuotaErrorOr<std::set<BucketLocator>>> future;
+    base::test::TestFuture<QuotaErrorOr<std::set<BucketInfo>>> future;
     quota_manager_impl_->GetBucketsForHost(host, storage_type,
                                            future.GetCallback());
     return future.Take();
   }
 
-  QuotaErrorOr<std::set<BucketLocator>> GetBucketsForStorageKey(
+  QuotaErrorOr<std::set<BucketInfo>> GetBucketsForStorageKey(
       const StorageKey& storage_key,
       blink::mojom::StorageType storage_type) {
-    base::test::TestFuture<QuotaErrorOr<std::set<BucketLocator>>> future;
+    base::test::TestFuture<QuotaErrorOr<std::set<BucketInfo>>> future;
     quota_manager_impl_->GetBucketsForStorageKey(storage_key, storage_type,
                                                  future.GetCallback());
     return future.Take();
@@ -961,18 +961,18 @@ TEST_F(QuotaManagerImplTest, GetBucketsForType) {
   EXPECT_TRUE(bucket.ok());
   BucketInfo bucket_c = bucket.value();
 
-  QuotaErrorOr<std::set<BucketLocator>> result = GetBucketsForType(kTemp);
+  QuotaErrorOr<std::set<BucketInfo>> result = GetBucketsForType(kTemp);
   EXPECT_TRUE(result.ok());
 
-  std::set<BucketLocator> buckets = result.value();
+  std::set<BucketInfo> buckets = result.value();
   EXPECT_EQ(2U, buckets.size());
-  EXPECT_THAT(buckets, testing::Contains(bucket_a.ToBucketLocator()));
-  EXPECT_THAT(buckets, testing::Contains(bucket_b.ToBucketLocator()));
+  EXPECT_THAT(buckets, testing::Contains(bucket_a));
+  EXPECT_THAT(buckets, testing::Contains(bucket_b));
 
   result = GetBucketsForType(kPerm);
   buckets = result.value();
   EXPECT_EQ(1U, buckets.size());
-  EXPECT_THAT(buckets, testing::Contains(bucket_c.ToBucketLocator()));
+  EXPECT_THAT(buckets, testing::Contains(bucket_c));
 }
 
 TEST_F(QuotaManagerImplTest, GetBucketsForHost) {
@@ -994,19 +994,18 @@ TEST_F(QuotaManagerImplTest, GetBucketsForHost) {
   EXPECT_TRUE(bucket.ok());
   BucketInfo host_b_bucket = bucket.value();
 
-  QuotaErrorOr<std::set<BucketLocator>> result =
-      GetBucketsForHost("a.com", kTemp);
+  QuotaErrorOr<std::set<BucketInfo>> result = GetBucketsForHost("a.com", kTemp);
   EXPECT_TRUE(result.ok());
 
-  std::set<BucketLocator> buckets = result.value();
+  std::set<BucketInfo> buckets = result.value();
   EXPECT_EQ(2U, buckets.size());
-  EXPECT_THAT(buckets, testing::Contains(host_a_bucket_1.ToBucketLocator()));
-  EXPECT_THAT(buckets, testing::Contains(host_a_bucket_2.ToBucketLocator()));
+  EXPECT_THAT(buckets, testing::Contains(host_a_bucket_1));
+  EXPECT_THAT(buckets, testing::Contains(host_a_bucket_2));
 
   result = GetBucketsForHost("b.com", kPerm);
   buckets = result.value();
   EXPECT_EQ(1U, buckets.size());
-  EXPECT_THAT(buckets, testing::Contains(host_b_bucket.ToBucketLocator()));
+  EXPECT_THAT(buckets, testing::Contains(host_b_bucket));
 }
 
 TEST_F(QuotaManagerImplTest, GetBucketsForStorageKey) {
@@ -1030,14 +1029,14 @@ TEST_F(QuotaManagerImplTest, GetBucketsForStorageKey) {
   EXPECT_TRUE(bucket.ok());
   BucketInfo bucket_c = bucket.value();
 
-  QuotaErrorOr<std::set<BucketLocator>> result =
+  QuotaErrorOr<std::set<BucketInfo>> result =
       GetBucketsForStorageKey(storage_key_a, kTemp);
   EXPECT_TRUE(result.ok());
 
-  std::set<BucketLocator> buckets = result.value();
+  std::set<BucketInfo> buckets = result.value();
   EXPECT_EQ(2U, buckets.size());
-  EXPECT_THAT(buckets, testing::Contains(bucket_a1.ToBucketLocator()));
-  EXPECT_THAT(buckets, testing::Contains(bucket_a2.ToBucketLocator()));
+  EXPECT_THAT(buckets, testing::Contains(bucket_a1));
+  EXPECT_THAT(buckets, testing::Contains(bucket_a2));
 
   result = GetBucketsForStorageKey(storage_key_a, kPerm);
   EXPECT_TRUE(result.ok());
@@ -1048,7 +1047,7 @@ TEST_F(QuotaManagerImplTest, GetBucketsForStorageKey) {
 
   buckets = result.value();
   EXPECT_EQ(1U, buckets.size());
-  EXPECT_THAT(buckets, testing::Contains(bucket_c.ToBucketLocator()));
+  EXPECT_THAT(buckets, testing::Contains(bucket_c));
 }
 
 TEST_F(QuotaManagerImplTest, GetUsageAndQuota_Simple) {

storage/browser/quota/usage_tracker.cc

@@ -193,7 +193,7 @@ void UsageTracker::SetUsageCacheEnabled(QuotaClientType client_type,
 }
 
 void UsageTracker::DidGetBucketsForType(
-    QuotaErrorOr<std::set<BucketLocator>> result) {
+    QuotaErrorOr<std::set<BucketInfo>> result) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto info = std::make_unique<AccumulateInfo>();
   if (!result.ok()) {
@@ -204,12 +204,15 @@ void UsageTracker::DidGetBucketsForType(
     return;
   }
 
-  const std::set<BucketLocator>& buckets = result.value();
+  const std::set<BucketInfo>& buckets = result.value();
   if (buckets.empty()) {
     FinallySendGlobalUsage(std::move(info));
     return;
   }
 
+  std::set<BucketLocator> bucket_locators =
+      BucketInfosToBucketLocators(buckets);
+
   auto* info_ptr = info.get();
   base::RepeatingClosure barrier = base::BarrierClosure(
       client_tracker_count_,
@@ -219,7 +222,7 @@ void UsageTracker::DidGetBucketsForType(
   for (const auto& client_type_and_trackers : client_tracker_map_) {
     for (const auto& client_tracker : client_type_and_trackers.second) {
       client_tracker->GetBucketsUsage(
-          buckets,
+          bucket_locators,
           // base::Unretained usage is safe here because BarrierClosure holds
           // the std::unque_ptr that keeps AccumulateInfo alive, and the
           // BarrierClosure will outlive all the AccumulateClientGlobalUsage
@@ -233,7 +236,7 @@ void UsageTracker::DidGetBucketsForType(
 
 void UsageTracker::DidGetBucketsForHost(
     const std::string& host,
-    QuotaErrorOr<std::set<BucketLocator>> result) {
+    QuotaErrorOr<std::set<BucketInfo>> result) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto info = std::make_unique<AccumulateInfo>();
   if (!result.ok()) {
@@ -244,12 +247,15 @@ void UsageTracker::DidGetBucketsForHost(
     return;
   }
 
-  const std::set<BucketLocator>& buckets = result.value();
+  const std::set<BucketInfo>& buckets = result.value();
   if (buckets.empty()) {
     FinallySendHostUsageWithBreakdown(std::move(info), host);
     return;
   }
 
+  std::set<BucketLocator> bucket_locators =
+      BucketInfosToBucketLocators(buckets);
+
   auto* info_ptr = info.get();
   base::RepeatingClosure barrier = base::BarrierClosure(
       client_tracker_count_,
@@ -259,7 +265,7 @@ void UsageTracker::DidGetBucketsForHost(
   for (const auto& client_type_and_trackers : client_tracker_map_) {
     for (const auto& client_tracker : client_type_and_trackers.second) {
       client_tracker->GetBucketsUsage(
-          buckets,
+          bucket_locators,
           // base::Unretained usage is safe here because BarrierClosure holds
           // the std::unque_ptr that keeps AccumulateInfo alive, and the
           // BarrierClosure will outlive all the AccumulateClientGlobalUsage
@@ -274,7 +280,7 @@ void UsageTracker::DidGetBucketsForHost(
 
 void UsageTracker::DidGetBucketsForStorageKey(
     const blink::StorageKey& storage_key,
-    QuotaErrorOr<std::set<BucketLocator>> result) {
+    QuotaErrorOr<std::set<BucketInfo>> result) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto info = std::make_unique<AccumulateInfo>();
   if (!result.ok()) {
@@ -285,12 +291,15 @@ void UsageTracker::DidGetBucketsForStorageKey(
     return;
   }
 
-  const std::set<BucketLocator>& buckets = result.value();
+  const std::set<BucketInfo>& buckets = result.value();
   if (buckets.empty()) {
     FinallySendStorageKeyUsageWithBreakdown(std::move(info), storage_key);
     return;
   }
 
+  std::set<BucketLocator> bucket_locators =
+      BucketInfosToBucketLocators(buckets);
+
   auto* info_ptr = info.get();
   base::RepeatingClosure barrier = base::BarrierClosure(
       client_tracker_count_,
@@ -300,7 +309,7 @@ void UsageTracker::DidGetBucketsForStorageKey(
   for (const auto& client_type_and_trackers : client_tracker_map_) {
     for (const auto& client_tracker : client_type_and_trackers.second) {
       client_tracker->GetBucketsUsage(
-          buckets,
+          bucket_locators,
           // base::Unretained usage is safe here because BarrierClosure holds
           // the std::unque_ptr that keeps AccumulateInfo alive, and the
           // BarrierClosure will outlive all the AccumulateClientGlobalUsage

storage/browser/quota/usage_tracker.h

@@ -122,11 +122,11 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) UsageTracker
   struct AccumulateInfo;
   friend class ClientUsageTracker;
 
-  void DidGetBucketsForType(QuotaErrorOr<std::set<BucketLocator>> result);
+  void DidGetBucketsForType(QuotaErrorOr<std::set<BucketInfo>> result);
   void DidGetBucketsForHost(const std::string& host,
-                            QuotaErrorOr<std::set<BucketLocator>> result);
+                            QuotaErrorOr<std::set<BucketInfo>> result);
   void DidGetBucketsForStorageKey(const blink::StorageKey& storage_key,
-                                  QuotaErrorOr<std::set<BucketLocator>> result);
+                                  QuotaErrorOr<std::set<BucketInfo>> result);
   void DidGetBucketForUsage(QuotaClientType client_type,
                             int64_t delta,
                             QuotaErrorOr<BucketInfo> result);