Start the migration of passwords from the Keychain.

chrome/browser/password_manager/password_store_proxy_mac.cc

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 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(