Commit 998e3702 authored by Vasilii Sukhanov's avatar Vasilii Sukhanov Committed by Commit Bot

Handle correctly the situation when PasswordStore is destroyed on the UI thread.

PasswordStore uses SKIP_ON_SHUTDOWN for background sequence. That means that PasswordStore::DestroyOnBackgroundThread() can be skipped. In this case PasswordStore::* may be destroyed on the UI thread.
It's not a problem for PasswordSyncableService. We just need to remove the check in the destructor.
PasswordReuseDetector has this check implicitly via base::CancelableTaskTracker. Thus, we can just leak the object.

Bug: 741660
Change-Id: Ic49cb068733c159bca55e8d76eb58f70793f5fbe
Reviewed-on: https://chromium-review.googlesource.com/598008Reviewed-by: default avatarVadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491422}
parent 0494e1ef
...@@ -778,9 +778,9 @@ void PasswordStore::InitOnBackgroundThread( ...@@ -778,9 +778,9 @@ void PasswordStore::InitOnBackgroundThread(
syncable_service_->InjectStartSyncFlare(flare); syncable_service_->InjectStartSyncFlare(flare);
// TODO(crbug.com/706392): Fix password reuse detection for Android. // TODO(crbug.com/706392): Fix password reuse detection for Android.
#if !defined(OS_ANDROID) && !defined(OS_IOS) #if !defined(OS_ANDROID) && !defined(OS_IOS)
reuse_detector_ = base::MakeUnique<PasswordReuseDetector>(); reuse_detector_ = new PasswordReuseDetector;
GetAutofillableLoginsImpl( GetAutofillableLoginsImpl(
base::MakeUnique<GetLoginsRequest>(reuse_detector_.get())); base::MakeUnique<GetLoginsRequest>(reuse_detector_));
#endif #endif
} }
...@@ -789,7 +789,8 @@ void PasswordStore::DestroyOnBackgroundThread() { ...@@ -789,7 +789,8 @@ void PasswordStore::DestroyOnBackgroundThread() {
syncable_service_.reset(); syncable_service_.reset();
// TODO(crbug.com/706392): Fix password reuse detection for Android. // TODO(crbug.com/706392): Fix password reuse detection for Android.
#if !defined(OS_ANDROID) && !defined(OS_IOS) #if !defined(OS_ANDROID) && !defined(OS_IOS)
reuse_detector_.reset(); delete reuse_detector_;
reuse_detector_ = nullptr;
#endif #endif
} }
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "base/observer_list_threadsafe.h" #include "base/observer_list_threadsafe.h"
#include "base/sequenced_task_runner.h" #include "base/sequenced_task_runner.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "build/build_config.h"
#include "components/keyed_service/core/refcounted_keyed_service.h" #include "components/keyed_service/core/refcounted_keyed_service.h"
#include "components/password_manager/core/browser/password_reuse_defines.h" #include "components/password_manager/core/browser/password_reuse_defines.h"
#include "components/password_manager/core/browser/password_store_change.h" #include "components/password_manager/core/browser/password_store_change.h"
...@@ -583,7 +584,8 @@ class PasswordStore : protected PasswordStoreSync, ...@@ -583,7 +584,8 @@ class PasswordStore : protected PasswordStoreSync,
void InitOnBackgroundThread( void InitOnBackgroundThread(
const syncer::SyncableService::StartSyncFlare& flare); const syncer::SyncableService::StartSyncFlare& flare);
// Deletes objest that should be destroyed on the background thread. // Deletes object that should be destroyed on the background thread.
// WARNING: this method can be skipped on shutdown.
void DestroyOnBackgroundThread(); void DestroyOnBackgroundThread();
// The observers. // The observers.
...@@ -593,7 +595,10 @@ class PasswordStore : protected PasswordStoreSync, ...@@ -593,7 +595,10 @@ class PasswordStore : protected PasswordStoreSync,
std::unique_ptr<AffiliatedMatchHelper> affiliated_match_helper_; std::unique_ptr<AffiliatedMatchHelper> affiliated_match_helper_;
// TODO(crbug.com/706392): Fix password reuse detection for Android. // TODO(crbug.com/706392): Fix password reuse detection for Android.
#if !defined(OS_ANDROID) && !defined(OS_IOS) #if !defined(OS_ANDROID) && !defined(OS_IOS)
std::unique_ptr<PasswordReuseDetector> reuse_detector_; // PasswordReuseDetector can be only destroyed on the background thread. It
// can't be owned by PasswordStore because PasswordStore can be destroyed on
// UI thread and DestroyOnBackgroundThread isn't guaranteed to be called.
PasswordReuseDetector* reuse_detector_ = nullptr;
#endif #endif
#if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED) #if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)
std::unique_ptr<PasswordStoreSigninNotifier> notifier_; std::unique_ptr<PasswordStoreSigninNotifier> notifier_;
......
...@@ -340,9 +340,7 @@ PasswordSyncableService::PasswordSyncableService( ...@@ -340,9 +340,7 @@ PasswordSyncableService::PasswordSyncableService(
clock_(new base::DefaultClock), clock_(new base::DefaultClock),
is_processing_sync_changes_(false) {} is_processing_sync_changes_(false) {}
PasswordSyncableService::~PasswordSyncableService() { PasswordSyncableService::~PasswordSyncableService() = default;
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
}
syncer::SyncMergeResult PasswordSyncableService::MergeDataAndStartSyncing( syncer::SyncMergeResult PasswordSyncableService::MergeDataAndStartSyncing(
syncer::ModelType type, syncer::ModelType type,
......
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