From 19209e174a4249faac3f6ee9051399f5b8639515 Mon Sep 17 00:00:00 2001 From: vasilii Date: Mon, 22 Jun 2015 08:01:24 -0700 Subject: [PATCH] Implement PasswordStoreProxyMac and SimplePasswordStoreMac. They aren't instantiated yet in the code base. SimplePasswordStoreMac is a PasswordStore implementation on Mac in the future. PasswordStoreProxyMac is a proxy used for migration from PasswordStoreMac to SimplePasswordStoreMac. BUG=466638 Review URL: https://codereview.chromium.org/1192963002 Cr-Commit-Position: refs/heads/master@{#335502} --- .../password_manager/password_store_proxy_mac.cc | 118 +++++++++++++++++++++ .../password_manager/password_store_proxy_mac.h | 80 ++++++++++++++ .../password_manager/simple_password_store_mac.cc | 27 +++++ .../password_manager/simple_password_store_mac.h | 35 ++++++ .../simple_password_store_mac_unittest.cc | 113 ++++++++++++++++++++ chrome/chrome_browser.gypi | 4 + chrome/chrome_tests_unit.gypi | 1 + .../password_manager/core/browser/password_store.h | 4 + .../core/browser/password_store_default.cc | 12 ++- .../core/browser/password_store_default.h | 5 + 10 files changed, 397 insertions(+), 2 deletions(-) create mode 100644 chrome/browser/password_manager/password_store_proxy_mac.cc create mode 100644 chrome/browser/password_manager/password_store_proxy_mac.h create mode 100644 chrome/browser/password_manager/simple_password_store_mac.cc create mode 100644 chrome/browser/password_manager/simple_password_store_mac.h create mode 100644 chrome/browser/password_manager/simple_password_store_mac_unittest.cc diff --git a/chrome/browser/password_manager/password_store_proxy_mac.cc b/chrome/browser/password_manager/password_store_proxy_mac.cc new file mode 100644 index 000000000000..8e7bd5b7fb87 --- /dev/null +++ b/chrome/browser/password_manager/password_store_proxy_mac.cc @@ -0,0 +1,118 @@ +// Copyright 2015 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_proxy_mac.h" + +#include "chrome/browser/password_manager/password_store_mac.h" +#include "chrome/browser/password_manager/simple_password_store_mac.h" +#include "crypto/apple_keychain.h" + +using password_manager::PasswordStoreChangeList; + +PasswordStoreProxyMac::PasswordStoreProxyMac( + scoped_refptr main_thread_runner, + scoped_ptr keychain, + scoped_ptr login_db) + : PasswordStore(main_thread_runner, nullptr) { + // TODO(vasilii): for now the class is just a wrapper around PasswordStoreMac. + password_store_mac_ = new PasswordStoreMac(main_thread_runner, nullptr, + keychain.Pass(), login_db.Pass()); +} + +PasswordStoreProxyMac::~PasswordStoreProxyMac() { +} + +bool PasswordStoreProxyMac::Init( + const syncer::SyncableService::StartSyncFlare& flare) { + return GetBackend()->Init(flare); +} + +void PasswordStoreProxyMac::Shutdown() { + return GetBackend()->Shutdown(); +} + +password_manager::PasswordStore* PasswordStoreProxyMac::GetBackend() const { + if (password_store_mac_) + return password_store_mac_.get(); + return password_store_simple_.get(); +} + +scoped_refptr +PasswordStoreProxyMac::GetBackgroundTaskRunner() { + return GetBackend()->GetBackgroundTaskRunner(); +} + +void PasswordStoreProxyMac::ReportMetricsImpl( + const std::string& sync_username, + bool custom_passphrase_sync_enabled) { + GetBackend()->ReportMetricsImpl(sync_username, + custom_passphrase_sync_enabled); +} + +PasswordStoreChangeList PasswordStoreProxyMac::AddLoginImpl( + const autofill::PasswordForm& form) { + return GetBackend()->AddLoginImpl(form); +} + +PasswordStoreChangeList PasswordStoreProxyMac::UpdateLoginImpl( + const autofill::PasswordForm& form) { + return GetBackend()->UpdateLoginImpl(form); +} + +PasswordStoreChangeList PasswordStoreProxyMac::RemoveLoginImpl( + const autofill::PasswordForm& form) { + return GetBackend()->RemoveLoginImpl(form); +} + +PasswordStoreChangeList PasswordStoreProxyMac::RemoveLoginsCreatedBetweenImpl( + base::Time delete_begin, + base::Time delete_end) { + return GetBackend()->RemoveLoginsCreatedBetweenImpl(delete_begin, delete_end); +} + +PasswordStoreChangeList PasswordStoreProxyMac::RemoveLoginsSyncedBetweenImpl( + base::Time delete_begin, + base::Time delete_end) { + return GetBackend()->RemoveLoginsSyncedBetweenImpl(delete_begin, delete_end); +} + +ScopedVector PasswordStoreProxyMac::FillMatchingLogins( + const autofill::PasswordForm& form, + AuthorizationPromptPolicy prompt_policy) { + return GetBackend()->FillMatchingLogins(form, prompt_policy); +} + +void PasswordStoreProxyMac::GetAutofillableLoginsImpl( + scoped_ptr request) { + GetBackend()->GetAutofillableLoginsImpl(request.Pass()); +} + +void PasswordStoreProxyMac::GetBlacklistLoginsImpl( + scoped_ptr request) { + GetBackend()->GetBlacklistLoginsImpl(request.Pass()); +} + +bool PasswordStoreProxyMac::FillAutofillableLogins( + ScopedVector* forms) { + return GetBackend()->FillAutofillableLogins(forms); +} + +bool PasswordStoreProxyMac::FillBlacklistLogins( + ScopedVector* forms) { + return GetBackend()->FillBlacklistLogins(forms); +} + +void PasswordStoreProxyMac::AddSiteStatsImpl( + const password_manager::InteractionsStats& stats) { + GetBackend()->AddSiteStatsImpl(stats); +} + +void PasswordStoreProxyMac::RemoveSiteStatsImpl(const GURL& origin_domain) { + GetBackend()->RemoveSiteStatsImpl(origin_domain); +} + +scoped_ptr +PasswordStoreProxyMac::GetSiteStatsImpl(const GURL& origin_domain) { + return GetBackend()->GetSiteStatsImpl(origin_domain); +} diff --git a/chrome/browser/password_manager/password_store_proxy_mac.h b/chrome/browser/password_manager/password_store_proxy_mac.h new file mode 100644 index 000000000000..8e1020915a80 --- /dev/null +++ b/chrome/browser/password_manager/password_store_proxy_mac.h @@ -0,0 +1,80 @@ +// Copyright 2015 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. + +#ifndef CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_PROXY_MAC_H_ +#define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_PROXY_MAC_H_ + +#include "components/password_manager/core/browser/password_store.h" + +namespace crypto { +class AppleKeychain; +} + +namespace password_manager { +class LoginDatabase; +} + +class PasswordStoreMac; +class SimplePasswordStoreMac; + +// The class is a proxy for either PasswordStoreMac or SimplePasswordStoreMac. +// It is responsible for performing migration from PasswordStoreMac to +// SimplePasswordStoreMac and instantiating a correct backend according to the +// user's state. +class PasswordStoreProxyMac : public password_manager::PasswordStore { + public: + PasswordStoreProxyMac( + scoped_refptr main_thread_runner, + scoped_ptr keychain, + scoped_ptr login_db); + + bool Init(const syncer::SyncableService::StartSyncFlare& flare) override; + void Shutdown() override; + + private: + ~PasswordStoreProxyMac() override; + + password_manager::PasswordStore* GetBackend() const; + + // PasswordStore: + scoped_refptr GetBackgroundTaskRunner() + override; + void ReportMetricsImpl(const std::string& sync_username, + bool custom_passphrase_sync_enabled) override; + password_manager::PasswordStoreChangeList AddLoginImpl( + const autofill::PasswordForm& form) override; + password_manager::PasswordStoreChangeList UpdateLoginImpl( + const autofill::PasswordForm& form) override; + password_manager::PasswordStoreChangeList RemoveLoginImpl( + const autofill::PasswordForm& form) override; + password_manager::PasswordStoreChangeList RemoveLoginsCreatedBetweenImpl( + base::Time delete_begin, + base::Time delete_end) override; + password_manager::PasswordStoreChangeList RemoveLoginsSyncedBetweenImpl( + base::Time delete_begin, + base::Time delete_end) override; + ScopedVector FillMatchingLogins( + const autofill::PasswordForm& form, + AuthorizationPromptPolicy prompt_policy) override; + void GetAutofillableLoginsImpl( + scoped_ptr request) override; + void GetBlacklistLoginsImpl( + scoped_ptr request) override; + bool FillAutofillableLogins( + ScopedVector* forms) override; + bool FillBlacklistLogins( + ScopedVector* forms) override; + void AddSiteStatsImpl( + const password_manager::InteractionsStats& stats) override; + void RemoveSiteStatsImpl(const GURL& origin_domain) override; + scoped_ptr GetSiteStatsImpl( + const GURL& origin_domain) override; + + scoped_refptr password_store_mac_; + scoped_refptr password_store_simple_; + + DISALLOW_COPY_AND_ASSIGN(PasswordStoreProxyMac); +}; + +#endif // CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_PROXY_MAC_H_ diff --git a/chrome/browser/password_manager/simple_password_store_mac.cc b/chrome/browser/password_manager/simple_password_store_mac.cc new file mode 100644 index 000000000000..6c60f984a67d --- /dev/null +++ b/chrome/browser/password_manager/simple_password_store_mac.cc @@ -0,0 +1,27 @@ +// Copyright 2015 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/simple_password_store_mac.h" + +SimplePasswordStoreMac::SimplePasswordStoreMac( + scoped_refptr main_thread_runner, + scoped_refptr background_thread_runner, + scoped_ptr login_db) + : PasswordStoreDefault(main_thread_runner, background_thread_runner, + login_db.Pass()) { + this->login_db()->set_clear_password_values(false); +} + +SimplePasswordStoreMac::~SimplePasswordStoreMac() { +} + +bool SimplePasswordStoreMac::Init( + const syncer::SyncableService::StartSyncFlare& flare) { + // All the initialization has to be done by the owner of the object. + return true; +} + +void SimplePasswordStoreMac::Shutdown() { + PasswordStoreDefault::Shutdown(); +} diff --git a/chrome/browser/password_manager/simple_password_store_mac.h b/chrome/browser/password_manager/simple_password_store_mac.h new file mode 100644 index 000000000000..3e046b9ea436 --- /dev/null +++ b/chrome/browser/password_manager/simple_password_store_mac.h @@ -0,0 +1,35 @@ +// Copyright 2015 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. + +#ifndef CHROME_BROWSER_PASSWORD_MANAGER_SIMPLE_PASSWORD_STORE_MAC_H_ +#define CHROME_BROWSER_PASSWORD_MANAGER_SIMPLE_PASSWORD_STORE_MAC_H_ + +#include "components/password_manager/core/browser/password_store_default.h" + +// The same as PasswordStoreDefault but running on the dedicated thread. The +// owner is responsible for the thread lifetime. +class SimplePasswordStoreMac : public password_manager::PasswordStoreDefault { + public: + SimplePasswordStoreMac( + scoped_refptr main_thread_runner, + scoped_refptr background_thread_runner, + scoped_ptr login_db); + + // Just hides the parent method. All the initialization is to be done by + // PasswordStoreProxyMac that is an owner of the class. + bool Init(const syncer::SyncableService::StartSyncFlare& flare) override; + + // Clean |background_thread_runner_|. + void Shutdown() override; + + using PasswordStoreDefault::GetBackgroundTaskRunner; + + protected: + ~SimplePasswordStoreMac() override; + + private: + DISALLOW_COPY_AND_ASSIGN(SimplePasswordStoreMac); +}; + +#endif // CHROME_BROWSER_PASSWORD_MANAGER_SIMPLE_PASSWORD_STORE_MAC_H_ diff --git a/chrome/browser/password_manager/simple_password_store_mac_unittest.cc b/chrome/browser/password_manager/simple_password_store_mac_unittest.cc new file mode 100644 index 000000000000..517f5ce5796d --- /dev/null +++ b/chrome/browser/password_manager/simple_password_store_mac_unittest.cc @@ -0,0 +1,113 @@ +// Copyright 2015 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/simple_password_store_mac.h" + +#include "base/callback.h" +#include "base/files/scoped_temp_dir.h" +#include "base/strings/utf_string_conversions.h" +#include "components/os_crypt/os_crypt.h" +#include "components/password_manager/core/browser/login_database.h" +#include "content/public/browser/browser_thread.h" +#include "content/public/test/test_browser_thread_bundle.h" +#include "content/public/test/test_utils.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +using content::BrowserThread; +using password_manager::PasswordStoreChange; +using testing::ElementsAreArray; + +namespace { + +class MockPasswordStoreObserver + : public password_manager::PasswordStore::Observer { + public: + MOCK_METHOD1(OnLoginsChanged, + void(const password_manager::PasswordStoreChangeList& changes)); +}; + +class SimplePasswordStoreMacTest : public testing::Test { + public: + SimplePasswordStoreMacTest() + : thread_bundle_(content::TestBrowserThreadBundle::REAL_FILE_THREAD) {} + + void SetUp() override { + ASSERT_TRUE(db_dir_.CreateUniqueTempDir()); + + // Ensure that LoginDatabase will use the mock keychain if it needs to + // encrypt/decrypt a password. + OSCrypt::UseMockKeychain(true); + + // Setup LoginDatabase. + scoped_ptr login_db( + new password_manager::LoginDatabase(test_login_db_file_path())); + scoped_refptr file_task_runner = + BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE); + ASSERT_TRUE(file_task_runner); + file_task_runner->PostTask( + FROM_HERE, + base::Bind(&InitOnBackgroundThread, base::Unretained(login_db.get()))); + store_ = new SimplePasswordStoreMac( + BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI), + file_task_runner, login_db.Pass()); + ASSERT_TRUE(store_->Init(syncer::SyncableService::StartSyncFlare())); + // Make sure deferred initialization is finished. + FinishAsyncProcessing(); + } + + void TearDown() override { + store_->Shutdown(); + EXPECT_TRUE(store_->GetBackgroundTaskRunner()); + store_ = nullptr; + } + + base::FilePath test_login_db_file_path() const { + return db_dir_.path().Append(FILE_PATH_LITERAL("login.db")); + } + + scoped_refptr store() { return store_; } + + void FinishAsyncProcessing() { + scoped_refptr runner( + new content::MessageLoopRunner); + ASSERT_TRUE(BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE, + base::Bind(&Noop), + runner->QuitClosure())); + runner->Run(); + } + + private: + static void InitOnBackgroundThread(password_manager::LoginDatabase* db) { + ASSERT_TRUE(db->Init()); + } + + static void Noop() {} + + base::ScopedTempDir db_dir_; + scoped_refptr store_; + + content::TestBrowserThreadBundle thread_bundle_; +}; + +TEST_F(SimplePasswordStoreMacTest, Sanity) { + autofill::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"); + MockPasswordStoreObserver observer; + store()->AddObserver(&observer); + + const PasswordStoreChange expected_add_changes[] = { + PasswordStoreChange(PasswordStoreChange::ADD, form), + }; + EXPECT_CALL(observer, + OnLoginsChanged(ElementsAreArray(expected_add_changes))); + // Adding a login should trigger a notification. + store()->AddLogin(form); + FinishAsyncProcessing(); +} + +} // namespace diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 293ca8946d3f..2f2ebe90aa00 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -2019,12 +2019,16 @@ 'browser/password_manager/password_store_mac.cc', 'browser/password_manager/password_store_mac.h', 'browser/password_manager/password_store_mac_internal.h', + 'browser/password_manager/password_store_proxy_mac.cc', + 'browser/password_manager/password_store_proxy_mac.h', 'browser/password_manager/password_store_win.cc', 'browser/password_manager/password_store_win.h', 'browser/password_manager/password_store_x.cc', 'browser/password_manager/password_store_x.h', 'browser/password_manager/save_password_infobar_delegate.cc', 'browser/password_manager/save_password_infobar_delegate.h', + 'browser/password_manager/simple_password_store_mac.cc', + 'browser/password_manager/simple_password_store_mac.h', 'browser/password_manager/sync_metrics.cc', 'browser/password_manager/sync_metrics.h', ], diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index c8ed6ee00935..777bd1a91e19 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -163,6 +163,7 @@ 'browser/password_manager/password_store_win_unittest.cc', 'browser/password_manager/password_store_x_unittest.cc', 'browser/password_manager/save_password_infobar_delegate_unittest.cc', + 'browser/password_manager/simple_password_store_mac_unittest.cc', 'browser/permissions/permission_manager_unittest.cc', 'browser/predictors/autocomplete_action_predictor_table_unittest.cc', 'browser/predictors/autocomplete_action_predictor_unittest.cc', diff --git a/components/password_manager/core/browser/password_store.h b/components/password_manager/core/browser/password_store.h index 91ec3b5bd197..d875a51baeb6 100644 --- a/components/password_manager/core/browser/password_store.h +++ b/components/password_manager/core/browser/password_store.h @@ -26,6 +26,8 @@ namespace syncer { class SyncableService; } +class PasswordStoreProxyMac; + namespace password_manager { class AffiliatedMatchHelper; @@ -290,6 +292,8 @@ class PasswordStore : protected PasswordStoreSync, FRIEND_TEST_ALL_PREFIXES(PasswordStoreTest, GetLoginImpl); FRIEND_TEST_ALL_PREFIXES(PasswordStoreTest, UpdatePasswordsStoredForAffiliatedWebsites); + // TODO(vasilii): remove this together with PasswordStoreProxyMac. + friend class ::PasswordStoreProxyMac; // Schedule the given |func| to be run in the PasswordStore's own thread with // responses delivered to |consumer| on the current thread. diff --git a/components/password_manager/core/browser/password_store_default.cc b/components/password_manager/core/browser/password_store_default.cc index 40b070e32123..4f305909444f 100644 --- a/components/password_manager/core/browser/password_store_default.cc +++ b/components/password_manager/core/browser/password_store_default.cc @@ -24,8 +24,6 @@ PasswordStoreDefault::PasswordStoreDefault( } PasswordStoreDefault::~PasswordStoreDefault() { - if (!GetBackgroundTaskRunner()->BelongsToCurrentThread()) - GetBackgroundTaskRunner()->DeleteSoon(FROM_HERE, login_db_.release()); } bool PasswordStoreDefault::Init( @@ -34,6 +32,11 @@ bool PasswordStoreDefault::Init( return PasswordStore::Init(flare); } +void PasswordStoreDefault::Shutdown() { + PasswordStore::Shutdown(); + ScheduleTask(base::Bind(&PasswordStoreDefault::ResetLoginDB, this)); +} + void PasswordStoreDefault::InitOnDBThread() { DCHECK(GetBackgroundTaskRunner()->BelongsToCurrentThread()); DCHECK(login_db_); @@ -169,4 +172,9 @@ scoped_ptr PasswordStoreDefault::GetSiteStatsImpl( : scoped_ptr(); } +void PasswordStoreDefault::ResetLoginDB() { + DCHECK(GetBackgroundTaskRunner()->BelongsToCurrentThread()); + login_db_.reset(); +} + } // namespace password_manager diff --git a/components/password_manager/core/browser/password_store_default.h b/components/password_manager/core/browser/password_store_default.h index ab020048141f..95966f8e9aae 100644 --- a/components/password_manager/core/browser/password_store_default.h +++ b/components/password_manager/core/browser/password_store_default.h @@ -26,6 +26,8 @@ class PasswordStoreDefault : public PasswordStore { bool Init(const syncer::SyncableService::StartSyncFlare& flare) override; + void Shutdown() override; + // To be used only for testing. LoginDatabase* login_db() const { return login_db_.get(); } @@ -69,6 +71,9 @@ class PasswordStoreDefault : public PasswordStore { } private: + // Resets |login_db_| on the background thread. + void ResetLoginDB(); + // The login SQL database. The LoginDatabase instance is received via the // in an uninitialized state, so as to allow injecting mocks, then Init() is // called on the DB thread in a deferred manner. If opening the DB fails, -- 2.11.4.GIT