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_consumer.h"
+#include "components/password_manager/core/browser/password_store_origin_unittest.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;
 using content::BrowserThread;
@@ skipping 35 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 72 matching lines @@
         form->date_synced);
     EXPECT_EQ(GURL(password_manager::kTestingIconUrlSpec), form->icon_url);
   }
 }
 
 PasswordStoreChangeList AddChangeForForm(const PasswordForm& form) {
   return PasswordStoreChangeList(
       1, PasswordStoreChange(PasswordStoreChange::ADD, form));
 }
 
+class PasswordStoreMacTestDelegate {
+ public:
+  PasswordStoreMacTestDelegate();
+  ~PasswordStoreMacTestDelegate();
+
+  PasswordStoreMac* store() { return store_.get(); }
+
+  base::Thread* thread() { return thread_.get(); }
+
+  void FinishAsyncProcessing();
+
+ private:
+  static void InitLoginDatabase(LoginDatabase* login_db);
+  void Initialize();
+  base::FilePath test_login_db_file_path() const;
+
+  void ClosePasswordStore();
+
+  base::MessageLoopForUI message_loop_;
+  content::TestBrowserThread ui_thread_;

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

It's deprecated. Use TestBrowserThreadBundle. It can also be used to get rid of
|thread_ and |message_loop_|

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

Okay, done. Apart from adding TestBrowserThreadBundle, I also replaced all
references to threads by ThreadTaskRunnerHandle. Please have a look and let me
know if the approach is correct.

[vasilii, 2015/11/23 15:04:56]

In the implementation you made the test single-threaded because you don't set up
a real background thread while constructing TestBrowserThreadBundle. As I
already said I'm fine with that if you don't touch PasswordStoreMacTest.
Otherwise, you should set up a second thread.
FYI, this class is deprecated and will be ultimately removed.

[Timo Reimann, 2015/11/24 02:36:09]

This part was and still is somewhat unclear to me: As I needed to refactor
PasswordStoreMacTest in order to break out PasswordStoreMacTestDelegate, it
seems impossible to keep the former completely unchanged. So as I understand, I
have no other option but to set up a second thread if I want to stick to the
delegate (which is a requirement for the type-parameterized tests in
PasswordStoreOriginTest).

What I did now was enable the IO_MAINLOOP option on the TestBrowserThreadBundle
instance and post all tasks to the IO thread. (I tried doing the same on the
REAL_DB_THREAD but that seg-faulted miserably.) This was my best guess on how to
use TestBrowserThreadBundle in order to create a separate thread. Please let me
know if this is the right way or not.

Alternatively, will the deprecation of this class simplify things somehow? My
understanding is that SimplePasswordStoreMac (which is implemented in terms of
PasswordStoreDefault) will take over in the future. Does this mean we could
possibly dump this CL altogether (assuming my feature won't need to ship before
PasswordStoreMac's deprecation)?

(In case the latter is true, please still let me know how to create a second
thread properly . I'd like to understand how things work.)

[vasilii, 2015/11/24 18:14:54]

You don't have to refactor PasswordStoreMacTest. You could just write the
delegate and instantiate your tests. That's it.
If you want to refactor then you need to create a TestBrowserThreadBundle with
REAL_FILE_THREAD (for example) flag. You can post to that thread
BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, callback); or you can
get a task runner GetMessageLoopProxyForThread(BrowserThread::FILE).
We don't know when the class will be deleted. It's not happening soon.  Thus we
need to test your feature. Ideally without putting a lot of efforts. That's why
I prefer to repeat basically what you did for PasswordStoreDefault.

[Timo Reimann, 2015/11/24 23:06:02]

Without any refactoring permitted, wouldn't that prevent usage of the Mac
delegate as a typed test in PasswordStoreOriginTest? As of now, all
parameterized test delegates need to adhere to a certain interface and
encapsulate the store life cycle. For instance, they must support a
FinishAsyncProcessing() method. I couldn't provide that for
PasswordStoreMacTestDelegate if I wasn't allowed to move (refactor) it from
PasswordStoreMacTest into the delegate.
I'm using REAL_FILE_THREAD now. I think it's not the worst choice after all as
we've made a few improvements to PasswordStoreMacTest along the way. These might
come handy even if the class will be removed at some remote point in the future.

[vasilii, 2015/11/25 18:18:39]

I don't understand your concern. You have PasswordStoreMacTest which works fine.
Then you wrote PasswordStoreOriginTest and PasswordStoreMacTestDelegate as a
delegate. Why do you feel like you have to change PasswordStoreMacTest? You
don't have to move, you can copy what you need.
I am suggesting the single-threaded PasswordStoreMacTestDelegate because of
simplicity. The current patch set contains quite a few mistakes related to
multi-threading.

[Timo Reimann, 2015/11/25 18:47:32]

I believe I'm starting to understand where you're heading: Create the delegate
and re-implement/copy whatever functionality is necessary to support store
management. I didn't consider this because it will create a bit of duplicated
code which I tried to avoid (such as the Initialize, Terminate, and
FinishAsyncProcessing methods). Is this what you mean?

[Timo Reimann, 2015/11/26 01:12:51]

I followed your advice and build a simplified PasswordStoreMacDelegate. Turned
out it contains much less duplicate code than I thought due to its
single-threaded nature -- apologies for not realizing this earlier. The
multi-threading-related problems you addressed in the other comments should all
be obsolete now.

The changes to PasswordStoreMacTest have been reverted (except for the minor
MockPasswordStoreObserver update). Let me know what you think.

+  // 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_;
+
+  DISALLOW_COPY_AND_ASSIGN(PasswordStoreMacTestDelegate);
+};
+
+PasswordStoreMacTestDelegate::PasswordStoreMacTestDelegate()
+    : ui_thread_(BrowserThread::UI, &message_loop_) {
+  Initialize();
+}
+
+PasswordStoreMacTestDelegate::~PasswordStoreMacTestDelegate() {
+  ClosePasswordStore();
+  thread_.reset();
+  login_db_.reset();
+}
+
+void PasswordStoreMacTestDelegate::Initialize() {
+  ASSERT_TRUE(db_dir_.CreateUniqueTempDir());
+
+  // Ensure that LoginDatabase will use the mock keychain if it needs to
+  // encrypt/decrypt a password.
+  OSCrypt::UseMockKeychain(true);
+  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()))));
+
+  // Creat and initialize the password store.
+  store_ = new PasswordStoreMac(
+      base::ThreadTaskRunnerHandle::Get(), nullptr,
+      make_scoped_ptr<AppleKeychain>(new MockAppleKeychain));

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

I think it should work without the type specifier.

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

It does. Removed.

+  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_.get()))));
+
+  // Make sure deferred initialization is performed before some tests start
+  // accessing the |login_db| directly.
+  FinishAsyncProcessing();
+}
+
+void PasswordStoreMacTestDelegate::FinishAsyncProcessing() {
+  scoped_refptr<content::MessageLoopRunner> runner =
+      new content::MessageLoopRunner;
+  ASSERT_TRUE(thread_->task_runner()->PostTaskAndReply(
+      FROM_HERE, base::Bind(&Noop), runner->QuitClosure()));
+  runner->Run();
+}
+

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

// static

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

Done.

+void PasswordStoreMacTestDelegate::InitLoginDatabase(LoginDatabase* login_db) {
+  ASSERT_TRUE(login_db->Init());
+}
+
+base::FilePath PasswordStoreMacTestDelegate::test_login_db_file_path() const {
+  return db_dir_.path().Append(FILE_PATH_LITERAL("login.db"));
+}
+
+void PasswordStoreMacTestDelegate::ClosePasswordStore() {
+  if (!store_)
+    return;
+
+  store_->Shutdown();
+  FinishAsyncProcessing();
+  store_ = nullptr;
+}
+
 }  // namespace
 
+namespace password_manager {
+
+INSTANTIATE_TYPED_TEST_CASE_P(Mac,
+                              PasswordStoreOriginTest,
+                              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()) {}
 
-  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_; }
 
-  PasswordStoreMac* store() { return store_.get(); }
+  PasswordStoreMacTestDelegate& delegate() { return delegate_; }
 
  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_;
+  PasswordStoreMacTestDelegate delegate_;
+  PasswordStoreMac* store_;

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

Remove

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

Done (and replaced all references to store_ by delegate_.store()).

I also removed |store_| and store() from PasswordStoreMacTest and redirected
calls through delegate_. However, I had to add a const overload for
PasswordStoreMacTestDelegate.store() in order to satisfy login_db()'s constness.

 };
 
 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) {
+ public:
+  explicit 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:
+ private:
   ScopedObserver<password_manager::PasswordStore,
                 PasswordsChangeObserver> observer_;
 };
 
 password_manager::PasswordStoreChangeList GetAddChangeList(
     const PasswordForm& form) {
   password_manager::PasswordStoreChange change(
       password_manager::PasswordStoreChange::ADD, form);
   return password_manager::PasswordStoreChangeList(1, change);
 }
@@ skipping 127 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();
 }