[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"
]
}