Start the migration of passwords from the Keychain.

chrome/browser/password_manager/password_store_proxy_mac_unittest.cc

Index: chrome/browser/password_manager/password_store_proxy_mac_unittest.cc
diff --git a/chrome/browser/password_manager/password_store_proxy_mac_unittest.cc b/chrome/browser/password_manager/password_store_proxy_mac_unittest.cc
index 76c8e64fee003d4cd1d46aec7debc21ea92a40e0..c9e4d81527195e33d1f428f893feb7bda251f3d7 100644
--- a/chrome/browser/password_manager/password_store_proxy_mac_unittest.cc
+++ b/chrome/browser/password_manager/password_store_proxy_mac_unittest.cc
@@ -7,10 +7,14 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/scoped_observer.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/password_manager/password_store_mac_internal.h"
+#include "chrome/browser/prefs/browser_prefs.h"
+#include "chrome/test/base/testing_pref_service_syncable.h"
 #include "components/os_crypt/os_crypt.h"
 #include "components/password_manager/core/browser/login_database.h"
 #include "components/password_manager/core/browser/password_manager_test_utils.h"
 #include "components/password_manager/core/browser/password_store_consumer.h"
+#include "components/password_manager/core/common/password_manager_pref_names.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "crypto/mock_apple_keychain.h"
@@ -21,6 +25,8 @@ namespace {
 
 using autofill::PasswordForm;
 using content::BrowserThread;
+using password_manager::PasswordStoreChange;
+using password_manager::PasswordStoreChangeList;
 using testing::_;
 using testing::ElementsAre;
 using testing::IsEmpty;
@@ -31,6 +37,11 @@ ACTION(QuitUIMessageLoop) {
   base::MessageLoop::current()->Quit();
 }
 
+PasswordStoreChangeList AddChangeForForm(const PasswordForm& form) {
+  return PasswordStoreChangeList(
+      1, PasswordStoreChange(PasswordStoreChange::ADD, form));
+}
+
 class MockPasswordStoreConsumer
     : public password_manager::PasswordStoreConsumer {
  public:
@@ -70,8 +81,12 @@ class BadLoginDatabase : public password_manager::LoginDatabase {
   DISALLOW_COPY_AND_ASSIGN(BadLoginDatabase);
 };
 
-class PasswordStoreProxyMacTest : public testing::Test {
+class PasswordStoreProxyMacTest
+    : public testing::TestWithParam<PasswordStoreProxyMac::MigrationStatus> {
  public:
+  PasswordStoreProxyMacTest();
+  ~PasswordStoreProxyMacTest() override;
+
   void SetUp() override;
   void TearDown() override;
 
@@ -106,14 +121,23 @@ class PasswordStoreProxyMacTest : public testing::Test {
 
   base::ScopedTempDir db_dir_;
   scoped_refptr<PasswordStoreProxyMac> store_;
+  TestingPrefServiceSyncable testing_prefs_;
 };
 
-void PasswordStoreProxyMacTest::SetUp() {
-  ASSERT_TRUE(db_dir_.CreateUniqueTempDir());
-
+PasswordStoreProxyMacTest::PasswordStoreProxyMacTest() {
+  EXPECT_TRUE(db_dir_.CreateUniqueTempDir());
+  chrome::RegisterUserProfilePrefs(testing_prefs_.registry());
+  testing_prefs_.SetInteger(password_manager::prefs::kKeychainMigrationStatus,
+                            GetParam());
   // Ensure that LoginDatabase will use the mock keychain if it needs to
   // encrypt/decrypt a password.
   OSCrypt::UseMockKeychain(true);
+}
+
+PasswordStoreProxyMacTest::~PasswordStoreProxyMacTest() {
+}
+
+void PasswordStoreProxyMacTest::SetUp() {
   scoped_ptr<password_manager::LoginDatabase> login_db(
       new password_manager::LoginDatabase(test_login_db_file_path()));
   CreateAndInitPasswordStore(login_db.Pass());
@@ -130,7 +154,8 @@ void PasswordStoreProxyMacTest::CreateAndInitPasswordStore(
     scoped_ptr<password_manager::LoginDatabase> login_db) {
   store_ = new PasswordStoreProxyMac(
       BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI),
-      make_scoped_ptr(new crypto::MockAppleKeychain), login_db.Pass());
+      make_scoped_ptr(new crypto::MockAppleKeychain), login_db.Pass(),
+      &testing_prefs_);
   ASSERT_TRUE(store_->Init(syncer::SyncableService::StartSyncFlare()));
 }
 
@@ -227,7 +252,9 @@ void PasswordStoreProxyMacTest::CheckRemoveLoginsBetween(bool check_created) {
   FinishAsyncProcessing();
 }
 
-TEST_F(PasswordStoreProxyMacTest, FormLifeCycle) {
+// ----------- Tests -------------
+
+TEST_P(PasswordStoreProxyMacTest, FormLifeCycle) {
   PasswordForm password_form;
   password_form.origin = GURL("http://example.com");
   password_form.username_value = base::ASCIIToUTF16("test1@gmail.com");
@@ -238,17 +265,26 @@ TEST_F(PasswordStoreProxyMacTest, FormLifeCycle) {
   password_form.password_value = base::ASCIIToUTF16("password");
   UpdateForm(password_form);
   RemoveForm(password_form);
+
+  int status = testing_prefs_.GetInteger(
+      password_manager::prefs::kKeychainMigrationStatus);
+  if (GetParam() == PasswordStoreProxyMac::NOT_STARTED ||
+      GetParam() == PasswordStoreProxyMac::FAILED_ONCE) {
+    EXPECT_EQ(PasswordStoreProxyMac::MIGRATED, status);
+  } else {
+    EXPECT_EQ(GetParam(), status);
+  }
 }
 
-TEST_F(PasswordStoreProxyMacTest, TestRemoveLoginsCreatedBetween) {
+TEST_P(PasswordStoreProxyMacTest, TestRemoveLoginsCreatedBetween) {
   CheckRemoveLoginsBetween(true);
 }
 
-TEST_F(PasswordStoreProxyMacTest, TestRemoveLoginsSyncedBetween) {
+TEST_P(PasswordStoreProxyMacTest, TestRemoveLoginsSyncedBetween) {
   CheckRemoveLoginsBetween(false);
 }
 
-TEST_F(PasswordStoreProxyMacTest, FillLogins) {
+TEST_P(PasswordStoreProxyMacTest, FillLogins) {
   PasswordForm password_form;
   password_form.origin = GURL("http://example.com");
   password_form.signon_realm = "http://example.com/";
@@ -283,7 +319,7 @@ TEST_F(PasswordStoreProxyMacTest, FillLogins) {
   base::MessageLoop::current()->Run();
 }
 
-TEST_F(PasswordStoreProxyMacTest, OperationsOnABadDatabaseSilentlyFail) {
+TEST_P(PasswordStoreProxyMacTest, OperationsOnABadDatabaseSilentlyFail) {
   // Verify that operations on a PasswordStore with a bad database cause no
   // explosions, but fail without side effect, return no data and trigger no
   // notifications.
@@ -348,4 +384,90 @@ TEST_F(PasswordStoreProxyMacTest, OperationsOnABadDatabaseSilentlyFail) {
   // Verify no notifications are fired during shutdown either.
   ClosePasswordStore();
 }
+
+INSTANTIATE_TEST_CASE_P(,
+                        PasswordStoreProxyMacTest,
+                        testing::Values(PasswordStoreProxyMac::NOT_STARTED,
+                                        PasswordStoreProxyMac::MIGRATED,
+                                        PasswordStoreProxyMac::FAILED_ONCE,
+                                        PasswordStoreProxyMac::FAILED_TWICE));
+
+// Test the migration process.
+class PasswordStoreProxyMacMigrationTest : public PasswordStoreProxyMacTest {
+ public:
+  void SetUp() override;
+
+  void TestMigration(bool lock_keychain);
+
+ protected:
+  scoped_ptr<password_manager::LoginDatabase> login_db_;
+  scoped_ptr<crypto::MockAppleKeychain> keychain_;
+};
+
+void PasswordStoreProxyMacMigrationTest::SetUp() {
+  login_db_.reset(
+      new password_manager::LoginDatabase(test_login_db_file_path()));
+  keychain_.reset(new crypto::MockAppleKeychain);
+}
+
+void PasswordStoreProxyMacMigrationTest::TestMigration(bool lock_keychain) {
+  PasswordForm form;
+  form.origin = GURL("http://accounts.google.com/LoginAuth");
+  form.signon_realm = "http://accounts.google.com/";
+  form.username_value = base::ASCIIToUTF16("my_username");
+  form.password_value = base::ASCIIToUTF16("12345");
+
+  login_db_->set_clear_password_values(true);
+  EXPECT_TRUE(login_db_->Init());
+  EXPECT_EQ(AddChangeForForm(form), login_db_->AddLogin(form));
+  login_db_.reset(

[vabr (Chromium), 2015/07/03 08:40:48]

Is this just to undo lines 420 and 421? Perhaps a code comment would help to
understand the purpose of this line in any case.

[vasilii, 2015/07/03 12:59:11]

I needed another instance because LoginDatabase doesn't tolerate double init.
The second init happened inside PasswordStoreProxyMac. The content on the other
hand is the same.

[vabr (Chromium), 2015/07/06 09:50:19]

Acknowledged.

+      new password_manager::LoginDatabase(test_login_db_file_path()));
+  MacKeychainPasswordFormAdapter adapter(keychain_.get());
+  EXPECT_TRUE(adapter.AddPassword(form));
+
+  // Force migration.
+  if (lock_keychain)
+    keychain_->set_locked(true);
+  crypto::MockAppleKeychain* weak_keychain = keychain_.get();
+  store_ = new PasswordStoreProxyMac(
+      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI),
+      keychain_.Pass(), login_db_.Pass(), &testing_prefs_);
+  ASSERT_TRUE(store_->Init(syncer::SyncableService::StartSyncFlare()));
+  FinishAsyncProcessing();
+
+  // Check the password is still there.
+  if (lock_keychain)
+    weak_keychain->set_locked(false);
+  MockPasswordStoreConsumer mock_consumer;
+  store()->GetLogins(form, PasswordStoreProxyMac::ALLOW_PROMPT, &mock_consumer);
+  EXPECT_CALL(mock_consumer,
+              OnGetPasswordStoreResultsConstRef(ElementsAre(Pointee(form))))
+      .WillOnce(QuitUIMessageLoop());
+  base::MessageLoop::current()->Run();
+
+  int status = testing_prefs_.GetInteger(
+      password_manager::prefs::kKeychainMigrationStatus);
+  if (lock_keychain) {
+    EXPECT_EQ(GetParam() == PasswordStoreProxyMac::NOT_STARTED
+                  ? PasswordStoreProxyMac::FAILED_ONCE
+                  : PasswordStoreProxyMac::FAILED_TWICE,
+              status);
+  } else {
+    EXPECT_EQ(PasswordStoreProxyMac::MIGRATED, status);
+  }
+}
+
+TEST_P(PasswordStoreProxyMacMigrationTest, TestSuccessfullMigration) {
+  TestMigration(false);
+}
+
+TEST_P(PasswordStoreProxyMacMigrationTest, TestFailedMigration) {
+  TestMigration(true);
+}
+
+INSTANTIATE_TEST_CASE_P(,
+                        PasswordStoreProxyMacMigrationTest,
+                        testing::Values(PasswordStoreProxyMac::NOT_STARTED,

[vabr (Chromium), 2015/07/03 08:40:48]

Would it make sense to run this test also for the other migration statuses (with
appropriate changes to lines 451-453)?
While I understand that no migration should happen in those other cases, that
probably also needs to be tested (in the sense that whatever happens, the logins
are available from the proxy after it is created with a non-empty database and
keychain).

[vasilii, 2015/07/03 12:59:11]

Done.

+                                        PasswordStoreProxyMac::FAILED_ONCE));
+
 }  // namespace