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 <string>
 
 #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(); }
+
+  static void FinishAsyncProcessing();
+
+ private:
+  void Initialize();
+
+  void ClosePasswordStore();
+
+  base::FilePath test_login_db_file_path() const;
+
+  base::MessageLoopForUI message_loop_;
+  base::ScopedTempDir db_dir_;
+  scoped_ptr<LoginDatabase> login_db_;
+  scoped_refptr<PasswordStoreMac> store_;
+
+  DISALLOW_COPY_AND_ASSIGN(PasswordStoreMacTestDelegate);
+};
+
+PasswordStoreMacTestDelegate::PasswordStoreMacTestDelegate() {
+  Initialize();
+}
+
+PasswordStoreMacTestDelegate::~PasswordStoreMacTestDelegate() {
+  ClosePasswordStore();
+  login_db_.reset();

[vasilii, 2015/11/26 11:03:38]

You don't need it. Also I think that |login_db_| should outlive |store_|. Thus,
the default destruction order is fine.

[Timo Reimann, 2015/11/26 14:38:29]

Removed.

+}
+
+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()));
+  ASSERT_TRUE(login_db_->Init());
+
+  // Create and initialize the password store.
+  store_ = new PasswordStoreMac(base::ThreadTaskRunnerHandle::Get(),
+                                base::ThreadTaskRunnerHandle::Get(),
+                                make_scoped_ptr(new MockAppleKeychain));
+  store_->set_login_metadata_db(login_db_.get());
+}
+
+base::FilePath PasswordStoreMacTestDelegate::test_login_db_file_path() const {

[vasilii, 2015/11/26 11:03:38]

Declaration order should match definition order.

[Timo Reimann, 2015/11/26 14:38:29]

Updated.

+  return db_dir_.path().Append(FILE_PATH_LITERAL("login.db"));
+}
+
+void PasswordStoreMacTestDelegate::ClosePasswordStore() {
+  store_->ShutdownOnUIThread();
+  FinishAsyncProcessing();
+}
+
+void PasswordStoreMacTestDelegate::FinishAsyncProcessing() {
+  base::MessageLoop::current()->RunUntilIdle();
+}
+
 }  // namespace
 
+namespace password_manager {

[vasilii, 2015/11/26 11:03:38]

We shouldn't declare namespace password_manager in chrome/. 

[Timo Reimann, 2015/11/26 14:38:29]

I tried to keep the typed test case instantiations out of the password_manager
namespace before already but kept running into Google Test-related compiler
errors I couldn't resolve: Trying without gives

../../chrome/browser/password_manager/password_store_mac_unittest.cc:236:31: use
of undeclared identifier 'PasswordStoreOriginTest'; did you mean
'password_manager::PasswordStoreOriginTest'?

and fully qualifying as suggested leads to

BEGIN-OF-COMPILER-OUTPUT
../../chrome/browser/password_manager/password_store_mac_unittest.cc:235:1:
error: use of undeclared identifier 'gtest_Mac_password_manager'
INSTANTIATE_TYPED_TEST_CASE_P(Mac,
^
../../testing/gtest/include/gtest/gtest-typed-test.h:252:8: note: expanded from
macro 'INSTANTIATE_TYPED_TEST_CASE_P'
  bool gtest_##Prefix##_##CaseName GTEST_ATTRIBUTE_UNUSED_ = \
       ^
<scratch space>:102:1: note: expanded from here
gtest_Mac_password_manager
END-OF-COMPILER-OUTPUT

Any idea how to resolve this?

[vasilii, 2015/11/26 14:53:39]

Try to write 'using password_manager::PasswordStoreOriginTest;' and then use the
unqualified class name.

+
+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 1480 matching lines @@
   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 166 matching lines @@
   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();
 }