Reland "Quota: Add SequenceChecker to QuotaManager"
This is a reland of d82ad3d0eee4f2b98d6f7be7fb2489c55713da69
In the original CL, QuotaManager was modified in two ways:
1. Added SequenceChecker
2. Remove custom deleter and inherit RefCountedDeleteOnSequence
In the reland, I split out the latter into a separate CL and this CL
only adds SequenceChecker to QuotaManager. This was done to
minimize the impact of a potential revert.
RefCountedDeleteOnSequence CL: https://crrev.com/c/1481591
Original change's description:
> Quota: Add SequenceChecker to QuotaManager
>
> Change-Id: I7caa60bf82fdcd63469d58afa0e29f893795ae32
> Reviewed-on: https://chromium-review.googlesource.com/c/1457000
> Commit-Queue: Victor Costan <pwnall@chromium.org>
> Reviewed-by: Victor Costan <pwnall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#631891}
Change-Id: Ia6076d9076e0e0f646b4286a8533ca0a964e3903
Reviewed-on: https://chromium-review.googlesource.com/c/1479302
Commit-Queue: Jarryd Goodman <jarrydg@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634892}diff --git a/storage/browser/quota/quota_manager.cc b/storage/browser/quota/quota_manager.cc
index 4bd78c4..bf98887 100644
--- a/storage/browser/quota/quota_manager.cc
+++ b/storage/browser/quota/quota_manager.cc
@@ -901,14 +901,18 @@
settings_.refresh_interval = base::TimeDelta();
get_settings_task_runner_ = base::ThreadTaskRunnerHandle::Get();
}
+ DETACH_FROM_SEQUENCE(sequence_checker_);
}
void QuotaManager::SetQuotaSettings(const QuotaSettings& settings) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
settings_ = settings;
settings_timestamp_ = base::TimeTicks::Now();
}
void QuotaManager::GetUsageInfo(GetUsageInfoCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
GetUsageInfoTask* get_usage_info =
new GetUsageInfoTask(this, std::move(callback));
@@ -918,6 +922,7 @@
void QuotaManager::GetUsageAndQuotaForWebApps(const url::Origin& origin,
StorageType type,
UsageAndQuotaCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
GetUsageAndQuotaWithBreakdown(
origin, type,
base::BindOnce(&DidGetUsageAndQuotaForWebApps, std::move(callback)));
@@ -927,6 +932,7 @@
const url::Origin& origin,
StorageType type,
UsageAndQuotaWithBreakdownCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!IsSupportedType(type) ||
(is_incognito_ && !IsSupportedIncognitoType(type))) {
std::move(callback).Run(
@@ -950,6 +956,7 @@
void QuotaManager::GetUsageAndQuota(const url::Origin& origin,
StorageType type,
UsageAndQuotaCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (IsStorageUnlimited(origin, type)) {
// TODO(michaeln): This seems like a non-obvious odd behavior, probably for
// apps/extensions, but it would be good to eliminate this special case.
@@ -963,6 +970,7 @@
void QuotaManager::NotifyStorageAccessed(QuotaClient::ID client_id,
const url::Origin& origin,
StorageType type) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
NotifyStorageAccessedInternal(client_id, origin, type, base::Time::Now());
}
@@ -970,16 +978,19 @@
const url::Origin& origin,
StorageType type,
int64_t delta) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
NotifyStorageModifiedInternal(client_id, origin, type, delta,
base::Time::Now());
}
void QuotaManager::NotifyOriginInUse(const url::Origin& origin) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(io_thread_->BelongsToCurrentThread());
origins_in_use_[origin]++;
}
void QuotaManager::NotifyOriginNoLongerInUse(const url::Origin& origin) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(io_thread_->BelongsToCurrentThread());
DCHECK(IsOriginInUse(origin));
int& count = origins_in_use_[origin];
@@ -991,6 +1002,7 @@
const url::Origin& origin,
StorageType type,
bool enabled) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
DCHECK(GetUsageTracker(type));
GetUsageTracker(type)->SetUsageCacheEnabled(client_id, origin, enabled);
@@ -1000,6 +1012,7 @@
StorageType type,
int quota_client_mask,
StatusCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DeleteOriginDataInternal(origin, type, quota_client_mask, false,
std::move(callback));
}
@@ -1007,6 +1020,7 @@
void QuotaManager::PerformStorageCleanup(StorageType type,
int quota_client_mask,
base::OnceClosure callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
StorageCleanupHelper* deleter = new StorageCleanupHelper(
this, type, quota_client_mask, std::move(callback));
deleter->Start();
@@ -1016,6 +1030,7 @@
StorageType type,
int quota_client_mask,
StatusCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
if (host.empty() || clients_.empty()) {
std::move(callback).Run(blink::mojom::QuotaStatusCode::kOk);
@@ -1029,6 +1044,7 @@
void QuotaManager::GetPersistentHostQuota(const std::string& host,
QuotaCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
if (host.empty()) {
// This could happen if we are called on file:///.
@@ -1053,6 +1069,7 @@
void QuotaManager::SetPersistentHostQuota(const std::string& host,
int64_t new_quota,
QuotaCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
if (host.empty()) {
// This could happen if we are called on file:///.
@@ -1088,6 +1105,7 @@
void QuotaManager::GetGlobalUsage(StorageType type,
GlobalUsageCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
DCHECK(GetUsageTracker(type));
GetUsageTracker(type)->GetGlobalUsage(std::move(callback));
@@ -1096,6 +1114,7 @@
void QuotaManager::GetHostUsage(const std::string& host,
StorageType type,
UsageCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
DCHECK(GetUsageTracker(type));
GetUsageTracker(type)->GetHostUsage(host, std::move(callback));
@@ -1105,6 +1124,7 @@
StorageType type,
QuotaClient::ID client_id,
UsageCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
DCHECK(GetUsageTracker(type));
ClientUsageTracker* tracker =
@@ -1120,6 +1140,7 @@
const std::string& host,
StorageType type,
UsageWithBreakdownCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
DCHECK(GetUsageTracker(type));
GetUsageTracker(type)->GetHostUsageWithBreakdown(host, std::move(callback));
@@ -1127,11 +1148,13 @@
bool QuotaManager::IsTrackingHostUsage(StorageType type,
QuotaClient::ID client_id) const {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
UsageTracker* tracker = GetUsageTracker(type);
return tracker && tracker->GetClientTracker(client_id);
}
std::map<std::string, std::string> QuotaManager::GetStatistics() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
std::map<std::string, std::string> statistics;
if (temporary_storage_evictor_) {
std::map<std::string, int64_t> stats;
@@ -1146,6 +1169,7 @@
bool QuotaManager::IsStorageUnlimited(const url::Origin& origin,
StorageType type) const {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// For syncable storage we should always enforce quota (since the
// quota must be capped by the server limit).
if (type == StorageType::kSyncable)
@@ -1159,6 +1183,7 @@
void QuotaManager::GetOriginsModifiedSince(StorageType type,
base::Time modified_since,
GetOriginsCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
GetModifiedSinceHelper* helper = new GetModifiedSinceHelper;
PostTaskAndReplyWithResultForDBThread(
@@ -1171,6 +1196,7 @@
}
bool QuotaManager::ResetUsageTracker(StorageType type) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(GetUsageTracker(type));
if (GetUsageTracker(type)->IsWorking())
return false;
@@ -1198,11 +1224,13 @@
void QuotaManager::AddStorageObserver(
StorageObserver* observer, const StorageObserver::MonitorParams& params) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(observer);
storage_monitor_->AddObserver(observer, params);
}
void QuotaManager::RemoveStorageObserver(StorageObserver* observer) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(observer);
storage_monitor_->RemoveObserver(observer);
}
@@ -1221,6 +1249,7 @@
QuotaManager::EvictionContext::~EvictionContext() = default;
void QuotaManager::LazyInitialize() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(io_thread_->BelongsToCurrentThread());
if (database_) {
// Already initialized.
@@ -1256,6 +1285,7 @@
}
void QuotaManager::FinishLazyInitialize(bool is_database_bootstrapped) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
is_database_bootstrapped_ = is_database_bootstrapped;
StartEviction();
}
@@ -1264,6 +1294,7 @@
GetOriginCallback did_get_origin_callback,
int64_t usage,
int64_t unlimited_usage) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// The usage cache should be fully populated now so we can
// seed the database with origins we know about.
std::set<url::Origin>* origins = new std::set<url::Origin>;
@@ -1279,17 +1310,20 @@
void QuotaManager::DidBootstrapDatabase(
GetOriginCallback did_get_origin_callback,
bool success) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
is_database_bootstrapped_ = success;
DidDatabaseWork(success);
GetLRUOrigin(StorageType::kTemporary, std::move(did_get_origin_callback));
}
void QuotaManager::RegisterClient(QuotaClient* client) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(!database_.get());
clients_.push_back(client);
}
UsageTracker* QuotaManager::GetUsageTracker(StorageType type) const {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
switch (type) {
case StorageType::kTemporary:
return temporary_usage_tracker_.get();
@@ -1307,6 +1341,7 @@
void QuotaManager::GetCachedOrigins(StorageType type,
std::set<url::Origin>* origins) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(origins);
LazyInitialize();
DCHECK(GetUsageTracker(type));
@@ -1317,6 +1352,7 @@
const url::Origin& origin,
StorageType type,
base::Time accessed_time) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
if (type == StorageType::kTemporary && is_getting_eviction_origin_) {
// Record the accessed origins while GetLRUOrigin task is runing
@@ -1338,6 +1374,7 @@
StorageType type,
int64_t delta,
base::Time modified_time) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
DCHECK(GetUsageTracker(type));
GetUsageTracker(type)->UpdateUsageCache(client_id, origin, delta);
@@ -1351,6 +1388,7 @@
}
void QuotaManager::DumpQuotaTable(DumpQuotaTableCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DumpQuotaTableHelper* helper = new DumpQuotaTableHelper;
PostTaskAndReplyWithResultForDBThread(
FROM_HERE,
@@ -1362,6 +1400,7 @@
}
void QuotaManager::DumpOriginInfoTable(DumpOriginInfoTableCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DumpOriginInfoTableHelper* helper = new DumpOriginInfoTableHelper;
PostTaskAndReplyWithResultForDBThread(
FROM_HERE,
@@ -1373,6 +1412,7 @@
}
void QuotaManager::StartEviction() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(!temporary_storage_evictor_.get());
if (eviction_disabled_)
return;
@@ -1384,6 +1424,7 @@
void QuotaManager::DeleteOriginFromDatabase(const url::Origin& origin,
StorageType type,
bool is_eviction) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
if (db_disabled_)
return;
@@ -1396,6 +1437,7 @@
}
void QuotaManager::DidOriginDataEvicted(blink::mojom::QuotaStatusCode status) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(io_thread_->BelongsToCurrentThread());
// We only try evict origins that are not in use, so basically
@@ -1413,6 +1455,7 @@
int quota_client_mask,
bool is_eviction,
StatusCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
if (clients_.empty()) {
@@ -1426,6 +1469,7 @@
}
void QuotaManager::ReportHistogram() {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(!is_incognito_);
GetGlobalUsage(
StorageType::kTemporary,
@@ -1436,6 +1480,7 @@
void QuotaManager::DidGetTemporaryGlobalUsageForHistogram(
int64_t usage,
int64_t unlimited_usage) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
UMA_HISTOGRAM_MBYTES("Quota.GlobalUsageOfTemporaryStorage", usage);
std::set<url::Origin> origins;
@@ -1463,6 +1508,7 @@
void QuotaManager::DidGetPersistentGlobalUsageForHistogram(
int64_t usage,
int64_t unlimited_usage) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
UMA_HISTOGRAM_MBYTES("Quota.GlobalUsageOfPersistentStorage", usage);
std::set<url::Origin> origins;
@@ -1490,6 +1536,7 @@
void QuotaManager::DidDumpOriginInfoTableForHistogram(
const OriginInfoTableEntries& entries) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
using UsageMap = std::map<url::Origin, int64_t>;
UsageMap usage_map;
GetUsageTracker(StorageType::kTemporary)->GetCachedOriginsUsage(&usage_map);
@@ -1519,6 +1566,7 @@
std::set<url::Origin> QuotaManager::GetEvictionOriginExceptions(
const std::set<url::Origin>& extra_exceptions) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
std::set<url::Origin> exceptions = extra_exceptions;
for (const auto& p : origins_in_use_) {
if (p.second > 0)
@@ -1536,6 +1584,7 @@
void QuotaManager::DidGetEvictionOrigin(
GetOriginCallback callback,
const base::Optional<url::Origin>& origin) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Make sure the returned origin is (still) not in the origin_in_use_ set
// and has not been accessed since we posted the task.
DCHECK(!origin.has_value() || !origin->GetURL().is_empty());
@@ -1556,6 +1605,7 @@
const std::set<url::Origin>& extra_exceptions,
int64_t global_quota,
GetOriginCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
// This must not be called while there's an in-flight task.
DCHECK(!is_getting_eviction_origin_);
@@ -1580,6 +1630,7 @@
void QuotaManager::EvictOriginData(const url::Origin& origin,
StorageType type,
StatusCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(io_thread_->BelongsToCurrentThread());
DCHECK_EQ(type, StorageType::kTemporary);
@@ -1593,6 +1644,7 @@
}
void QuotaManager::GetEvictionRoundInfo(EvictionRoundInfoCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(io_thread_->BelongsToCurrentThread());
LazyInitialize();
EvictionRoundInfoHelper* helper =
@@ -1601,6 +1653,7 @@
}
void QuotaManager::GetLRUOrigin(StorageType type, GetOriginCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
LazyInitialize();
// This must not be called while there's an in-flight task.
DCHECK(lru_origin_callback_.is_null());
@@ -1625,6 +1678,7 @@
void QuotaManager::DidGetPersistentHostQuota(const std::string& host,
const int64_t* quota,
bool success) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DidDatabaseWork(success);
persistent_host_quota_callbacks_.Run(
host, blink::mojom::QuotaStatusCode::kOk,
@@ -1635,6 +1689,7 @@
QuotaCallback callback,
const int64_t* new_quota,
bool success) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DidDatabaseWork(success);
std::move(callback).Run(
success ? blink::mojom::QuotaStatusCode::kOk
@@ -1645,6 +1700,7 @@
void QuotaManager::DidGetLRUOrigin(
std::unique_ptr<base::Optional<url::Origin>> origin,
bool success) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DidDatabaseWork(success);
std::move(lru_origin_callback_).Run(*origin);
@@ -1660,6 +1716,7 @@
} // namespace
void QuotaManager::GetQuotaSettings(QuotaSettingsCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (base::TimeTicks::Now() - settings_timestamp_ <
settings_.refresh_interval) {
std::move(callback).Run(settings_);
@@ -1684,6 +1741,7 @@
void QuotaManager::DidGetSettings(base::TimeTicks start_ticks,
base::Optional<QuotaSettings> settings) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!settings) {
settings = settings_;
settings->refresh_interval = base::TimeDelta::FromMinutes(1);
@@ -1698,6 +1756,7 @@
}
void QuotaManager::GetStorageCapacity(StorageCapacityCallback callback) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (!storage_capacity_callbacks_.Add(std::move(callback)))
return;
if (is_incognito_) {
@@ -1716,6 +1775,7 @@
void QuotaManager::ContinueIncognitoGetStorageCapacity(
const QuotaSettings& settings) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
int64_t current_usage =
GetUsageTracker(StorageType::kTemporary)->GetCachedUsage();
current_usage += GetUsageTracker(StorageType::kPersistent)->GetCachedUsage();
@@ -1726,11 +1786,13 @@
void QuotaManager::DidGetStorageCapacity(
const std::tuple<int64_t, int64_t>& total_and_available) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
storage_capacity_callbacks_.Run(std::get<0>(total_and_available),
std::get<1>(total_and_available));
}
void QuotaManager::DidDatabaseWork(bool success) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
db_disabled_ = !success;
}
@@ -1746,6 +1808,7 @@
const base::Location& from_here,
base::OnceCallback<bool(QuotaDatabase*)> task,
base::OnceCallback<void(bool)> reply) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Deleting manager will post another task to DB sequence to delete
// |database_|, therefore we can be sure that database_ is alive when this
// task runs.
diff --git a/storage/browser/quota/quota_manager.h b/storage/browser/quota/quota_manager.h
index 8393f2f..188a436 100644
--- a/storage/browser/quota/quota_manager.h
+++ b/storage/browser/quota/quota_manager.h
@@ -23,6 +23,7 @@
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
+#include "base/sequence_checker.h"
#include "base/stl_util.h"
#include "base/timer/timer.h"
#include "storage/browser/quota/quota_callbacks.h"
@@ -178,6 +179,7 @@
void NotifyOriginInUse(const url::Origin& origin);
void NotifyOriginNoLongerInUse(const url::Origin& origin);
bool IsOriginInUse(const url::Origin& origin) const {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return base::ContainsKey(origins_in_use_, origin);
}
@@ -423,7 +425,8 @@
const bool is_incognito_;
const base::FilePath profile_path_;
- scoped_refptr<QuotaManagerProxy> proxy_;
+ // proxy_ can be accessed by any thread so it must be thread-safe
+ const scoped_refptr<QuotaManagerProxy> proxy_;
bool db_disabled_;
bool eviction_disabled_;
scoped_refptr<base::SingleThreadTaskRunner> io_thread_;
@@ -477,6 +480,8 @@
std::unique_ptr<StorageMonitor> storage_monitor_;
+ SEQUENCE_CHECKER(sequence_checker_);
+
base::WeakPtrFactory<QuotaManager> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(QuotaManager);
