Implement origin-based deletion for passwords in PasswordDefaultMac.

chrome/browser/password_manager/password_store_mac_unittest.cc

 // Copyright (c) 2012 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_mac.h"
 
 #include "base/basictypes.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/scoped_observer.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/test/histogram_tester.h"
 #include "base/thread_task_runner_handle.h"
 #include "chrome/browser/password_manager/password_store_mac_internal.h"
 #include "chrome/common/chrome_paths.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_common_unittest.h"
 #include "components/password_manager/core/browser/password_store_consumer.h"
 #include "content/public/test/test_browser_thread.h"
 #include "content/public/test/test_utils.h"
 #include "crypto/mock_apple_keychain.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using autofill::PasswordForm;
 using base::ASCIIToUTF16;
 using base::WideToUTF16;
@@ skipping 36 matching lines @@
  public:
   MOCK_METHOD1(OnGetPasswordStoreResultsConstRef,
                void(const std::vector<PasswordForm*>&));
 
   // GMock cannot mock methods with move-only args.
   void OnGetPasswordStoreResults(ScopedVector<PasswordForm> results) override {
     OnGetPasswordStoreResultsConstRef(results.get());
   }
 };
 
-class MockPasswordStoreObserver : public PasswordStore::Observer {
- public:
-  MOCK_METHOD1(OnLoginsChanged,
-               void(const password_manager::PasswordStoreChangeList& changes));
-};
-
 // A LoginDatabase that simulates an Init() method that takes a long time.
 class SlowToInitLoginDatabase : public password_manager::LoginDatabase {
  public:
   // Creates an instance whose Init() method will block until |event| is
   // signaled. |event| must outlive |this|.
   SlowToInitLoginDatabase(const base::FilePath& db_path,
                           base::WaitableEvent* event)
       : password_manager::LoginDatabase(db_path), event_(event) {}
   ~SlowToInitLoginDatabase() override {}
 
@@ skipping 74 matching lines @@
   }
 }
 
 PasswordStoreChangeList AddChangeForForm(const PasswordForm& form) {
   return PasswordStoreChangeList(
       1, PasswordStoreChange(PasswordStoreChange::ADD, form));
 }
 
 }  // namespace
 
+namespace password_manager {

[vasilii, 2015/10/21 16:47:56]

You don't need this namespace. Use the anonymous one.

[Timo Reimann, 2015/11/17 23:10:55]

Done.

+
+class PasswordStoreMacTestDelegate {
+ public:
+  PasswordStoreMacTestDelegate()
+      : ui_thread_(BrowserThread::UI, &message_loop_) {
+    initialize();

[vasilii, 2015/10/21 16:47:56]

Method's name should start with a capital letter.

[Timo Reimann, 2015/11/17 23:10:55]

Done.

+  }
+  ~PasswordStoreMacTestDelegate() {
+    ClosePasswordStore();
+    thread_.reset();
+    login_db_.reset();
+  }
+
+  PasswordStoreMac* store() { return store_.get(); }
+
+  void FinishAsyncProcessing() {
+    scoped_refptr<content::MessageLoopRunner> runner =
+        new content::MessageLoopRunner;
+    ASSERT_TRUE(thread_->task_runner()->PostTaskAndReply(
+        FROM_HERE, base::Bind(&Noop), runner->QuitClosure()));
+    runner->Run();
+  }
+
+  base::Thread* thread() { return thread_.get(); }

[vasilii, 2015/10/21 16:47:56]

Why is it needed?

[Timo Reimann, 2015/11/17 23:10:55]

PasswordStoreMacTest.ImportFromKeychain,
PasswordStoreMacTest.ImportFederatedFromLockedKeychain, and
PasswordStoreMacTest.ImportFromLockedKeychainError all post
PasswordStoreMac::ImportFromKeychain to the thread's task runner during the
respective tests.

[vasilii, 2015/11/18 16:27:42]

You can expose a task runner, not a thread.

[Timo Reimann, 2015/11/18 23:42:14]

I use ThreadTaskRunnerHandle now. Hope that's the right approach.

+
+ private:
+  base::MessageLoopForUI message_loop_;

[vasilii, 2015/10/21 16:47:56]

Methods should be above. They are complex enough so don't make them inline.

[Timo Reimann, 2015/11/17 23:10:55]

Done (for PasswordStoreDefaultTest too).

+  content::TestBrowserThread ui_thread_;
+  // Thread that the synchronous methods are run on.
+  scoped_ptr<base::Thread> thread_;
+
+  base::ScopedTempDir db_dir_;
+  scoped_ptr<LoginDatabase> login_db_;
+  scoped_refptr<PasswordStoreMac> store_;
+
+  static void InitLoginDatabase(LoginDatabase* login_db) {
+    ASSERT_TRUE(login_db->Init());
+  }
+
+  void initialize() {

[Timo Reimann, 2015/10/20 20:47:09]

The initialization of PasswordStoreMac for testing purposes seems more
complicated that the equivalent for PasswordStoreDefault. For example, the Mac
version manages at least one more thread.

I am wondering if we can (and should) try to align the two approaches, either by
making one more sophisticated (and thus supposedly more correct) or streamlining
the other (and thus remove supposedly extra complexity).

[vasilii, 2015/10/21 16:47:56]

For your tests you can definitely simplify PasswordStoreMac creation by using
just one thread as for PasswordStoreDefault.
However, it should not change PasswordStoreMacTest logic. So choose one
approach.

+    ASSERT_TRUE(db_dir_.CreateUniqueTempDir());
+
+    // Ensure that LoginDatabase will use the mock keychain if it needs to
+    // encrypt/decrypt a password.
+    OSCrypt::UseMockKeychain(true);

[vasilii, 2015/10/21 16:47:56]

You just pass the Keychain instance into constructor.

[Timo Reimann, 2015/11/17 23:10:55]

Which constructor do you mean? The test already passes an instance of
MockAppleKeychain into PasswordStoreMac's constructor. However, that does not
seem to suffice: Commenting out the call to OSCrypt::UseMockKeychain(true) leads
to failing tests.

+    login_db_.reset(new LoginDatabase(test_login_db_file_path()));
+    thread_.reset(new base::Thread("Chrome_PasswordStore_Thread"));
+    ASSERT_TRUE(thread_->Start());
+    ASSERT_TRUE(thread_->task_runner()->PostTask(
+        FROM_HERE, base::Bind(&PasswordStoreMacTestDelegate::InitLoginDatabase,
+                              base::Unretained(login_db_.get()))));
+    CreateAndInitPasswordStore(login_db_.get());
+    // Make sure deferred initialization is performed before some tests start
+    // accessing the |login_db| directly.
+    FinishAsyncProcessing();
+  }
+
+  base::FilePath test_login_db_file_path() const {
+    return db_dir_.path().Append(FILE_PATH_LITERAL("login.db"));
+  }
+
+  void CreateAndInitPasswordStore(LoginDatabase* login_db) {

[vasilii, 2015/10/21 16:47:56]

merge the method into initialize()

[Timo Reimann, 2015/11/17 23:10:55]

Done.

+    store_ = new PasswordStoreMac(
+        base::ThreadTaskRunnerHandle::Get(), nullptr,
+        make_scoped_ptr<AppleKeychain>(new MockAppleKeychain));
+    ASSERT_TRUE(thread_->task_runner()->PostTask(
+        FROM_HERE, base::Bind(&PasswordStoreMac::InitWithTaskRunner, store_,
+                              thread_->task_runner())));
+
+    ASSERT_TRUE(thread_->task_runner()->PostTask(
+        FROM_HERE, base::Bind(&PasswordStoreMac::set_login_metadata_db, store_,
+                              base::Unretained(login_db))));
+  }
+
+  void ClosePasswordStore() {
+    if (!store_)

[vasilii, 2015/10/21 16:47:56]

It can't be true.

[Timo Reimann, 2015/11/17 23:10:55]

I agree, unless there's a bug where store is attempted to be deleted twice.

Should we completely drop the if-clause or replace it by a DCHECK, just to be
safe and catch potential bugs early on?

[vasilii, 2015/11/18 16:27:42]

Just drop.

[Timo Reimann, 2015/11/18 23:42:14]

Done.

+      return;
+
+    store_->Shutdown();
+    store_ = nullptr;
+  }
+};
+
+INSTANTIATE_TYPED_TEST_CASE_P(Mac,
+                              PasswordStoreCommonTest,
+                              PasswordStoreMacTestDelegate);
+
+}  // namespace password_manager
+
 #pragma mark -
 
 class PasswordStoreMacInternalsTest : public testing::Test {
  public:
   void SetUp() override {
     MockAppleKeychain::KeychainTestData test_data[] = {
         // Basic HTML form.
         {kSecAuthenticationTypeHTMLForm,
          "some.domain.com",
          kSecProtocolTypeHTTP,
@@ skipping 979 matching lines @@
 
   ScopedVector<autofill::PasswordForm> owned_passwords =
       owned_keychain_adapter.GetAllPasswordFormPasswords();
   EXPECT_EQ(arraysize(owned_password_data), owned_passwords.size());
 }
 
 #pragma mark -
 
 class PasswordStoreMacTest : public testing::Test {
  public:
-  PasswordStoreMacTest() : ui_thread_(BrowserThread::UI, &message_loop_) {}
+  PasswordStoreMacTest()
+      : histogram_tester_(new base::HistogramTester),
+        store_(delegate_.store()) {}

[vasilii, 2015/10/21 16:47:56]

I'm generally against changing this class. Instead you can simplify the delegate
code a lot.

[Timo Reimann, 2015/11/17 23:10:55]

I'm not sure I understand: The delegate was created by moving parts of
PasswordStoreMacTest into it in order to provide a self-contained class to the
new type-parameterized test template. So it seems we need to do at least a
certain amount of modifications to PasswordStoreMacTest.

Could you please elaborate on how the delegate should be simplified from your
perspective?

 
-  void SetUp() override {
-    ASSERT_TRUE(db_dir_.CreateUniqueTempDir());
-    histogram_tester_.reset(new base::HistogramTester);
-
-    // Ensure that LoginDatabase will use the mock keychain if it needs to
-    // encrypt/decrypt a password.
-    OSCrypt::UseMockKeychain(true);
-    login_db_.reset(
-        new password_manager::LoginDatabase(test_login_db_file_path()));
-    thread_.reset(new base::Thread("Chrome_PasswordStore_Thread"));
-    ASSERT_TRUE(thread_->Start());
-    ASSERT_TRUE(thread_->task_runner()->PostTask(
-        FROM_HERE, base::Bind(&PasswordStoreMacTest::InitLoginDatabase,
-                              base::Unretained(login_db_.get()))));
-    CreateAndInitPasswordStore(login_db_.get());
-    // Make sure deferred initialization is performed before some tests start
-    // accessing the |login_db| directly.
-    FinishAsyncProcessing();
-  }
-
-  void TearDown() override {
-    ClosePasswordStore();
-    thread_.reset();
-    login_db_.reset();
+  ~PasswordStoreMacTest() override {
     // Whatever a test did, PasswordStoreMac stores only empty password values
     // in LoginDatabase. The empty valus do not require encryption and therefore
     // OSCrypt shouldn't call the Keychain. The histogram doesn't cover the
     // internet passwords.
     if (histogram_tester_) {
       histogram_tester_->ExpectTotalCount("OSX.Keychain.Access", 0);
     }
   }
 
-  static void InitLoginDatabase(password_manager::LoginDatabase* login_db) {
-    ASSERT_TRUE(login_db->Init());
-  }
-
-  void CreateAndInitPasswordStore(password_manager::LoginDatabase* login_db) {
-    store_ = new PasswordStoreMac(
-        base::ThreadTaskRunnerHandle::Get(), nullptr,
-        make_scoped_ptr<AppleKeychain>(new MockAppleKeychain));
-    ASSERT_TRUE(thread_->task_runner()->PostTask(
-        FROM_HERE, base::Bind(&PasswordStoreMac::InitWithTaskRunner, store_,
-                              thread_->task_runner())));
-
-    ASSERT_TRUE(thread_->task_runner()->PostTask(
-        FROM_HERE, base::Bind(&PasswordStoreMac::set_login_metadata_db, store_,
-                              base::Unretained(login_db))));
-  }
-
-  void ClosePasswordStore() {
-    if (!store_)
-      return;
-
-    store_->Shutdown();
-    store_ = nullptr;
-  }
-
   // Verifies that the given |form| can be properly stored so that it can be
   // retrieved by FillMatchingLogins() and GetAutofillableLogins(), and then it
   // can be properly removed.
   void VerifyCredentialLifecycle(const PasswordForm& form) {
     // Run everything twice to make sure no garbage is left behind that would
     // prevent storing the form a second time.
     for (size_t iteration = 0; iteration < 2; ++iteration) {
       SCOPED_TRACE(testing::Message("Iteration: ") << iteration);
 
       MockPasswordStoreConsumer mock_consumer;
       EXPECT_CALL(mock_consumer, OnGetPasswordStoreResultsConstRef(IsEmpty()))
           .WillOnce(QuitUIMessageLoop());
       store()->GetAutofillableLogins(&mock_consumer);
       base::MessageLoop::current()->Run();
       ::testing::Mock::VerifyAndClearExpectations(&mock_consumer);
 
       store()->AddLogin(form);
-      FinishAsyncProcessing();
+      delegate_.FinishAsyncProcessing();
 
       PasswordForm returned_form;
       EXPECT_CALL(mock_consumer, OnGetPasswordStoreResultsConstRef(SizeIs(1u)))
           .WillOnce(
               DoAll(SaveACopyOfFirstForm(&returned_form), QuitUIMessageLoop()));
 
       // The query operations will also do some housekeeping: they will remove
       // dangling credentials in the LoginDatabase without a matching Keychain
       // item when one is expected. If the logic that stores the Keychain item
       // is incorrect, this will wipe the newly added form before the second
@@ skipping 12 matching lines @@
       store()->GetLogins(query_form, PasswordStore::ALLOW_PROMPT,
                          &mock_consumer);
       base::MessageLoop::current()->Run();
       ::testing::Mock::VerifyAndClearExpectations(&mock_consumer);
       EXPECT_EQ(form, returned_form);
 
       store()->RemoveLogin(form);
     }
   }
 
-  base::FilePath test_login_db_file_path() const {
-    return db_dir_.path().Append(FILE_PATH_LITERAL("login.db"));
-  }
-
   password_manager::LoginDatabase* login_db() const {
     return store_->login_metadata_db();
   }
 
   MockAppleKeychain* keychain() {
     return static_cast<MockAppleKeychain*>(store_->keychain());
   }
 
-  void FinishAsyncProcessing() {
-    scoped_refptr<content::MessageLoopRunner> runner =
-        new content::MessageLoopRunner;
-    ASSERT_TRUE(thread_->task_runner()->PostTaskAndReply(
-        FROM_HERE, base::Bind(&Noop), runner->QuitClosure()));
-    runner->Run();
+  PasswordStoreMac* store() { return store_; }
+
+  password_manager::PasswordStoreMacTestDelegate& delegate() {
+    return delegate_;
   }
 
-  PasswordStoreMac* store() { return store_.get(); }
-
  protected:
-  base::MessageLoopForUI message_loop_;
-  content::TestBrowserThread ui_thread_;
-  // Thread that the synchronous methods are run on.
-  scoped_ptr<base::Thread> thread_;
-
-  base::ScopedTempDir db_dir_;
-  scoped_ptr<password_manager::LoginDatabase> login_db_;
-  scoped_refptr<PasswordStoreMac> store_;
   scoped_ptr<base::HistogramTester> histogram_tester_;
+  password_manager::PasswordStoreMacTestDelegate delegate_;
+  PasswordStoreMac* store_;
 };
 
 TEST_F(PasswordStoreMacTest, TestStoreUpdate) {
   // Insert a password into both the database and the keychain.
   // This is done manually, rather than through store_->AddLogin, because the
   // Mock Keychain isn't smart enough to be able to support update generically,
   // so some.domain.com triggers special handling to test it that make inserting
   // fail.
   PasswordFormData joint_data = {
     PasswordForm::SCHEME_HTML, "http://some.domain.com/",
@@ skipping 48 matching lines @@
         true, false, 2 },
       NULL,
     },
   };
   for (unsigned int i = 0; i < arraysize(updates); ++i) {
     scoped_ptr<PasswordForm> form =
         CreatePasswordFormFromDataForTesting(updates[i].form_data);
     store_->UpdateLogin(*form);
   }
 
-  FinishAsyncProcessing();
+  delegate_.FinishAsyncProcessing();
 
   MacKeychainPasswordFormAdapter keychain_adapter(keychain());
   for (unsigned int i = 0; i < arraysize(updates); ++i) {
     scoped_ptr<PasswordForm> query_form =
         CreatePasswordFormFromDataForTesting(updates[i].form_data);
 
     ScopedVector<autofill::PasswordForm> matching_items =
         keychain_adapter.PasswordsFillingForm(query_form->signon_realm,
                                               query_form->scheme);
     if (updates[i].password) {
@@ skipping 56 matching lines @@
   // 3. Add the returned password for m.facebook.com.
   returned_form.signon_realm = "http://m.facebook.com";
   returned_form.origin = GURL("http://m.facebook.com/index.html");
   EXPECT_EQ(AddChangeForForm(returned_form),
             login_db()->AddLogin(returned_form));
   owned_keychain_adapter.AddPassword(m_form);
 
   // 4. Remove both passwords.
   store_->RemoveLogin(*www_form);
   store_->RemoveLogin(m_form);
-  FinishAsyncProcessing();
+  delegate_.FinishAsyncProcessing();
 
   // No trace of www.facebook.com.
   ScopedVector<autofill::PasswordForm> matching_items =
       owned_keychain_adapter.PasswordsFillingForm(www_form->signon_realm,
                                                   www_form->scheme);
   EXPECT_EQ(0u, matching_items.size());
   EXPECT_TRUE(login_db()->GetLogins(*www_form, &matching_items));
   EXPECT_EQ(0u, matching_items.size());
   // No trace of m.facebook.com.
   matching_items = owned_keychain_adapter.PasswordsFillingForm(
       m_form.signon_realm, m_form.scheme);
   EXPECT_EQ(0u, matching_items.size());
   EXPECT_TRUE(login_db()->GetLogins(m_form, &matching_items));
   EXPECT_EQ(0u, matching_items.size());
 }
 
 namespace {
 
 class PasswordsChangeObserver :
     public password_manager::PasswordStore::Observer {
 public:
  PasswordsChangeObserver(PasswordStoreMac* store) : observer_(this) {
     observer_.Add(store);
   }
 
   void WaitAndVerify(PasswordStoreMacTest* test) {
-    test->FinishAsyncProcessing();
+    test->delegate().FinishAsyncProcessing();
     ::testing::Mock::VerifyAndClearExpectations(this);
   }
 
   // password_manager::PasswordStore::Observer:
   MOCK_METHOD1(OnLoginsChanged,
                void(const password_manager::PasswordStoreChangeList& changes));
 
 private:
   ScopedObserver<password_manager::PasswordStore,
                 PasswordsChangeObserver> observer_;
@@ skipping 135 matching lines @@
       CreatePasswordFormFromDataForTesting(www_form_data1);
   EXPECT_TRUE(owned_keychain_adapter.AddPassword(*www_form));
 
   // Add a password from the current profile.
   PasswordFormData www_form_data2 = {
       PasswordForm::SCHEME_HTML, "http://www.facebook.com/",
       "http://www.facebook.com/index.html", "login", L"username", L"password",
       L"submit", L"not_joe_user", L"12345", true, false, 1 };
   www_form = CreatePasswordFormFromDataForTesting(www_form_data2);
   store_->AddLogin(*www_form);
-  FinishAsyncProcessing();
+  delegate_.FinishAsyncProcessing();
 
   ScopedVector<PasswordForm> matching_items;
   EXPECT_TRUE(login_db()->GetLogins(*www_form, &matching_items));
   EXPECT_EQ(1u, matching_items.size());
 
   store_->RemoveLoginsCreatedBetween(base::Time(), base::Time(),
                                      base::Closure());
-  FinishAsyncProcessing();
+  delegate_.FinishAsyncProcessing();
 
   // Check the second facebook form is gone.
   EXPECT_TRUE(login_db()->GetLogins(*www_form, &matching_items));
   EXPECT_EQ(0u, matching_items.size());
 
   // Check the first facebook form is still there.
   matching_items = owned_keychain_adapter.PasswordsFillingForm(
       www_form->signon_realm, www_form->scheme);
   ASSERT_EQ(1u, matching_items.size());
   EXPECT_EQ(ASCIIToUTF16("joe_user"), matching_items[0]->username_value);
 
   // Check the third-party password is still there.
   owned_keychain_adapter.SetFindsOnlyOwnedItems(false);
   matching_items = owned_keychain_adapter.PasswordsFillingForm(
       "http://some.domain.com/insecure.html", PasswordForm::SCHEME_HTML);
   ASSERT_EQ(1u, matching_items.size());
 }
 
 // Add a facebook form to the store but not to the keychain. The form is to be
 // implicitly deleted. However, the observers shouldn't get notified about
 // deletion of non-existent forms like m.facebook.com.
 TEST_F(PasswordStoreMacTest, SilentlyRemoveOrphanedForm) {
-  testing::StrictMock<MockPasswordStoreObserver> mock_observer;
+  testing::StrictMock<password_manager::MockPasswordStoreObserver>
+      mock_observer;
   store()->AddObserver(&mock_observer);
 
   // 1. Add a password for www.facebook.com to the LoginDatabase.
   PasswordFormData www_form_data = {
     PasswordForm::SCHEME_HTML, "http://www.facebook.com/",
     "http://www.facebook.com/index.html", "login",
     L"username", L"password", L"submit", L"joe_user", L"", true, false, 1
   };
   scoped_ptr<PasswordForm> www_form(
       CreatePasswordFormFromDataForTesting(www_form_data));
@@ skipping 75 matching lines @@
   form2.password_value = ASCIIToUTF16("my_password");
 
   PasswordForm blacklisted_form;
   blacklisted_form.origin = GURL("http://badsite.com/Login");
   blacklisted_form.signon_realm = "http://badsite.com/";
   blacklisted_form.blacklisted_by_user = true;
 
   store()->AddLogin(form1);
   store()->AddLogin(form2);
   store()->AddLogin(blacklisted_form);
-  FinishAsyncProcessing();
+  delegate_.FinishAsyncProcessing();
 
   ASSERT_TRUE(base::PostTaskAndReplyWithResult(
-      thread_->task_runner().get(), FROM_HERE,
+      delegate_.thread()->task_runner().get(), FROM_HERE,
       base::Bind(&PasswordStoreMac::ImportFromKeychain, store()),
       base::Bind(&CheckMigrationResult, PasswordStoreMac::MIGRATION_OK)));
-  FinishAsyncProcessing();
+  delegate_.FinishAsyncProcessing();
 
   // The password should be stored in the database by now.
   ScopedVector<PasswordForm> matching_items;
   EXPECT_TRUE(login_db()->GetLogins(form1, &matching_items));
   ASSERT_EQ(1u, matching_items.size());
   EXPECT_EQ(form1, *matching_items[0]);
 
   EXPECT_TRUE(login_db()->GetLogins(form2, &matching_items));
   ASSERT_EQ(1u, matching_items.size());
   EXPECT_EQ(form2, *matching_items[0]);
@@ skipping 11 matching lines @@
 // Import a federated credential while the Keychain is locked.
 TEST_F(PasswordStoreMacTest, ImportFederatedFromLockedKeychain) {
   keychain()->set_locked(true);
   PasswordForm form1;
   form1.origin = GURL("http://example.com/Login");
   form1.signon_realm = "http://example.com/";
   form1.username_value = ASCIIToUTF16("my_username");
   form1.federation_url = GURL("https://accounts.google.com/");
 
   store()->AddLogin(form1);
-  FinishAsyncProcessing();
+  delegate_.FinishAsyncProcessing();
   ASSERT_TRUE(base::PostTaskAndReplyWithResult(
-      thread_->task_runner().get(), FROM_HERE,
+      delegate_.thread()->task_runner().get(), FROM_HERE,
       base::Bind(&PasswordStoreMac::ImportFromKeychain, store()),
       base::Bind(&CheckMigrationResult, PasswordStoreMac::MIGRATION_OK)));
-  FinishAsyncProcessing();
+  delegate_.FinishAsyncProcessing();
 
   ScopedVector<PasswordForm> matching_items;
   EXPECT_TRUE(login_db()->GetLogins(form1, &matching_items));
   ASSERT_EQ(1u, matching_items.size());
   EXPECT_EQ(form1, *matching_items[0]);
 }
 
 // Try to import while the Keychain is locked but the encryption key had been
 // read earlier.
 TEST_F(PasswordStoreMacTest, ImportFromLockedKeychainError) {
   PasswordForm form1;
   form1.origin = GURL("http://accounts.google.com/LoginAuth");
   form1.signon_realm = "http://accounts.google.com/";
   form1.username_value = ASCIIToUTF16("my_username");
   form1.password_value = ASCIIToUTF16("my_password");
   store()->AddLogin(form1);
-  FinishAsyncProcessing();
+  delegate_.FinishAsyncProcessing();
 
   // Add a second keychain item matching the Database entry.
   PasswordForm form2 = form1;
   form2.origin = GURL("http://accounts.google.com/Login");
   form2.password_value = ASCIIToUTF16("1234");
   MacKeychainPasswordFormAdapter adapter(keychain());
   EXPECT_TRUE(adapter.AddPassword(form2));
 
   keychain()->set_locked(true);
   ASSERT_TRUE(base::PostTaskAndReplyWithResult(
-      thread_->task_runner().get(), FROM_HERE,
+      delegate_.thread()->task_runner().get(), FROM_HERE,
       base::Bind(&PasswordStoreMac::ImportFromKeychain, store()),
       base::Bind(&CheckMigrationResult, PasswordStoreMac::KEYCHAIN_BLOCKED)));
-  FinishAsyncProcessing();
+  delegate_.FinishAsyncProcessing();
 
   ScopedVector<PasswordForm> matching_items;
   EXPECT_TRUE(login_db()->GetLogins(form1, &matching_items));
   ASSERT_EQ(1u, matching_items.size());
   EXPECT_EQ(base::string16(), matching_items[0]->password_value);
 
   histogram_tester_->ExpectUniqueSample(
       "PasswordManager.KeychainMigration.NumPasswordsOnFailure", 1, 1);
   histogram_tester_->ExpectUniqueSample(
       "PasswordManager.KeychainMigration.NumFailedPasswords", 1, 1);
   histogram_tester_->ExpectUniqueSample(
       "PasswordManager.KeychainMigration.NumChromeOwnedInaccessiblePasswords",
       2, 1);
   // Don't test the encryption key access.
   histogram_tester_.reset();
 }