Storage buckets: implement a bucket cap

Reject `navigator.storageBuckets.open()` when too many buckets already
exist. The bucket cap scales with the quota for the storage key.

Also update some error cases to match spec, e.g. reject with TypeError
when the bucket name isn't valid.

Fixed: 1413481
Change-Id: I5c9424e7c4a4882dc09cbda21d3a8f4f8ce6f57f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4194969
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Nathan Memmott <memmott@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1101890}

components/services/storage/public/cpp/quota_error_or.h

@@ -16,7 +16,8 @@ enum class QuotaError {
   kNotFound,
   kEntryExistsError,
   kFileOperationError,
-  kIllegalOperation,
+  kInvalidExpiration,
+  kQuotaExceeded,
 };
 
 // Helper for methods which perform database operations which may fail. Objects

content/browser/buckets/bucket_manager_host.cc

@@ -128,9 +128,31 @@ void BucketManagerHost::DidGetBucket(
     storage::QuotaErrorOr<storage::BucketInfo> result) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  if (!result.ok() || !bucket_context) {
-    // Getting a bucket can fail if there is a database error.
-    std::move(callback).Run(mojo::NullRemote());
+  if (!bucket_context) {
+    std::move(callback).Run(mojo::NullRemote(),
+                            blink::mojom::BucketError::kUnknown);
+    return;
+  }
+
+  if (!result.ok()) {
+    auto error = [](storage::QuotaError code) {
+      switch (code) {
+        case storage::QuotaError::kQuotaExceeded:
+          return blink::mojom::BucketError::kQuotaExceeded;
+        case storage::QuotaError::kInvalidExpiration:
+          return blink::mojom::BucketError::kInvalidExpiration;
+        case storage::QuotaError::kNone:
+        case storage::QuotaError::kNotFound:
+        case storage::QuotaError::kEntryExistsError:
+        case storage::QuotaError::kFileOperationError:
+          NOTREACHED();
+          ABSL_FALLTHROUGH_INTENDED;
+        case storage::QuotaError::kDatabaseError:
+        case storage::QuotaError::kUnknownError:
+          return blink::mojom::BucketError::kUnknown;
+      }
+    }(result.error());
+    std::move(callback).Run(mojo::NullRemote(), error);
     return;
   }
 
@@ -143,7 +165,8 @@ void BucketManagerHost::DidGetBucket(
   }
 
   auto pending_remote = it->second->CreateStorageBucketBinding(bucket_context);
-  std::move(callback).Run(std::move(pending_remote));
+  std::move(callback).Run(std::move(pending_remote),
+                          blink::mojom::BucketError::kUnknown);
 }
 
 void BucketManagerHost::DidGetBuckets(

content/browser/buckets/bucket_manager_host_unittest.cc

@@ -136,7 +136,8 @@ TEST_F(BucketManagerHostTest, OpenBucket) {
   bucket_manager_host_remote_->OpenBucket(
       "inbox_bucket", blink::mojom::BucketPolicies::New(),
       base::BindLambdaForTesting(
-          [&](mojo::PendingRemote<blink::mojom::BucketHost> remote) {
+          [&](mojo::PendingRemote<blink::mojom::BucketHost> remote,
+              blink::mojom::BucketError error) {
             EXPECT_TRUE(remote.is_valid());
             run_loop.Quit();
           }));
@@ -182,7 +183,8 @@ TEST_F(BucketManagerHostTest, OpenBucketValidateName) {
       remote->OpenBucket(
           it->second, blink::mojom::BucketPolicies::New(),
           base::BindLambdaForTesting(
-              [&](mojo::PendingRemote<blink::mojom::BucketHost> remote) {
+              [&](mojo::PendingRemote<blink::mojom::BucketHost> remote,
+                  blink::mojom::BucketError error) {
                 EXPECT_EQ(remote.is_valid(), it->first);
                 run_loop.Quit();
               }));
@@ -203,7 +205,8 @@ TEST_F(BucketManagerHostTest, DeleteBucket) {
   bucket_manager_host_remote_->OpenBucket(
       "inbox_bucket", blink::mojom::BucketPolicies::New(),
       base::BindLambdaForTesting(
-          [&](mojo::PendingRemote<blink::mojom::BucketHost> remote) {
+          [&](mojo::PendingRemote<blink::mojom::BucketHost> remote,
+              blink::mojom::BucketError error) {
             EXPECT_TRUE(remote.is_valid());
             run_loop.Quit();
           }));
@@ -250,7 +253,8 @@ TEST_F(BucketManagerHostTest, PermissionCheck) {
         bucket_manager_host_remote_->OpenBucket(
             "foo", blink::mojom::BucketPolicies::New(),
             base::BindLambdaForTesting(
-                [&](mojo::PendingRemote<blink::mojom::BucketHost> remote) {
+                [&](mojo::PendingRemote<blink::mojom::BucketHost> remote,
+                    blink::mojom::BucketError error) {
                   EXPECT_TRUE(remote.is_valid());
                   bucket_remote.Bind(std::move(remote));
                   run_loop.Quit();
@@ -297,7 +301,8 @@ TEST_F(BucketManagerHostTest, PermissionCheck) {
         bucket_manager_host_remote_->OpenBucket(
             "foo", std::move(policies),
             base::BindLambdaForTesting(
-                [&](mojo::PendingRemote<blink::mojom::BucketHost> remote) {
+                [&](mojo::PendingRemote<blink::mojom::BucketHost> remote,
+                    blink::mojom::BucketError error) {
                   EXPECT_TRUE(remote.is_valid());
                   bucket_remote2.Bind(std::move(remote));
                   run_loop.Quit();

storage/browser/file_system/obfuscated_file_util_unittest.cc

@@ -241,7 +241,7 @@ class ObfuscatedFileUtilTest : public testing::Test,
         FROM_HERE, base::BindOnce(
                        [](const scoped_refptr<QuotaManager>& quota_manager) {
                          QuotaSettings settings;
-                         settings.per_storage_key_quota = 25 * 1024 * 1024;
+                         settings.per_storage_key_quota = 250 * 1024 * 1024;
                          settings.pool_size =
                              settings.per_storage_key_quota * 5;
                          settings.must_remain_available = 10 * 1024 * 1024;

storage/browser/quota/quota_database.cc

@@ -220,7 +220,8 @@ QuotaDatabase::~QuotaDatabase() {
 constexpr char QuotaDatabase::kDatabaseName[];
 
 QuotaErrorOr<BucketInfo> QuotaDatabase::UpdateOrCreateBucket(
-    const BucketInitParams& params) {
+    const BucketInitParams& params,
+    int max_bucket_count) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   QuotaErrorOr<BucketInfo> bucket_result =
@@ -228,7 +229,8 @@ QuotaErrorOr<BucketInfo> QuotaDatabase::UpdateOrCreateBucket(
 
   if (!bucket_result.ok()) {
     if (bucket_result.error() == QuotaError::kNotFound) {
-      return CreateBucketInternal(params, StorageType::kTemporary);
+      return CreateBucketInternal(params, StorageType::kTemporary,
+                                  max_bucket_count);
     }
 
     return bucket_result;
@@ -1168,7 +1170,8 @@ QuotaError QuotaDatabase::DumpBucketTable(const BucketTableCallback& callback) {
 
 QuotaErrorOr<BucketInfo> QuotaDatabase::CreateBucketInternal(
     const BucketInitParams& params,
-    StorageType type) {
+    StorageType type,
+    int max_bucket_count) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   // TODO(crbug/1210259): Add DCHECKs for input validation.
   QuotaError open_error = EnsureOpened();
@@ -1176,6 +1179,33 @@ QuotaErrorOr<BucketInfo> QuotaDatabase::CreateBucketInternal(
     return open_error;
   }
 
+  // First verify this won't exceed the max bucket count if one is given.
+  if (max_bucket_count > 0) {
+    DCHECK_NE(params.name, kDefaultBucketName);
+    // Note that technically we should be filtering out default buckets when
+    // counting existing buckets so that the max count only applies to
+    // non-default buckets. However the precise bucket count is not that
+    // important and we don't want to perform a lot of string comparisons.
+    static constexpr char kSql[] =
+        // clang-format off
+        "SELECT count(*) "
+          "FROM buckets "
+          "WHERE storage_key = ? AND type = ?";
+    // clang-format on
+    sql::Statement statement(db_->GetCachedStatement(SQL_FROM_HERE, kSql));
+    statement.BindString(0, params.storage_key.Serialize());
+    statement.BindInt(1, static_cast<int>(type));
+
+    if (!statement.Step()) {
+      return QuotaError::kDatabaseError;
+    }
+
+    const int64_t current_bucket_count = statement.ColumnInt64(0);
+    if (current_bucket_count >= max_bucket_count) {
+      return QuotaError::kQuotaExceeded;
+    }
+  }
+
   static constexpr char kSql[] =
       // clang-format off
       "INSERT INTO buckets " BUCKETS_FIELDS_INSERTER

storage/browser/quota/quota_database.h

@@ -82,8 +82,12 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) QuotaDatabase {
   // `params`. If the bucket exists but policies don't match what's provided in
   // `params`, the existing bucket will be updated and returned (for those
   // policies that are possible to modify --- expiration and persistence).
-  // Returns a QuotaError if the operation has failed.
-  QuotaErrorOr<BucketInfo> UpdateOrCreateBucket(const BucketInitParams& params);
+  // Returns a QuotaError if the operation has failed. If `max_bucket_count` is
+  // greater than zero, and this operation would create a new bucket, then fail
+  // to create the new bucket if the total bucket count for this storage key is
+  // already at or above the max.
+  QuotaErrorOr<BucketInfo> UpdateOrCreateBucket(const BucketInitParams& params,
+                                                int max_bucket_count);
 
   // Same as UpdateOrCreateBucket but takes in StorageType. This should only
   // be used by FileSystem, and is expected to be removed when
@@ -272,9 +276,12 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) QuotaDatabase {
   QuotaError DumpBucketTable(const BucketTableCallback& callback);
 
   // Adds a new bucket entry in the buckets table. Will return a
-  // QuotaError::kDatabaseError if the query fails.
+  // QuotaError::kDatabaseError if the query fails. Will fail if adding the new
+  // bucket would cause the count of buckets for that storage key and type to
+  // exceed `max_bucket_count`, if `max_bucket_count` is greater than zero.
   QuotaErrorOr<BucketInfo> CreateBucketInternal(const BucketInitParams& params,
-                                                blink::mojom::StorageType type);
+                                                blink::mojom::StorageType type,
+                                                int max_bucket_count = 0);
 
   SEQUENCE_CHECKER(sequence_checker_);
 

storage/browser/quota/quota_database_unittest.cc

@@ -199,7 +199,7 @@ TEST_P(QuotaDatabaseTest, UpdateOrCreateBucket) {
       StorageKey::CreateFromStringForTesting("http://google/"),
       "google_bucket");
 
-  QuotaErrorOr<BucketInfo> result = db->UpdateOrCreateBucket(params);
+  QuotaErrorOr<BucketInfo> result = db->UpdateOrCreateBucket(params, 0);
   ASSERT_TRUE(result.ok());
 
   BucketInfo created_bucket = result.value();
@@ -209,7 +209,7 @@ TEST_P(QuotaDatabaseTest, UpdateOrCreateBucket) {
   ASSERT_EQ(created_bucket.type, kTemp);
 
   // Should return the same bucket when querying again.
-  result = db->UpdateOrCreateBucket(params);
+  result = db->UpdateOrCreateBucket(params, 0);
   ASSERT_TRUE(result.ok());
 
   BucketInfo retrieved_bucket = result.value();
@@ -217,6 +217,18 @@ TEST_P(QuotaDatabaseTest, UpdateOrCreateBucket) {
   ASSERT_EQ(retrieved_bucket.name, created_bucket.name);
   ASSERT_EQ(retrieved_bucket.storage_key, created_bucket.storage_key);
   ASSERT_EQ(retrieved_bucket.type, created_bucket.type);
+
+  // Test `max_bucket_count`.
+  BucketInitParams params2(
+      StorageKey::CreateFromStringForTesting("http://google/"),
+      "google_bucket2");
+  result = db->UpdateOrCreateBucket(params2, 1);
+  ASSERT_FALSE(result.ok());
+  EXPECT_EQ(QuotaError::kQuotaExceeded, result.error());
+
+  // It doesn't affect the update case.
+  result = db->UpdateOrCreateBucket(params, 1);
+  ASSERT_TRUE(result.ok());
 }
 
 TEST_P(QuotaDatabaseTest, UpdateBucket) {
@@ -228,7 +240,7 @@ TEST_P(QuotaDatabaseTest, UpdateBucket) {
   params.expiration = base::Time::Now() + base::Days(1);
   params.persistent = true;
 
-  QuotaErrorOr<BucketInfo> result = db->UpdateOrCreateBucket(params);
+  QuotaErrorOr<BucketInfo> result = db->UpdateOrCreateBucket(params, 0);
   ASSERT_TRUE(result.ok());
   BucketInfo created_bucket = result.value();
 
@@ -240,7 +252,7 @@ TEST_P(QuotaDatabaseTest, UpdateBucket) {
   params.persistent = false;
   params.quota = 1024 * 1024 * 20;  // 20 MB
   params.durability = blink::mojom::BucketDurability::kStrict;
-  result = db->UpdateOrCreateBucket(params);
+  result = db->UpdateOrCreateBucket(params, 0);
   ASSERT_TRUE(result.ok());
   BucketInfo updated_bucket = result.value();
 
@@ -265,7 +277,7 @@ TEST_P(QuotaDatabaseTest, UpdateBucket) {
   // Query, but without explicit policies.
   params.expiration = base::Time();
   params.persistent.reset();
-  result = db->UpdateOrCreateBucket(params);
+  result = db->UpdateOrCreateBucket(params, 0);
   ASSERT_TRUE(result.ok());
 
   // Expiration and persistence are unchanged.
@@ -1013,7 +1025,7 @@ TEST_F(QuotaDatabaseTest, QuotaDatabasePathMigration) {
   // Create database, add bucket and close by leaving scope.
   {
     auto db = CreateDatabase(/*is_incognito=*/false);
-    auto result = db->UpdateOrCreateBucket(params);
+    auto result = db->UpdateOrCreateBucket(params, 0);
     ASSERT_TRUE(result.ok());
   }
   // Move db file paths to legacy file path for path migration test setup.
@@ -1042,7 +1054,7 @@ TEST_F(QuotaDatabaseTest, QuotaDatabasePathBadMigration) {
   // Create database, add bucket and close by leaving scope.
   {
     auto db = CreateDatabase(/*is_incognito=*/false);
-    auto result = db->UpdateOrCreateBucket(params);
+    auto result = db->UpdateOrCreateBucket(params, 0);
     ASSERT_TRUE(result.ok());
   }
   // Copy db file paths to legacy file path to mimic bad migration state.
@@ -1074,9 +1086,9 @@ TEST_F(QuotaDatabaseTest, QuotaDatabaseDirectoryMigrationError) {
   {
     auto db = CreateDatabase(/*is_incognito=*/false);
     // Create two buckets to check that ids are different after database reset.
-    auto result = db->UpdateOrCreateBucket(google_params);
+    auto result = db->UpdateOrCreateBucket(google_params, 0);
     ASSERT_TRUE(result.ok());
-    result = db->UpdateOrCreateBucket(example_params);
+    result = db->UpdateOrCreateBucket(example_params, 0);
     ASSERT_TRUE(result.ok());
     example_id = result->id;
   }
@@ -1093,7 +1105,7 @@ TEST_F(QuotaDatabaseTest, QuotaDatabaseDirectoryMigrationError) {
     // Open database to trigger migration. Migration failure forces a database
     // reset.
     auto db = CreateDatabase(/*is_incognito=*/false);
-    auto result = db->UpdateOrCreateBucket(example_params);
+    auto result = db->UpdateOrCreateBucket(example_params, 0);
     ASSERT_TRUE(result.ok());
     // Validate database reset by checking that bucket id doesn't match.
     EXPECT_NE(result->id, example_id);
@@ -1108,7 +1120,7 @@ TEST_F(QuotaDatabaseTest, UpdateOrCreateBucket_CorruptedDatabase) {
       "google_bucket");
 
   {
-    QuotaErrorOr<BucketInfo> result = db.UpdateOrCreateBucket(params);
+    QuotaErrorOr<BucketInfo> result = db.UpdateOrCreateBucket(params, 0);
     ASSERT_TRUE(result.ok()) << "Failed to create bucket to be used in test";
   }
 
@@ -1124,7 +1136,7 @@ TEST_F(QuotaDatabaseTest, UpdateOrCreateBucket_CorruptedDatabase) {
   {
     base::HistogramTester histograms;
 
-    QuotaErrorOr<BucketInfo> result = db.UpdateOrCreateBucket(params);
+    QuotaErrorOr<BucketInfo> result = db.UpdateOrCreateBucket(params, 0);
     EXPECT_FALSE(result.ok());
 
     histograms.ExpectTotalCount("Quota.QuotaDatabaseError", 1);
@@ -1140,7 +1152,7 @@ TEST_P(QuotaDatabaseTest, Expiration) {
   BucketInitParams params(
       StorageKey::CreateFromStringForTesting("http://google/"),
       "google_bucket");
-  QuotaErrorOr<BucketInfo> result = db.UpdateOrCreateBucket(params);
+  QuotaErrorOr<BucketInfo> result = db.UpdateOrCreateBucket(params, 0);
   ASSERT_TRUE(result.ok());
   EXPECT_TRUE(result->expiration.is_null());
 
@@ -1153,7 +1165,7 @@ TEST_P(QuotaDatabaseTest, Expiration) {
       StorageKey::CreateFromStringForTesting("http://example/"),
       "example_bucket");
   params2.expiration = base::Time::Now();
-  result = db.UpdateOrCreateBucket(params2);
+  result = db.UpdateOrCreateBucket(params2, 0);
   ASSERT_TRUE(result.ok());
   EXPECT_EQ(params2.expiration, result->expiration);
 
@@ -1185,7 +1197,7 @@ TEST_P(QuotaDatabaseTest, Persistent) {
   BucketInitParams params(
       StorageKey::CreateFromStringForTesting("http://google/"),
       "google_bucket");
-  QuotaErrorOr<BucketInfo> result = db.UpdateOrCreateBucket(params);
+  QuotaErrorOr<BucketInfo> result = db.UpdateOrCreateBucket(params, 0);
   ASSERT_TRUE(result.ok());
   EXPECT_FALSE(params.persistent.has_value());
   EXPECT_FALSE(result->persistent);
@@ -1195,7 +1207,7 @@ TEST_P(QuotaDatabaseTest, Persistent) {
       StorageKey::CreateFromStringForTesting("http://example/"),
       "example_bucket");
   params2.persistent = !params2.persistent;
-  result = db.UpdateOrCreateBucket(params2);
+  result = db.UpdateOrCreateBucket(params2, 0);
   ASSERT_TRUE(result.ok());
   EXPECT_EQ(params2.persistent, result->persistent);
 

storage/browser/quota/quota_manager_impl.cc

@@ -211,12 +211,8 @@ class QuotaManagerImpl::UsageAndQuotaInfoGatherer : public QuotaTask {
                          weak_factory_.GetWeakPtr(), barrier));
       SetDesiredStorageKeyQuota(barrier, blink::mojom::QuotaStatusCode::kOk,
                                 kNoLimit);
-    } else if (type_ == StorageType::kSyncable) {
-      SetDesiredStorageKeyQuota(barrier, blink::mojom::QuotaStatusCode::kOk,
-                                kSyncableStorageDefaultStorageKeyQuota);
     } else {
-      DCHECK_EQ(StorageType::kTemporary, type_);
-      // For temporary storage,  OnGotSettings will set the host quota.
+      // For limited storage,  OnGotSettings will set the host quota.
     }
   }
 
@@ -283,14 +279,11 @@ class QuotaManagerImpl::UsageAndQuotaInfoGatherer : public QuotaTask {
 
     settings_ = settings;
     barrier_closure.Run();
-    if (type_ == StorageType::kTemporary && !is_unlimited_) {
-      int64_t storage_key_quota =
-          manager()->IsSessionOnly(storage_key_, type_)
-              ? settings.session_only_per_storage_key_quota
-              : settings.per_storage_key_quota;
+    const int64_t quota =
+        manager()->GetQuotaForStorageKey(storage_key_, type_, settings);
+    if (quota != kNoLimit) {
       SetDesiredStorageKeyQuota(std::move(barrier_closure),
-                                blink::mojom::QuotaStatusCode::kOk,
-                                storage_key_quota);
+                                blink::mojom::QuotaStatusCode::kOk, quota);
     }
   }
 
@@ -1039,19 +1032,29 @@ void QuotaManagerImpl::UpdateOrCreateBucket(
   }
   if (!bucket_params.expiration.is_null() &&
       (bucket_params.expiration <= QuotaDatabase::GetNow())) {
-    std::move(callback).Run(QuotaError::kIllegalOperation);
+    std::move(callback).Run(QuotaError::kInvalidExpiration);
     return;
   }
 
-  PostTaskAndReplyWithResultForDBThread(
-      base::BindOnce(
-          [](const BucketInitParams& params, QuotaDatabase* database) {
-            DCHECK(database);
-            return database->UpdateOrCreateBucket(params);
-          },
-          bucket_params),
-      base::BindOnce(&QuotaManagerImpl::DidGetBucketCheckExpiration,
-                     weak_factory_.GetWeakPtr(), bucket_params,
+  // The default bucket skips the quota check.
+  if (bucket_params.name == kDefaultBucketName) {
+    PostTaskAndReplyWithResultForDBThread(
+        base::BindOnce(
+            [](const BucketInitParams& params, QuotaDatabase* database) {
+              DCHECK(database);
+              return database->UpdateOrCreateBucket(params,
+                                                    /*max_bucket_count=*/0);
+            },
+            bucket_params),
+        base::BindOnce(&QuotaManagerImpl::DidGetBucketCheckExpiration,
+                       weak_factory_.GetWeakPtr(), bucket_params,
+                       std::move(callback)));
+    return;
+  }
+
+  GetQuotaSettings(
+      base::BindOnce(&QuotaManagerImpl::DidGetQuotaSettingsForBucketCreation,
+                     weak_factory_.GetWeakPtr(), std::move(bucket_params),
                      std::move(callback)));
 }
 
@@ -1640,13 +1643,6 @@ void QuotaManagerImpl::GetBucketUsageWithBreakdown(
   usage_tracker->GetBucketUsageWithBreakdown(bucket, std::move(callback));
 }
 
-bool QuotaManagerImpl::IsSessionOnly(const StorageKey& storage_key,
-                                     StorageType type) const {
-  return type == StorageType::kTemporary && special_storage_policy_ &&
-         special_storage_policy_->IsStorageSessionOnly(
-             storage_key.origin().GetURL());
-}
-
 bool QuotaManagerImpl::IsStorageUnlimited(const StorageKey& storage_key,
                                           StorageType type) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -1662,6 +1658,27 @@ bool QuotaManagerImpl::IsStorageUnlimited(const StorageKey& storage_key,
              storage_key.origin().GetURL());
 }
 
+int64_t QuotaManagerImpl::GetQuotaForStorageKey(
+    const StorageKey& storage_key,
+    StorageType type,
+    const QuotaSettings& settings) const {
+  if (IsStorageUnlimited(storage_key, type)) {
+    return kNoLimit;
+  }
+
+  if (type == StorageType::kSyncable) {
+    return kSyncableStorageDefaultStorageKeyQuota;
+  }
+
+  if (type == StorageType::kTemporary && special_storage_policy_ &&
+      special_storage_policy_->IsStorageSessionOnly(
+          storage_key.origin().GetURL())) {
+    return settings.session_only_per_storage_key_quota;
+  }
+
+  return settings.per_storage_key_quota;
+}
+
 void QuotaManagerImpl::GetBucketsModifiedBetween(StorageType type,
                                                  base::Time begin,
                                                  base::Time end,
@@ -2644,6 +2661,29 @@ void QuotaManagerImpl::OnComplete(QuotaError result) {
   DidDatabaseWork(result != QuotaError::kDatabaseError);
 }
 
+void QuotaManagerImpl::DidGetQuotaSettingsForBucketCreation(
+    const BucketInitParams& bucket_params,
+    base::OnceCallback<void(QuotaErrorOr<BucketInfo>)> callback,
+    const QuotaSettings& settings) {
+  const int64_t quota = GetQuotaForStorageKey(
+      bucket_params.storage_key, StorageType::kTemporary, settings);
+  int64_t max_buckets = (quota == kNoLimit) ? 0 : (quota / kTypicalBucketUsage);
+  DCHECK_EQ(max_buckets == 0, IsStorageUnlimited(bucket_params.storage_key,
+                                                 StorageType::kTemporary));
+
+  PostTaskAndReplyWithResultForDBThread(
+      base::BindOnce(
+          [](const BucketInitParams& params, int max_buckets,
+             QuotaDatabase* database) {
+            DCHECK(database);
+            return database->UpdateOrCreateBucket(params, max_buckets);
+          },
+          bucket_params, max_buckets),
+      base::BindOnce(&QuotaManagerImpl::DidGetBucketCheckExpiration,
+                     weak_factory_.GetWeakPtr(), bucket_params,
+                     std::move(callback)));
+}
+
 void QuotaManagerImpl::DidGetBucket(
     base::OnceCallback<void(QuotaErrorOr<BucketInfo>)> callback,
     QuotaErrorOr<BucketInfo> result) {

storage/browser/quota/quota_manager_impl.h

@@ -160,6 +160,13 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) QuotaManagerImpl
   static constexpr int64_t kGBytes = 1024 * 1024 * 1024;
   static constexpr int64_t kNoLimit = INT64_MAX;
   static constexpr int64_t kMBytes = 1024 * 1024;
+
+  // A "typical" amount of usage expected for a bucket. This is used to
+  // dynamically limit the number of buckets that may be created: the quota for
+  // a site divided by this number is an upper bound for the number of buckets
+  // it's allowed.
+  static constexpr int64_t kTypicalBucketUsage = 20 * kMBytes;
+
   static constexpr int kMinutesInMilliSeconds = 60 * 1000;
 
   QuotaManagerImpl(bool is_incognito,
@@ -393,12 +400,16 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) QuotaManagerImpl
                                    UsageWithBreakdownCallback callback);
   void GetBucketUsageAndQuota(BucketId id, UsageAndQuotaCallback callback);
 
-  bool IsSessionOnly(const blink::StorageKey& storage_key,
-                     blink::mojom::StorageType type) const;
-
   bool IsStorageUnlimited(const blink::StorageKey& storage_key,
                           blink::mojom::StorageType type) const;
 
+  // Calculates the quota for the given storage key, taking into account whether
+  // the storage should be session only for this key. This will return 0 for
+  // unlimited storage situations.
+  int64_t GetQuotaForStorageKey(const blink::StorageKey& storage_key,
+                                blink::mojom::StorageType type,
+                                const QuotaSettings& settings) const;
+
   virtual void GetBucketsModifiedBetween(blink::mojom::StorageType type,
                                          base::Time begin,
                                          base::Time end,
@@ -655,6 +666,10 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) QuotaManagerImpl
   void DidRazeForReBootstrap(QuotaError raze_and_reopen_result);
   void OnComplete(QuotaError result);
 
+  void DidGetQuotaSettingsForBucketCreation(
+      const BucketInitParams& bucket_params,
+      base::OnceCallback<void(QuotaErrorOr<BucketInfo>)> callback,
+      const QuotaSettings& settings);
   void DidGetBucket(base::OnceCallback<void(QuotaErrorOr<BucketInfo>)> callback,
                     QuotaErrorOr<BucketInfo> result);
   void DidGetBucketCheckExpiration(

storage/browser/quota/quota_manager_proxy_unittest.cc

@@ -27,7 +27,12 @@ class QuotaManagerProxyTest : public testing::Test {
         /*is_incognito*/ false, profile_path_.GetPath(),
         base::SingleThreadTaskRunner::GetCurrentDefault().get(),
         /*quota_change_callback=*/base::DoNothing(),
-        /*storage_policy=*/nullptr, GetQuotaSettingsFunc());
+        /*storage_policy=*/nullptr,
+        base::BindRepeating([](OptionalQuotaSettingsCallback callback) {
+          QuotaSettings settings;
+          settings.per_storage_key_quota = 200 * 1024 * 1024;
+          std::move(callback).Run(settings);
+        }));
     quota_manager_proxy_ = base::MakeRefCounted<QuotaManagerProxy>(
         quota_manager_.get(), base::SingleThreadTaskRunner::GetCurrentDefault(),
         profile_path_.GetPath());

storage/browser/quota/quota_manager_unittest.cc

@@ -75,7 +75,7 @@ const storage::mojom::StorageType kStorageSync =
 const int64_t kAvailableSpaceForApp = 13377331U;
 const int64_t kMustRemainAvailableForSystem = kAvailableSpaceForApp / 2;
 const int64_t kDefaultPoolSize = 1000;
-const int64_t kDefaultPerStorageKeyQuota = 200;
+const int64_t kDefaultPerStorageKeyQuota = 200 * 1024 * 1024;
 const int64_t kGigabytes = QuotaManagerImpl::kGBytes;
 
 struct UsageAndQuotaResult {
@@ -818,6 +818,28 @@ TEST_F(QuotaManagerImplTest, UpdateOrCreateBucket_Expiration) {
   QuotaDatabase::SetClockForTesting(nullptr);
 }
 
+TEST_F(QuotaManagerImplTest, UpdateOrCreateBucket_Overflow) {
+  const int kPoolSize = 100;
+  // This quota for the storage key implies only two buckets can be constructed.
+  const int kPerStorageKeyQuota = 40 * 1024 * 1024;
+  SetQuotaSettings(kPoolSize, kPerStorageKeyQuota,
+                   kMustRemainAvailableForSystem);
+
+  StorageKey storage_key = ToStorageKey("http://a.com/");
+
+  auto bucket_a = UpdateOrCreateBucket({storage_key, "bucket_a"});
+  EXPECT_TRUE(bucket_a.ok());
+  auto bucket_b = UpdateOrCreateBucket({storage_key, "bucket_b"});
+  EXPECT_TRUE(bucket_b.ok());
+  auto bucket_c = UpdateOrCreateBucket({storage_key, "bucket_c"});
+  EXPECT_FALSE(bucket_c.ok());
+  EXPECT_EQ(QuotaError::kQuotaExceeded, bucket_c.error());
+
+  // Default bucket shouldn't be limited by the quota.
+  auto bucket_default = UpdateOrCreateBucket({storage_key, "default"});
+  EXPECT_TRUE(bucket_default.ok());
+}
+
 // Make sure `EvictExpiredBuckets` deletes expired buckets.
 TEST_F(QuotaManagerImplTest, EvictExpiredBuckets) {
   auto clock = std::make_unique<base::SimpleTestClock>();

storage/browser/quota/quota_settings.h

@@ -83,11 +83,6 @@ void GetNominalDynamicSettings(const base::FilePath& partition_path,
 
 COMPONENT_EXPORT(STORAGE_BROWSER)
 
-// Returns settings with a poolsize of zero and no per StorageKey quota.
-inline QuotaSettings GetNoQuotaSettings() {
-  return QuotaSettings();
-}
-
 // Returns settings that provide given `per_storage_key_quota` and a total
 // poolsize of five times that.
 inline QuotaSettings GetHardCodedSettings(int64_t per_storage_key_quota) {

storage/browser/quota/quota_temporary_storage_evictor_unittest.cc

@@ -45,7 +45,7 @@ class MockQuotaEvictionHandler : public QuotaEvictionHandler {
   void EvictBucketData(const BucketLocator& bucket,
                        base::OnceCallback<void(QuotaError)> callback) override {
     if (error_on_evict_buckets_data_) {
-      std::move(callback).Run(QuotaError::kIllegalOperation);
+      std::move(callback).Run(QuotaError::kUnknownError);
       return;
     }
     int64_t bucket_usage = EnsureBucketRemoved(bucket);

third_party/blink/public/mojom/buckets/bucket_manager_host.mojom

@@ -22,6 +22,12 @@ enum BucketDurability {
   kStrict = 1,
 };
 
+enum BucketError {
+  kUnknown = 0,
+  kQuotaExceeded = 1,
+  kInvalidExpiration = 2,
+};
+
 // The policies applied to a StorageBucket upon its creation.
 struct BucketPolicies {
   bool persisted;
@@ -98,9 +104,10 @@ interface BucketManagerHost {
   // Open or create or bucket with the specified name and policies.
   // On success, it will return a mojo data pipe to the BucketHost in the
   // browser process. Returns a null remote on error, e.g. if the storage bucket
-  // failed to be created or retrieved due to a database error.
+  // failed to be created or retrieved due to a database error. `error` is only
+  // meaningful if `remote` is null.
   OpenBucket(string name, BucketPolicies policy)
-      => (pending_remote<BucketHost>? remote);
+      => (pending_remote<BucketHost>? remote, BucketError error);
 
   // Returns a list of stored bucket names in alphabetical order.
   Keys() => (array<string> buckets, bool success);

third_party/blink/renderer/modules/buckets/storage_bucket_manager.cc

@@ -15,6 +15,7 @@
 #include "third_party/blink/renderer/modules/buckets/storage_bucket.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
+#include "third_party/blink/renderer/platform/bindings/v8_throw_exception.h"
 #include "third_party/blink/renderer/platform/weborigin/security_origin.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
 #include "third_party/blink/renderer/platform/wtf/text/ascii_ctype.h"
@@ -112,8 +113,8 @@ ScriptPromise StorageBucketManager::open(ScriptState* script_state,
   }
 
   if (!IsValidName(name)) {
-    resolver->Reject(MakeGarbageCollected<DOMException>(
-        DOMExceptionCode::kInvalidCharacterError,
+    resolver->Reject(V8ThrowException::CreateTypeError(
+        script_state->GetIsolate(),
         "The bucket name '" + name + "' is not a valid name."));
     return promise;
   }
@@ -166,8 +167,8 @@ ScriptPromise StorageBucketManager::Delete(ScriptState* script_state,
   }
 
   if (!IsValidName(name)) {
-    resolver->Reject(MakeGarbageCollected<DOMException>(
-        DOMExceptionCode::kInvalidCharacterError,
+    resolver->Reject(V8ThrowException::CreateTypeError(
+        script_state->GetIsolate(),
         "The bucket name " + name + " is not a valid name."));
     return promise;
   }
@@ -194,7 +195,8 @@ mojom::blink::BucketManagerHost* StorageBucketManager::GetBucketManager(
 
 void StorageBucketManager::DidOpen(
     ScriptPromiseResolver* resolver,
-    mojo::PendingRemote<mojom::blink::BucketHost> bucket_remote) {
+    mojo::PendingRemote<mojom::blink::BucketHost> bucket_remote,
+    mojom::blink::BucketError error) {
   ScriptState* script_state = resolver->GetScriptState();
   if (!script_state->ContextIsValid()) {
     return;
@@ -202,11 +204,24 @@ void StorageBucketManager::DidOpen(
   ScriptState::Scope scope(script_state);
 
   if (!bucket_remote) {
-    resolver->Reject(MakeGarbageCollected<DOMException>(
-        DOMExceptionCode::kUnknownError,
-        "Unknown error occured while creating a bucket."));
-    return;
+    switch (error) {
+      case mojom::blink::BucketError::kUnknown:
+        resolver->Reject(MakeGarbageCollected<DOMException>(
+            DOMExceptionCode::kUnknownError,
+            "Unknown error occured while creating a bucket."));
+        return;
+      case mojom::blink::BucketError::kQuotaExceeded:
+        resolver->Reject(MakeGarbageCollected<DOMException>(
+            DOMExceptionCode::kQuotaExceededError,
+            "Too many buckets created."));
+        return;
+      case mojom::blink::BucketError::kInvalidExpiration:
+        resolver->Reject(V8ThrowException::CreateTypeError(
+            script_state->GetIsolate(), "The bucket expiration is invalid."));
+        return;
+    }
   }
+
   resolver->Resolve(MakeGarbageCollected<StorageBucket>(
       navigator_base_, std::move(bucket_remote)));
 }

third_party/blink/renderer/modules/buckets/storage_bucket_manager.h

@@ -51,7 +51,8 @@ class MODULES_EXPORT StorageBucketManager final
   mojom::blink::BucketManagerHost* GetBucketManager(ScriptState* script_state);
 
   void DidOpen(ScriptPromiseResolver* resolver,
-               mojo::PendingRemote<mojom::blink::BucketHost> bucket_remote);
+               mojo::PendingRemote<mojom::blink::BucketHost> bucket_remote,
+               mojom::blink::BucketError error);
   void DidGetKeys(ScriptPromiseResolver* resolver,
                   const Vector<String>& keys,
                   bool success);

third_party/blink/web_tests/wpt_internal/storage/buckets/bucket_names.tentative.https.any.js

@@ -45,8 +45,8 @@ kBadBucketNameTests.forEach(test_data => {
   const test_description = test_data[1];
 
   promise_test(async testCase => {
-    await promise_rejects_dom(
-        testCase, 'InvalidCharacterError',
+    await promise_rejects_js(
+        testCase, TypeError,
         navigator.storageBuckets.open(bucket_name));
   }, `open() throws an error if bucket names ${test_description}`);
 });
@@ -74,8 +74,8 @@ kBadBucketNameTests.forEach(test_data => {
   const test_description = test_data[1];
 
   promise_test(async testCase => {
-    await promise_rejects_dom(
-        testCase, 'InvalidCharacterError',
+    await promise_rejects_js(
+        testCase, TypeError,
         navigator.storageBuckets.delete(bucket_name));
   }, `delete() throws an error if bucket names ${test_description}`);
 });