Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(47)

Unified Diff: chrome/browser/password_manager/password_store_proxy_mac.cc

Issue 1213043003: Start the migration of passwords from the Keychain. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: move the enum Created 5 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/password_manager/password_store_proxy_mac.cc
diff --git a/chrome/browser/password_manager/password_store_proxy_mac.cc b/chrome/browser/password_manager/password_store_proxy_mac.cc
index 2d16b7ecfef43b1104d789ce34c678d0ad4f26a1..bc400e7a43eba77b5b1f72916ee31bd663b785d8 100644
--- a/chrome/browser/password_manager/password_store_proxy_mac.cc
+++ b/chrome/browser/password_manager/password_store_proxy_mac.cc
@@ -4,23 +4,34 @@
#include "chrome/browser/password_manager/password_store_proxy_mac.h"
+#include "base/metrics/histogram_macros.h"
#include "chrome/browser/password_manager/password_store_mac.h"
#include "chrome/browser/password_manager/simple_password_store_mac.h"
#include "content/public/browser/browser_thread.h"
#include "crypto/apple_keychain.h"
+using password_manager::MigrationStatus;
using password_manager::PasswordStoreChangeList;
PasswordStoreProxyMac::PasswordStoreProxyMac(
scoped_refptr<base::SingleThreadTaskRunner> main_thread_runner,
scoped_ptr<crypto::AppleKeychain> keychain,
- scoped_ptr<password_manager::LoginDatabase> login_db)
+ scoped_ptr<password_manager::LoginDatabase> login_db,
+ PrefService* prefs)
: PasswordStore(main_thread_runner, nullptr),
login_metadata_db_(login_db.Pass()) {
DCHECK(login_metadata_db_);
- // TODO(vasilii): for now the class is just a wrapper around PasswordStoreMac.
- password_store_mac_ =
- new PasswordStoreMac(main_thread_runner, nullptr, keychain.Pass());
+ migration_status_.Init(password_manager::prefs::kKeychainMigrationStatus,
+ prefs);
+ if (migration_status_.GetValue() ==
+ static_cast<int>(MigrationStatus::MIGRATED)) {
+ // The login database will be set later after initialization.
+ password_store_simple_ =
+ new SimplePasswordStoreMac(main_thread_runner, nullptr, nullptr);
+ } else {
+ password_store_mac_ =
+ new PasswordStoreMac(main_thread_runner, nullptr, keychain.Pass());
+ }
}
PasswordStoreProxyMac::~PasswordStoreProxyMac() {
@@ -37,17 +48,28 @@ bool PasswordStoreProxyMac::Init(
return false;
}
- ScheduleTask(
- base::Bind(&PasswordStoreProxyMac::InitOnBackgroundThread, this));
- password_store_mac_->InitWithTaskRunner(GetBackgroundTaskRunner());
- return password_manager::PasswordStore::Init(flare);
+ if (!password_manager::PasswordStore::Init(flare))
+ return false;
+
+ return ScheduleTask(
+ base::Bind(&PasswordStoreProxyMac::InitOnBackgroundThread, this,
+ static_cast<MigrationStatus>(migration_status_.GetValue())));
}
void PasswordStoreProxyMac::Shutdown() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
PasswordStore::Shutdown();
- GetBackend()->Shutdown();
thread_->Stop();
+
+ // Execute the task which are still pending.
+ FlushPendingTasks();
+
+ // Unsubscribe the observer, otherwise it's too late in the destructor.
+ migration_status_.Destroy();
+
+ // After the thread has stopped it's impossible to switch from one backend to
+ // another. GetBackend() returns the correct result.
+ GetBackend()->Shutdown();
}
scoped_refptr<base::SingleThreadTaskRunner>
@@ -61,15 +83,62 @@ password_manager::PasswordStore* PasswordStoreProxyMac::GetBackend() const {
return password_store_simple_.get();
}
-void PasswordStoreProxyMac::InitOnBackgroundThread() {
+void PasswordStoreProxyMac::InitOnBackgroundThread(MigrationStatus status) {
DCHECK(GetBackgroundTaskRunner()->BelongsToCurrentThread());
if (!login_metadata_db_->Init()) {
login_metadata_db_.reset();
LOG(ERROR) << "Could not create/open login database.";
- return;
}
- if (password_store_mac_)
+
+ if (status == MigrationStatus::MIGRATED) {
+ password_store_simple_->InitWithTaskRunner(GetBackgroundTaskRunner(),
+ login_metadata_db_.Pass());
+ } else {
password_store_mac_->set_login_metadata_db(login_metadata_db_.get());
+ password_store_mac_->InitWithTaskRunner(GetBackgroundTaskRunner());
+ if (login_metadata_db_ && (status == MigrationStatus::NOT_STARTED ||
+ status == MigrationStatus::FAILED_ONCE)) {
+ // Let's try to migrate the passwords.
+ if (password_store_mac_->ImportFromKeychain() ==
+ PasswordStoreMac::MIGRATION_OK) {
+ status = MigrationStatus::MIGRATED;
+ // Switch from |password_store_mac_| to |password_store_simple_|.
+ password_store_mac_->set_login_metadata_db(nullptr);
+ pending_ui_tasks_.push_back(
+ base::Bind(&PasswordStoreMac::Shutdown, password_store_mac_));
+ password_store_mac_ = nullptr;
+ password_store_simple_ = new SimplePasswordStoreMac(
stuartmorgan 2015/07/07 17:40:25 Just to be safe (since the logic that creates this
vasilii 2015/07/08 09:56:58 Done.
+ main_thread_runner_, GetBackgroundTaskRunner(),
+ login_metadata_db_.Pass());
+ } else {
+ status = (status == MigrationStatus::FAILED_ONCE
+ ? MigrationStatus::FAILED_TWICE
+ : MigrationStatus::FAILED_ONCE);
+ }
+ pending_ui_tasks_.push_back(
+ base::Bind(&PasswordStoreProxyMac::UpdateStatusPref, this, status));
stuartmorgan 2015/07/07 17:40:25 Given that there's a race where the login DB could
vasilii 2015/07/08 09:56:58 There is no race. The preference is written in Pas
stuartmorgan 2015/07/08 14:18:40 I meant in case of a crash; there's a window of ti
vasilii 2015/07/08 15:45:54 I think it's rare and on the next run the password
+ }
+ }
+ if (!pending_ui_tasks_.empty()) {
+ main_thread_runner_->PostTask(
+ FROM_HERE, base::Bind(&PasswordStoreProxyMac::FlushPendingTasks, this));
+ }
+ UMA_HISTOGRAM_ENUMERATION(
+ "PasswordManager.KeychainMigration.Status", static_cast<int>(status),
+ static_cast<int>(MigrationStatus::MIGRATION_STATUS_COUNT));
+ DCHECK(GetBackend());
+}
+
+void PasswordStoreProxyMac::UpdateStatusPref(MigrationStatus status) {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+ migration_status_.SetValue(static_cast<int>(status));
+}
+
+void PasswordStoreProxyMac::FlushPendingTasks() {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+ for (auto& task : pending_ui_tasks_)
+ task.Run();
+ pending_ui_tasks_.clear();
}
void PasswordStoreProxyMac::ReportMetricsImpl(

Powered by Google App Engine
This is Rietveld 408576698