Skip to content

GitLab

T
tangled
Commit a1ba1638 authored by Marc Treib's avatar Marc Treib Committed by Commit Bot
Browse files
Options

PasswordStore: Add lots of DCHECKs that we're on the expected task runner 

This makes it clear to the casual reader (like me) which things run on
the main thread and which on the background thread.

Bug: none
Change-Id: Ica9eb3f0fc26e0080d5e7c07750e89f51b2192b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1627032
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Auto-Submit: Marc Treib <treib@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662589}
parent 32680a60
Hide whitespace changes
Inline Side-by-side
Showing with 29 additions and 2 deletions
components/password_manager/core/browser/password_store.cc
View file @ a1ba1638
...@@ -147,21 +147,25 @@ void PasswordStore::SetAffiliatedMatchHelper( ...@@ -147,21 +147,25 @@ void PasswordStore::SetAffiliatedMatchHelper(
} }
void PasswordStore::AddLogin(const PasswordForm& form) { void PasswordStore::AddLogin(const PasswordForm& form) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
ScheduleTask(base::BindOnce(&PasswordStore::AddLoginInternal, this, form)); ScheduleTask(base::BindOnce(&PasswordStore::AddLoginInternal, this, form));
} }
void PasswordStore::UpdateLogin(const PasswordForm& form) { void PasswordStore::UpdateLogin(const PasswordForm& form) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
ScheduleTask(base::BindOnce(&PasswordStore::UpdateLoginInternal, this, form)); ScheduleTask(base::BindOnce(&PasswordStore::UpdateLoginInternal, this, form));
} }
void PasswordStore::UpdateLoginWithPrimaryKey( void PasswordStore::UpdateLoginWithPrimaryKey(
const autofill::PasswordForm& new_form, const autofill::PasswordForm& new_form,
const autofill::PasswordForm& old_primary_key) { const autofill::PasswordForm& old_primary_key) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
ScheduleTask(base::BindOnce(&PasswordStore::UpdateLoginWithPrimaryKeyInternal, ScheduleTask(base::BindOnce(&PasswordStore::UpdateLoginWithPrimaryKeyInternal,
this, new_form, old_primary_key)); this, new_form, old_primary_key));
} }
void PasswordStore::RemoveLogin(const PasswordForm& form) { void PasswordStore::RemoveLogin(const PasswordForm& form) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
ScheduleTask(base::BindOnce(&PasswordStore::RemoveLoginInternal, this, form)); ScheduleTask(base::BindOnce(&PasswordStore::RemoveLoginInternal, this, form));
} }
...@@ -170,6 +174,7 @@ void PasswordStore::RemoveLoginsByURLAndTime( ...@@ -170,6 +174,7 @@ void PasswordStore::RemoveLoginsByURLAndTime(
base::Time delete_begin, base::Time delete_begin,
base::Time delete_end, base::Time delete_end,
const base::Closure& completion) { const base::Closure& completion) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
ScheduleTask(base::BindOnce(&PasswordStore::RemoveLoginsByURLAndTimeInternal, ScheduleTask(base::BindOnce(&PasswordStore::RemoveLoginsByURLAndTimeInternal,
this, url_filter, delete_begin, delete_end, this, url_filter, delete_begin, delete_end,
completion)); completion));
...@@ -179,6 +184,7 @@ void PasswordStore::RemoveLoginsCreatedBetween( ...@@ -179,6 +184,7 @@ void PasswordStore::RemoveLoginsCreatedBetween(
base::Time delete_begin, base::Time delete_begin,
base::Time delete_end, base::Time delete_end,
const base::Closure& completion) { const base::Closure& completion) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
ScheduleTask( ScheduleTask(
base::BindOnce(&PasswordStore::RemoveLoginsCreatedBetweenInternal, this, base::BindOnce(&PasswordStore::RemoveLoginsCreatedBetweenInternal, this,
delete_begin, delete_end, completion)); delete_begin, delete_end, completion));
...@@ -186,6 +192,7 @@ void PasswordStore::RemoveLoginsCreatedBetween( ...@@ -186,6 +192,7 @@ void PasswordStore::RemoveLoginsCreatedBetween(
void PasswordStore::RemoveLoginsSyncedBetween(base::Time delete_begin, void PasswordStore::RemoveLoginsSyncedBetween(base::Time delete_begin,
base::Time delete_end) { base::Time delete_end) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
ScheduleTask(base::BindOnce(&PasswordStore::RemoveLoginsSyncedBetweenInternal, ScheduleTask(base::BindOnce(&PasswordStore::RemoveLoginsSyncedBetweenInternal,
this, delete_begin, delete_end)); this, delete_begin, delete_end));
} }
...@@ -195,6 +202,7 @@ void PasswordStore::RemoveStatisticsByOriginAndTime( ...@@ -195,6 +202,7 @@ void PasswordStore::RemoveStatisticsByOriginAndTime(
base::Time delete_begin, base::Time delete_begin,
base::Time delete_end, base::Time delete_end,
const base::Closure& completion) { const base::Closure& completion) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
ScheduleTask(base::BindOnce( ScheduleTask(base::BindOnce(
&PasswordStore::RemoveStatisticsByOriginAndTimeInternal, this, &PasswordStore::RemoveStatisticsByOriginAndTimeInternal, this,
origin_filter, delete_begin, delete_end, completion)); origin_filter, delete_begin, delete_end, completion));
...@@ -203,6 +211,7 @@ void PasswordStore::RemoveStatisticsByOriginAndTime( ...@@ -203,6 +211,7 @@ void PasswordStore::RemoveStatisticsByOriginAndTime(
void PasswordStore::DisableAutoSignInForOrigins( void PasswordStore::DisableAutoSignInForOrigins(
const base::Callback<bool(const GURL&)>& origin_filter, const base::Callback<bool(const GURL&)>& origin_filter,
const base::Closure& completion) { const base::Closure& completion) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
ScheduleTask(base::BindOnce( ScheduleTask(base::BindOnce(
&PasswordStore::DisableAutoSignInForOriginsInternal, this, &PasswordStore::DisableAutoSignInForOriginsInternal, this,
base::RepeatingCallback<bool(const GURL&)>(origin_filter), completion)); base::RepeatingCallback<bool(const GURL&)>(origin_filter), completion));
...@@ -210,6 +219,7 @@ void PasswordStore::DisableAutoSignInForOrigins( ...@@ -210,6 +219,7 @@ void PasswordStore::DisableAutoSignInForOrigins(
void PasswordStore::GetLogins(const FormDigest& form, void PasswordStore::GetLogins(const FormDigest& form,
PasswordStoreConsumer* consumer) { PasswordStoreConsumer* consumer) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
// Per http://crbug.com/121738, we deliberately ignore saved logins for // Per http://crbug.com/121738, we deliberately ignore saved logins for
// http*://www.google.com/ that were stored prior to 2012. (Google now uses // http*://www.google.com/ that were stored prior to 2012. (Google now uses
// https://accounts.google.com/ for all login forms, so these should be // https://accounts.google.com/ for all login forms, so these should be
...@@ -247,23 +257,27 @@ void PasswordStore::GetLogins(const FormDigest& form, ...@@ -247,23 +257,27 @@ void PasswordStore::GetLogins(const FormDigest& form,
} }
void PasswordStore::GetAutofillableLogins(PasswordStoreConsumer* consumer) { void PasswordStore::GetAutofillableLogins(PasswordStoreConsumer* consumer) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
PostLoginsTaskAndReplyToConsumerWithResult( PostLoginsTaskAndReplyToConsumerWithResult(
consumer, consumer,
base::BindOnce(&PasswordStore::GetAutofillableLoginsImpl, this)); base::BindOnce(&PasswordStore::GetAutofillableLoginsImpl, this));
} }
void PasswordStore::GetBlacklistLogins(PasswordStoreConsumer* consumer) { void PasswordStore::GetBlacklistLogins(PasswordStoreConsumer* consumer) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
PostLoginsTaskAndReplyToConsumerWithResult( PostLoginsTaskAndReplyToConsumerWithResult(
consumer, base::BindOnce(&PasswordStore::GetBlacklistLoginsImpl, this)); consumer, base::BindOnce(&PasswordStore::GetBlacklistLoginsImpl, this));
} }
void PasswordStore::GetAllLogins(PasswordStoreConsumer* consumer) { void PasswordStore::GetAllLogins(PasswordStoreConsumer* consumer) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
PostLoginsTaskAndReplyToConsumerWithResult( PostLoginsTaskAndReplyToConsumerWithResult(
consumer, base::BindOnce(&PasswordStore::GetAllLoginsImpl, this)); consumer, base::BindOnce(&PasswordStore::GetAllLoginsImpl, this));
} }
void PasswordStore::GetAllLoginsWithAffiliationAndBrandingInformation( void PasswordStore::GetAllLoginsWithAffiliationAndBrandingInformation(
PasswordStoreConsumer* consumer) { PasswordStoreConsumer* consumer) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
PostLoginsTaskAndReplyToConsumerWithProcessedResult( PostLoginsTaskAndReplyToConsumerWithProcessedResult(
consumer, base::BindOnce(&PasswordStore::GetAllLoginsImpl, this), consumer, base::BindOnce(&PasswordStore::GetAllLoginsImpl, this),
base::BindOnce(&PasswordStore::InjectAffiliationAndBrandingInformation, base::BindOnce(&PasswordStore::InjectAffiliationAndBrandingInformation,
...@@ -273,6 +287,7 @@ void PasswordStore::GetAllLoginsWithAffiliationAndBrandingInformation( ...@@ -273,6 +287,7 @@ void PasswordStore::GetAllLoginsWithAffiliationAndBrandingInformation(
void PasswordStore::ReportMetrics(const std::string& sync_username, void PasswordStore::ReportMetrics(const std::string& sync_username,
bool custom_passphrase_sync_enabled, bool custom_passphrase_sync_enabled,
bool is_under_advanced_protection) { bool is_under_advanced_protection) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
if (background_task_runner_) { if (background_task_runner_) {
base::Closure task = base::Closure task =
base::Bind(&PasswordStore::ReportMetricsImpl, this, sync_username, base::Bind(&PasswordStore::ReportMetricsImpl, this, sync_username,
...@@ -295,21 +310,25 @@ void PasswordStore::ReportMetrics(const std::string& sync_username, ...@@ -295,21 +310,25 @@ void PasswordStore::ReportMetrics(const std::string& sync_username,
} }
void PasswordStore::AddSiteStats(const InteractionsStats& stats) { void PasswordStore::AddSiteStats(const InteractionsStats& stats) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
ScheduleTask(base::BindOnce(&PasswordStore::AddSiteStatsImpl, this, stats)); ScheduleTask(base::BindOnce(&PasswordStore::AddSiteStatsImpl, this, stats));
} }
void PasswordStore::RemoveSiteStats(const GURL& origin_domain) { void PasswordStore::RemoveSiteStats(const GURL& origin_domain) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
ScheduleTask( ScheduleTask(
base::BindOnce(&PasswordStore::RemoveSiteStatsImpl, this, origin_domain)); base::BindOnce(&PasswordStore::RemoveSiteStatsImpl, this, origin_domain));
} }
void PasswordStore::GetAllSiteStats(PasswordStoreConsumer* consumer) { void PasswordStore::GetAllSiteStats(PasswordStoreConsumer* consumer) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
PostStatsTaskAndReplyToConsumerWithResult( PostStatsTaskAndReplyToConsumerWithResult(
consumer, base::BindOnce(&PasswordStore::GetAllSiteStatsImpl, this)); consumer, base::BindOnce(&PasswordStore::GetAllSiteStatsImpl, this));
} }
void PasswordStore::GetSiteStats(const GURL& origin_domain, void PasswordStore::GetSiteStats(const GURL& origin_domain,
PasswordStoreConsumer* consumer) { PasswordStoreConsumer* consumer) {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
PostStatsTaskAndReplyToConsumerWithResult( PostStatsTaskAndReplyToConsumerWithResult(
consumer, consumer,
base::BindOnce(&PasswordStore::GetSiteStatsImpl, this, origin_domain)); base::BindOnce(&PasswordStore::GetSiteStatsImpl, this, origin_domain));
...@@ -335,6 +354,7 @@ PasswordStore::GetBackgroundTaskRunner() { ...@@ -335,6 +354,7 @@ PasswordStore::GetBackgroundTaskRunner() {
} }
bool PasswordStore::IsAbleToSavePasswords() const { bool PasswordStore::IsAbleToSavePasswords() const {
DCHECK(main_task_runner_->RunsTasksInCurrentSequence());
return init_status_ == InitStatus::kSuccess; return init_status_ == InitStatus::kSuccess;
} }
...@@ -668,6 +688,7 @@ void PasswordStore::PostStatsTaskAndReplyToConsumerWithResult( ...@@ -668,6 +688,7 @@ void PasswordStore::PostStatsTaskAndReplyToConsumerWithResult(
} }
void PasswordStore::AddLoginInternal(const PasswordForm& form) { void PasswordStore::AddLoginInternal(const PasswordForm& form) {
DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
SCOPED_UMA_HISTOGRAM_TIMER("PasswordManager.StorePerformance.AddLogin"); SCOPED_UMA_HISTOGRAM_TIMER("PasswordManager.StorePerformance.AddLogin");
BeginTransaction(); BeginTransaction();
PasswordStoreChangeList changes = AddLoginImpl(form); PasswordStoreChangeList changes = AddLoginImpl(form);
...@@ -680,6 +701,7 @@ void PasswordStore::AddLoginInternal(const PasswordForm& form) { ...@@ -680,6 +701,7 @@ void PasswordStore::AddLoginInternal(const PasswordForm& form) {
} }
void PasswordStore::UpdateLoginInternal(const PasswordForm& form) { void PasswordStore::UpdateLoginInternal(const PasswordForm& form) {
DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
SCOPED_UMA_HISTOGRAM_TIMER("PasswordManager.StorePerformance.UpdateLogin"); SCOPED_UMA_HISTOGRAM_TIMER("PasswordManager.StorePerformance.UpdateLogin");
BeginTransaction(); BeginTransaction();
PasswordStoreChangeList changes = UpdateLoginImpl(form); PasswordStoreChangeList changes = UpdateLoginImpl(form);
...@@ -692,6 +714,7 @@ void PasswordStore::UpdateLoginInternal(const PasswordForm& form) { ...@@ -692,6 +714,7 @@ void PasswordStore::UpdateLoginInternal(const PasswordForm& form) {
} }
void PasswordStore::RemoveLoginInternal(const PasswordForm& form) { void PasswordStore::RemoveLoginInternal(const PasswordForm& form) {
DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
SCOPED_UMA_HISTOGRAM_TIMER("PasswordManager.StorePerformance.RemoveLogin"); SCOPED_UMA_HISTOGRAM_TIMER("PasswordManager.StorePerformance.RemoveLogin");
BeginTransaction(); BeginTransaction();
PasswordStoreChangeList changes = RemoveLoginImpl(form); PasswordStoreChangeList changes = RemoveLoginImpl(form);
...@@ -706,6 +729,7 @@ void PasswordStore::RemoveLoginInternal(const PasswordForm& form) { ...@@ -706,6 +729,7 @@ void PasswordStore::RemoveLoginInternal(const PasswordForm& form) {
void PasswordStore::UpdateLoginWithPrimaryKeyInternal( void PasswordStore::UpdateLoginWithPrimaryKeyInternal(
const PasswordForm& new_form, const PasswordForm& new_form,
const PasswordForm& old_primary_key) { const PasswordForm& old_primary_key) {
DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
BeginTransaction(); BeginTransaction();
PasswordStoreChangeList all_changes = RemoveLoginImpl(old_primary_key); PasswordStoreChangeList all_changes = RemoveLoginImpl(old_primary_key);
PasswordStoreChangeList changes = AddLoginImpl(new_form); PasswordStoreChangeList changes = AddLoginImpl(new_form);
...@@ -723,6 +747,7 @@ void PasswordStore::RemoveLoginsByURLAndTimeInternal( ...@@ -723,6 +747,7 @@ void PasswordStore::RemoveLoginsByURLAndTimeInternal(
base::Time delete_begin, base::Time delete_begin,
base::Time delete_end, base::Time delete_end,
const base::Closure& completion) { const base::Closure& completion) {
DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
BeginTransaction(); BeginTransaction();
PasswordStoreChangeList changes = PasswordStoreChangeList changes =
RemoveLoginsByURLAndTimeImpl(url_filter, delete_begin, delete_end); RemoveLoginsByURLAndTimeImpl(url_filter, delete_begin, delete_end);
...@@ -740,6 +765,7 @@ void PasswordStore::RemoveLoginsCreatedBetweenInternal( ...@@ -740,6 +765,7 @@ void PasswordStore::RemoveLoginsCreatedBetweenInternal(
base::Time delete_begin, base::Time delete_begin,
base::Time delete_end, base::Time delete_end,
const base::Closure& completion) { const base::Closure& completion) {
DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
BeginTransaction(); BeginTransaction();
PasswordStoreChangeList changes = PasswordStoreChangeList changes =
RemoveLoginsCreatedBetweenImpl(delete_begin, delete_end); RemoveLoginsCreatedBetweenImpl(delete_begin, delete_end);
...@@ -755,6 +781,7 @@ void PasswordStore::RemoveLoginsCreatedBetweenInternal( ...@@ -755,6 +781,7 @@ void PasswordStore::RemoveLoginsCreatedBetweenInternal(
void PasswordStore::RemoveLoginsSyncedBetweenInternal(base::Time delete_begin, void PasswordStore::RemoveLoginsSyncedBetweenInternal(base::Time delete_begin,
base::Time delete_end) { base::Time delete_end) {
DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
BeginTransaction(); BeginTransaction();
PasswordStoreChangeList changes = PasswordStoreChangeList changes =
RemoveLoginsSyncedBetweenImpl(delete_begin, delete_end); RemoveLoginsSyncedBetweenImpl(delete_begin, delete_end);
...@@ -771,6 +798,7 @@ void PasswordStore::RemoveStatisticsByOriginAndTimeInternal( ...@@ -771,6 +798,7 @@ void PasswordStore::RemoveStatisticsByOriginAndTimeInternal(
base::Time delete_begin, base::Time delete_begin,
base::Time delete_end, base::Time delete_end,
const base::Closure& completion) { const base::Closure& completion) {
DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
RemoveStatisticsByOriginAndTimeImpl(origin_filter, delete_begin, delete_end); RemoveStatisticsByOriginAndTimeImpl(origin_filter, delete_begin, delete_end);
if (!completion.is_null()) if (!completion.is_null())
main_task_runner_->PostTask(FROM_HERE, completion); main_task_runner_->PostTask(FROM_HERE, completion);
...@@ -779,6 +807,7 @@ void PasswordStore::RemoveStatisticsByOriginAndTimeInternal( ...@@ -779,6 +807,7 @@ void PasswordStore::RemoveStatisticsByOriginAndTimeInternal(
void PasswordStore::DisableAutoSignInForOriginsInternal( void PasswordStore::DisableAutoSignInForOriginsInternal(
const base::Callback<bool(const GURL&)>& origin_filter, const base::Callback<bool(const GURL&)>& origin_filter,
const base::Closure& completion) { const base::Closure& completion) {
DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
DisableAutoSignInForOriginsImpl(origin_filter); DisableAutoSignInForOriginsImpl(origin_filter);
if (!completion.is_null()) if (!completion.is_null())
main_task_runner_->PostTask(FROM_HERE, completion); main_task_runner_->PostTask(FROM_HERE, completion);
......
components/password_manager/core/browser/password_store.h
View file @ a1ba1638
...@@ -304,8 +304,6 @@ class PasswordStore : protected PasswordStoreSync, ...@@ -304,8 +304,6 @@ class PasswordStore : protected PasswordStoreSync,
protected: protected:
friend class base::RefCountedThreadSafe<PasswordStore>; friend class base::RefCountedThreadSafe<PasswordStore>;
typedef base::Callback<PasswordStoreChangeList(void)> ModificationTask;
// TODO(crbug.com/706392): Fix password reuse detection for Android. // TODO(crbug.com/706392): Fix password reuse detection for Android.
#if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED) #if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)
// Represents a single CheckReuse() request. Implements functionality to // Represents a single CheckReuse() request. Implements functionality to
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment