Start the migration of passwords from the Keychain.

chrome/browser/password_manager/password_store_proxy_mac.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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() {
 }
 
 bool PasswordStoreProxyMac::Init(
     const syncer::SyncableService::StartSyncFlare& flare) {
   // Set up a background thread.
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   thread_.reset(new base::Thread("Chrome_PasswordStore_Thread"));
 
   if (!thread_->Start()) {
     thread_.reset();
     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();
+  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();
-  thread_->Stop();
 }
 
 scoped_refptr<base::SingleThreadTaskRunner>
 PasswordStoreProxyMac::GetBackgroundTaskRunner() {
   return thread_ ? thread_->task_runner() : nullptr;
 }
 
 password_manager::PasswordStore* PasswordStoreProxyMac::GetBackend() const {
   if (password_store_mac_)
     return password_store_mac_.get();
   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 conditionally is elsewhere in
the file), maybe DCHECK(!password_store_simple) just before 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 be updated and persistent
without the pref having been stored, perhaps the migration logic should be
updated to skip any entries in the DB that already have a non-empty password?

Although I can see the argument that it's so rare that just doing the work a
second time in that case isn't a big deal.

[vasilii, 2015/07/08 09:56:58]

There is no race. The preference is written in PasswordStoreProxyMac::Shutdown()
at latest. It's tested in PasswordStoreProxyMacTest.StartAndStop

[stuartmorgan, 2015/07/08 14:18:40]

I meant in case of a crash; there's a window of time where a crash could happen
after the DB has been written to disk, but before the pref.

[vasilii, 2015/07/08 15:45:54]

I think it's rare and on the next run the passwords still should be accessible.
This optimisation makes possible another bug when the encryption key has changed
for some reason.

+    }
+  }
+  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(
     const std::string& sync_username,
     bool custom_passphrase_sync_enabled) {
   GetBackend()->ReportMetricsImpl(sync_username,
                                   custom_passphrase_sync_enabled);
 }
 
 PasswordStoreChangeList PasswordStoreProxyMac::AddLoginImpl(
@@ skipping 45 matching lines @@
 }
 
 void PasswordStoreProxyMac::RemoveSiteStatsImpl(const GURL& origin_domain) {
   GetBackend()->RemoveSiteStatsImpl(origin_domain);
 }
 
 scoped_ptr<password_manager::InteractionsStats>
 PasswordStoreProxyMac::GetSiteStatsImpl(const GURL& origin_domain) {
   return GetBackend()->GetSiteStatsImpl(origin_domain);
 }