[Quota] Evict orphaned quota buckets

If we wrote inactive buckets using a nonce, they will never be loaded
again. Let's start removing them, but put that behind a new feature to
prevent any issues.

Bug: 40281870, 353555346
Change-Id: Ia8fe624b680515db1695a2f58c1cd13efa97d84d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5771035
Auto-Submit: Ari Chivukula <arichiv@chromium.org>
Commit-Queue: Ayu Ishii <ayui@chromium.org>
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1340613}
diff --git a/storage/browser/quota/quota_database.cc b/storage/browser/quota/quota_database.cc
index 715a801..ba4d860 100644
--- a/storage/browser/quota/quota_database.cc
+++ b/storage/browser/quota/quota_database.cc
@@ -868,16 +868,20 @@
   }
   base::UmaHistogramCounts100000("Quota.StaleBucketCount", buckets_found);
 
-  // TODO(crbug.com/353555346): Merge this with the query above and start
-  // clearing storage. For now, we are just gathering metrics on orphaned
-  // storage (inactive quota buckets with a nonce which cannot be reused).
+  // Return early if we don't need to gather orphan buckets as well.
+  if (!base::FeatureList::IsEnabled(features::kEvictOrphanQuotaStorage)) {
+    return expired_buckets;
+  }
+
+  // We gather orphan buckets in a different fetch round so that we can count
+  // the amount found. After launch it may be worth merging these queries.
   // We only need to check for ^1 and ^4 are these are indicators for the
   // presence of a nonce in the storage key.
   // For more on StorageKey encoding see EncodedAttribute in
-  // clang-format off
   // third_party/blink/common/storage_key/storage_key.cc
+  // clang-format off
   static constexpr char kSqlOrphan[] =
-      "SELECT count(*) "
+      "SELECT " BUCKET_INFO_FIELDS_SELECTOR
         "FROM buckets "
         "WHERE storage_key REGEXP '.*\\^(1|4).*' AND "
               "last_accessed < ? AND last_modified < ?";
@@ -888,10 +892,13 @@
   base::Time orphan_cutoff = GetNow() - base::Days(1);
   statement_orphan.BindTime(0, orphan_cutoff);
   statement_orphan.BindTime(1, orphan_cutoff);
-  if (statement_orphan.Step()) {
-    base::UmaHistogramCounts100000("Quota.OrphanBucketCount",
-                                   statement_orphan.ColumnInt64(0));
+
+  buckets_found = 0;
+  while ((bucket = BucketInfoFromSqlStatement(statement_orphan)).has_value()) {
+    expired_buckets.insert(*bucket);
+    buckets_found++;
   }
+  base::UmaHistogramCounts100000("Quota.OrphanBucketCount", buckets_found);
 
   return expired_buckets;
 }
diff --git a/storage/browser/quota/quota_database_unittest.cc b/storage/browser/quota/quota_database_unittest.cc
index 3fadf67..463e093 100644
--- a/storage/browser/quota/quota_database_unittest.cc
+++ b/storage/browser/quota/quota_database_unittest.cc
@@ -77,15 +77,17 @@
 }  // namespace
 
 // Test parameter indicates if the database should be created for incognito
-// mode. True will create the database in memory.
+// mode, if stale buckets should be evicted, and if orphan buckets should be
+// evicted.
 class QuotaDatabaseTest
-    : public testing::TestWithParam<std::tuple<bool, bool>> {
+    : public testing::TestWithParam<std::tuple<bool, bool, bool>> {
  public:
   QuotaDatabaseTest() {
     clock_ = std::make_unique<base::SimpleTestClock>();
     QuotaDatabase::SetClockForTesting(clock_.get());
-    feature_list_.InitWithFeatureState(features::kEvictStaleQuotaStorage,
-                                       evict_stale_buckets());
+    feature_list_.InitWithFeatureStates(
+        {{features::kEvictStaleQuotaStorage, evict_stale_buckets()},
+         {features::kEvictOrphanQuotaStorage, evict_orphan_buckets()}});
   }
 
  protected:
@@ -102,6 +104,8 @@
 
   bool evict_stale_buckets() const { return std::get<1>(GetParam()); }
 
+  bool evict_orphan_buckets() const { return std::get<2>(GetParam()); }
+
   base::SimpleTestClock* clock() { return clock_.get(); }
 
   base::FilePath ProfilePath() { return temp_directory_.GetPath(); }
@@ -1393,8 +1397,9 @@
     EXPECT_EQ(0, histograms.GetTotalSum("Quota.OrphanBucketCount"));
     db.SetAlreadyEvictedStaleStorageForTesting(false);
     ASSERT_OK_AND_ASSIGN(buckets, db.GetExpiredBuckets(nullptr));
-    EXPECT_EQ(0U, buckets.size());
-    EXPECT_EQ(evict_stale_buckets() ? 1 : 0,
+    EXPECT_EQ((evict_stale_buckets() && evict_orphan_buckets()) ? 1U : 0U,
+              buckets.size());
+    EXPECT_EQ((evict_stale_buckets() && evict_orphan_buckets()) ? 1 : 0,
               histograms.GetTotalSum("Quota.OrphanBucketCount"));
   }
 
@@ -1411,8 +1416,9 @@
     db.SetAlreadyEvictedStaleStorageForTesting(false);
     ASSERT_OK_AND_ASSIGN(std::set<BucketInfo> buckets,
                          db.GetExpiredBuckets(nullptr));
-    EXPECT_EQ(0U, buckets.size());
-    EXPECT_EQ(evict_stale_buckets() ? 1 : 0,
+    EXPECT_EQ((evict_stale_buckets() && evict_orphan_buckets()) ? 1U : 0U,
+              buckets.size());
+    EXPECT_EQ((evict_stale_buckets() && evict_orphan_buckets()) ? 1 : 0,
               histograms.GetTotalSum("Quota.OrphanBucketCount"));
 
     EXPECT_EQ(db.SetBucketLastAccessTime(third_party_bucket.id,
@@ -1423,8 +1429,9 @@
               QuotaError::kNone);
     db.SetAlreadyEvictedStaleStorageForTesting(false);
     ASSERT_OK_AND_ASSIGN(buckets, db.GetExpiredBuckets(nullptr));
-    EXPECT_EQ(0U, buckets.size());
-    EXPECT_EQ(evict_stale_buckets() ? 2 : 0,
+    EXPECT_EQ((evict_stale_buckets() && evict_orphan_buckets()) ? 1 : 0U,
+              buckets.size());
+    EXPECT_EQ((evict_stale_buckets() && evict_orphan_buckets()) ? 2 : 0,
               histograms.GetTotalSum("Quota.OrphanBucketCount"));
   }
 
@@ -1444,8 +1451,9 @@
     db.SetAlreadyEvictedStaleStorageForTesting(false);
     ASSERT_OK_AND_ASSIGN(std::set<BucketInfo> buckets,
                          db.GetExpiredBuckets(nullptr));
-    EXPECT_EQ(0U, buckets.size());
-    EXPECT_EQ(evict_stale_buckets() ? 1 : 0,
+    EXPECT_EQ((evict_stale_buckets() && evict_orphan_buckets()) ? 1U : 0U,
+              buckets.size());
+    EXPECT_EQ((evict_stale_buckets() && evict_orphan_buckets()) ? 1 : 0,
               histograms.GetTotalSum("Quota.OrphanBucketCount"));
 
     EXPECT_EQ(db.SetBucketLastAccessTime(third_party_nonce_bucket.id,
@@ -1456,12 +1464,13 @@
               QuotaError::kNone);
     ASSERT_OK_AND_ASSIGN(buckets, db.GetExpiredBuckets(nullptr));
     EXPECT_EQ(0U, buckets.size());
-    EXPECT_EQ(evict_stale_buckets() ? 1 : 0,
+    EXPECT_EQ((evict_stale_buckets() && evict_orphan_buckets()) ? 1 : 0,
               histograms.GetTotalSum("Quota.OrphanBucketCount"));
     db.SetAlreadyEvictedStaleStorageForTesting(false);
     ASSERT_OK_AND_ASSIGN(buckets, db.GetExpiredBuckets(nullptr));
-    EXPECT_EQ(0U, buckets.size());
-    EXPECT_EQ(evict_stale_buckets() ? 3 : 0,
+    EXPECT_EQ((evict_stale_buckets() && evict_orphan_buckets()) ? 2U : 0U,
+              buckets.size());
+    EXPECT_EQ((evict_stale_buckets() && evict_orphan_buckets()) ? 3 : 0,
               histograms.GetTotalSum("Quota.OrphanBucketCount"));
   }
 }
@@ -1505,6 +1514,7 @@
     All,
     QuotaDatabaseTest,
     testing::Combine(/*use_in_memory_db=*/testing::Bool(),
-                     /*evict_stale_buckets=*/testing::Bool()));
+                     /*evict_stale_buckets=*/testing::Bool(),
+                     /*evict_orphan_buckets=*/testing::Bool()));
 
 }  // namespace storage
diff --git a/storage/browser/quota/quota_features.cc b/storage/browser/quota/quota_features.cc
index 02f068e..2fc93bc 100644
--- a/storage/browser/quota/quota_features.cc
+++ b/storage/browser/quota/quota_features.cc
@@ -55,6 +55,10 @@
     &kStorageQuotaSettings, "ShouldRemainAvailableRatio", 0.1 /* 10% */
 };
 
+BASE_FEATURE(kEvictOrphanQuotaStorage,
+             "EvictOrphanQuotaStorage",
+             base::FEATURE_DISABLED_BY_DEFAULT);
+
 BASE_FEATURE(kEvictStaleQuotaStorage,
              "EvictStaleQuotaStorage",
              base::FEATURE_DISABLED_BY_DEFAULT);
diff --git a/storage/browser/quota/quota_features.h b/storage/browser/quota/quota_features.h
index 5a0fce1..f3471d1 100644
--- a/storage/browser/quota/quota_features.h
+++ b/storage/browser/quota/quota_features.h
@@ -29,6 +29,12 @@
 extern const base::FeatureParam<double> kShouldRemainAvailableBytes;
 extern const base::FeatureParam<double> kShouldRemainAvailableRatio;
 
+// Clears quota storage for opaque origins used in prior browsing sessions as
+// they will no longer be reachable. See crbug.com/40281870 for more info.
+// If kEvictStaleQuotaStorage is off this has no impact.
+COMPONENT_EXPORT(STORAGE_BROWSER)
+BASE_DECLARE_FEATURE(kEvictOrphanQuotaStorage);
+
 // Clears quota storage last accessed/modified more than 400 days ago.
 // See crbug.com/40281870 for more info.
 COMPONENT_EXPORT(STORAGE_BROWSER)
diff --git a/testing/variations/fieldtrial_testing_config.json b/testing/variations/fieldtrial_testing_config.json
index 2c73099..1f8945f 100644
--- a/testing/variations/fieldtrial_testing_config.json
+++ b/testing/variations/fieldtrial_testing_config.json
@@ -22022,6 +22022,7 @@
                         "DeleteOrphanLocalStorageOnStartup",
                         "DeleteStaleLocalStorageOnStartup",
                         "DeleteStaleSessionCookiesOnStartup",
+                        "EvictOrphanQuotaStorage",
                         "EvictStaleQuotaStorage"
                     ]
                 }