0

Remove explicit use of exclusive mode on database option

The database option `exclusive_locking` is enabled by default.

NOTE: This CL has no functional changes.

It was used to be recommended to explicitly specify it to avoid using
the default DatabaseOptions constructor which had `exclusive_locking`
to false (due historical reason related to unittests migration).

The following CL [landed] fixes the default constructor and by
default all database as using the `exclusive_locking` mode.
see: https://chromium-review.googlesource.com/c/chromium/src/+/5029223

The recommendation from now is to explicitly specify the non
exclusive case with an explanation of why the database can be
accessed in that mode.

Bug: 1120969
Change-Id: Ib278d967c76c1ea8494ebaa4bb1723f0fb74c30e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5251553
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1256381}
This commit is contained in:
Etienne Bergeron
2024-02-05 20:29:56 +00:00
committed by Chromium LUCI CQ
parent ad49690d02
commit bffef51ac1
10 changed files with 6 additions and 20 deletions
chrome/browser
extensions
predictors
components
blocklist
opt_out_blocklist
offline_pages
content/browser
services/network/trust_tokens
storage/browser/database

@ -40,7 +40,6 @@ static const int kSizeThresholdForFlush = 200;
ActivityDatabase::ActivityDatabase(ActivityDatabase::Delegate* delegate)
: delegate_(delegate),
db_({
.exclusive_locking = true,
.page_size = 4096,
.cache_size = 32,
// TODO(pwnall): Add a meta table and remove this option.

@ -100,8 +100,6 @@ PredictorDatabaseInternal::~PredictorDatabaseInternal() {
void PredictorDatabaseInternal::Initialize() {
DCHECK(db_task_runner_->RunsTasksInCurrentSequence());
// TODO(tburkard): figure out if we need this.
// db_->set_exclusive_locking();
if (autocomplete_table_->IsCancelled() ||
resource_prefetch_tables_->IsCancelled()) {
return;

@ -390,7 +390,6 @@ void OptOutStoreSQL::LoadBlockList(
DCHECK(io_task_runner_->BelongsToCurrentThread());
if (!db_) {
db_ = std::make_unique<sql::Database>(sql::DatabaseOptions{
.exclusive_locking = true,
// The entry size should be between 11 and 10 + x bytes, where x is the
// the length of the host name string in bytes.
// The total number of entries per host is bounded at 32, and the total

@ -587,8 +587,8 @@ RequestQueueStore::~RequestQueueStore() {
void RequestQueueStore::Initialize(InitializeCallback callback) {
DCHECK(!db_);
db_ = std::make_unique<sql::Database>(sql::DatabaseOptions{
.exclusive_locking = true, .page_size = 4096, .cache_size = 500});
db_ = std::make_unique<sql::Database>(
sql::DatabaseOptions{.page_size = 4096, .cache_size = 500});
background_task_runner_->PostTaskAndReplyWithResult(
FROM_HERE, base::BindOnce(&InitDatabaseSync, db_.get(), db_file_path_),

@ -100,7 +100,6 @@ void SqlStoreBase::Initialize(base::OnceClosure pending_command) {
// This is how we reset a pointer and provide deleter. This is necessary to
// ensure that we can close the store more than once.
db_ = DatabaseUniquePtr(new sql::Database({// These values are default.
.exclusive_locking = true,
.page_size = 4096,
.cache_size = 500}),
base::OnTaskRunnerDeleter(background_task_runner_));

@ -467,9 +467,7 @@ AttributionStorageSql::AttributionStorageSql(
: path_to_database_(user_data_directory.empty()
? base::FilePath()
: DatabasePath(user_data_directory)),
db_(sql::DatabaseOptions{.exclusive_locking = true,
.page_size = 4096,
.cache_size = 32}),
db_(sql::DatabaseOptions{.page_size = 4096, .cache_size = 32}),
delegate_(std::move(delegate)),
rate_limit_table_(delegate_.get()) {
DCHECK(delegate_);

@ -244,8 +244,8 @@ bool BrowsingTopicsSiteDataStorage::LazyInit() {
if (db_init_status_ != InitStatus::kUnattempted)
return db_init_status_ == InitStatus::kSuccess;
db_ = std::make_unique<sql::Database>(sql::DatabaseOptions{
.exclusive_locking = true, .page_size = 4096, .cache_size = 32});
db_ = std::make_unique<sql::Database>(
sql::DatabaseOptions{.page_size = 4096, .cache_size = 32});
db_->set_histogram_tag("BrowsingTopics");
// base::Unretained is safe here because this BrowsingTopicsSiteDataStorage

@ -82,9 +82,7 @@ ClientTraceReport::ClientTraceReport() = default;
ClientTraceReport::~ClientTraceReport() = default;
TraceReportDatabase::TraceReportDatabase()
: database_(sql::DatabaseOptions{.exclusive_locking = true,
.page_size = 4096,
.cache_size = 128}) {
: database_(sql::DatabaseOptions{.page_size = 4096, .cache_size = 128}) {
DETACH_FROM_SEQUENCE(sequence_checker_);
}

@ -90,10 +90,6 @@ NOINLINE TrustTokenDatabaseOwner::TrustTokenDatabaseOwner(
db_task_runner)),
db_task_runner_(db_task_runner),
backing_database_(std::make_unique<sql::Database>(sql::DatabaseOptions{
// As they work on deleting the feature (crbug.com/1120969), sql/
// owners prefer to see which clients are explicitly okay with using
// exclusive locking (the default).
.exclusive_locking = true,
.page_size = 4096,
.cache_size = 500,
// TODO(pwnall): Add a meta table and remove this option.

@ -112,7 +112,6 @@ DatabaseTracker::DatabaseTracker(
? profile_path_.Append(kIncognitoDatabaseDirectoryName)
: profile_path_.Append(kDatabaseDirectoryName)),
db_(std::make_unique<sql::Database>(sql::DatabaseOptions{
.exclusive_locking = true,
.page_size = 4096,
.cache_size = 500,
})),