Delete PasswordStoreMac.
The class is replaced by the common PasswordStoreDefault.
All the corresponding histograms are also deleted.
Bug: 975634,466638
Change-Id: Ie50e748f785da3eb32ba3b7d0d66aa45d9fa4259
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1670226
Commit-Queue: Vasilii Sukhanov <[email protected]>
Reviewed-by: Dominic Battré <[email protected]>
Reviewed-by: Alexei Svitkine <[email protected]>
Cr-Commit-Position: refs/heads/master@{#671637}
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index acc20bb..12a840e7 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -1014,8 +1014,6 @@
"password_manager/password_manager_util_win.h",
"password_manager/password_store_factory.cc",
"password_manager/password_store_factory.h",
- "password_manager/password_store_mac.cc",
- "password_manager/password_store_mac.h",
"password_manager/reauth_purpose.h",
"payments/payment_handler_permission_context.cc",
"payments/payment_handler_permission_context.h",
diff --git a/chrome/browser/password_manager/password_store_factory.cc b/chrome/browser/password_manager/password_store_factory.cc
index cd1d05d4..e509b888 100644
--- a/chrome/browser/password_manager/password_store_factory.cc
+++ b/chrome/browser/password_manager/password_store_factory.cc
@@ -44,7 +44,7 @@
#if defined(OS_WIN)
#include "chrome/browser/password_manager/password_manager_util_win.h"
#elif defined(OS_MACOSX)
-#include "chrome/browser/password_manager/password_store_mac.h"
+// Use default store.
#elif defined(OS_CHROMEOS) || defined(OS_ANDROID)
// Don't do anything. We're going to use the default store.
#elif defined(USE_X11)
@@ -178,9 +178,7 @@
scoped_refptr<PasswordStore> ps;
#if defined(OS_WIN)
ps = new password_manager::PasswordStoreDefault(std::move(login_db));
-#elif defined(OS_MACOSX)
- ps = new PasswordStoreMac(std::move(login_db), profile->GetPrefs());
-#elif defined(OS_CHROMEOS) || defined(OS_ANDROID)
+#elif defined(OS_CHROMEOS) || defined(OS_ANDROID) || defined(OS_MACOSX)
// For now, we use PasswordStoreDefault. We might want to make a native
// backend for PasswordStoreX (see below) in the future though.
ps = new password_manager::PasswordStoreDefault(std::move(login_db));
diff --git a/chrome/browser/password_manager/password_store_mac.cc b/chrome/browser/password_manager/password_store_mac.cc
deleted file mode 100644
index effbb927..0000000
--- a/chrome/browser/password_manager/password_store_mac.cc
+++ /dev/null
@@ -1,71 +0,0 @@
-// 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_mac.h"
-#include "base/bind.h"
-#include "base/metrics/histogram_macros.h"
-#include "components/os_crypt/os_crypt.h"
-#include "components/password_manager/core/common/password_manager_pref_names.h"
-
-using password_manager::MigrationStatus;
-
-PasswordStoreMac::PasswordStoreMac(
- std::unique_ptr<password_manager::LoginDatabase> login_db,
- PrefService* prefs)
- : PasswordStoreDefault(std::move(login_db)),
- initial_status_(MigrationStatus::NOT_STARTED) {
- migration_status_.Init(password_manager::prefs::kKeychainMigrationStatus,
- prefs);
-}
-
-bool PasswordStoreMac::Init(
- const syncer::SyncableService::StartSyncFlare& flare,
- PrefService* prefs) {
- initial_status_ = static_cast<MigrationStatus>(migration_status_.GetValue());
- return PasswordStoreDefault::Init(flare, prefs);
-}
-
-void PasswordStoreMac::ShutdownOnUIThread() {
- PasswordStoreDefault::ShutdownOnUIThread();
-
- // Unsubscribe the observer, otherwise it's too late in the destructor.
- migration_status_.Destroy();
-}
-
-PasswordStoreMac::~PasswordStoreMac() = default;
-
-bool PasswordStoreMac::InitOnBackgroundSequence(
- const syncer::SyncableService::StartSyncFlare& flare) {
- if (!PasswordStoreDefault::InitOnBackgroundSequence(flare))
- return false;
-
- if (!OSCrypt::IsEncryptionAvailable())
- return false;
-
- if (login_db() && (initial_status_ == MigrationStatus::NOT_STARTED ||
- initial_status_ == MigrationStatus::FAILED_ONCE ||
- initial_status_ == MigrationStatus::FAILED_TWICE)) {
- // Migration isn't possible due to Chrome changing the certificate. Just
- // drop the entries in the DB because they don't have passwords anyway.
- login_db()->RemoveLoginsCreatedBetween(base::Time(), base::Time(),
- /*changes=*/nullptr);
- initial_status_ = MigrationStatus::MIGRATION_STOPPED;
- main_task_runner()->PostTask(
- FROM_HERE, base::BindOnce(&PasswordStoreMac::UpdateStatusPref, this,
- initial_status_));
- }
-
- UMA_HISTOGRAM_ENUMERATION(
- "PasswordManager.KeychainMigration.Status",
- static_cast<int>(initial_status_),
- static_cast<int>(MigrationStatus::MIGRATION_STATUS_COUNT));
-
- return true;
-}
-
-void PasswordStoreMac::UpdateStatusPref(MigrationStatus status) {
- // The method can be called after ShutdownOnUIThread().
- if (migration_status_.prefs())
- migration_status_.SetValue(static_cast<int>(status));
-}
diff --git a/chrome/browser/password_manager/password_store_mac.h b/chrome/browser/password_manager/password_store_mac.h
deleted file mode 100644
index 3c7431e..0000000
--- a/chrome/browser/password_manager/password_store_mac.h
+++ /dev/null
@@ -1,54 +0,0 @@
-// 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_MAC_H_
-#define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_MAC_H_
-
-#include <memory>
-
-#include "base/macros.h"
-#include "components/password_manager/core/browser/keychain_migration_status_mac.h"
-#include "components/password_manager/core/browser/password_store.h"
-#include "components/password_manager/core/browser/password_store_default.h"
-#include "components/prefs/pref_member.h"
-
-namespace password_manager {
-class LoginDatabase;
-}
-
-// Password store for Mac.
-class PasswordStoreMac : public password_manager::PasswordStoreDefault {
- public:
- PasswordStoreMac(std::unique_ptr<password_manager::LoginDatabase> login_db,
- PrefService* prefs);
-
- // PasswordStore:
- bool Init(const syncer::SyncableService::StartSyncFlare& flare,
- PrefService* prefs) override;
- void ShutdownOnUIThread() override;
-
-#if defined(UNIT_TEST)
- password_manager::LoginDatabase* login_metadata_db() { return login_db(); }
-#endif
-
- private:
- ~PasswordStoreMac() override;
-
- // PasswordStore:
- bool InitOnBackgroundSequence(
- const syncer::SyncableService::StartSyncFlare& flare) override;
-
- // Writes status to the prefs.
- void UpdateStatusPref(password_manager::MigrationStatus status);
-
- // Bound to the pref containing migration status for the profile.
- IntegerPrefMember migration_status_;
-
- // Initial migration status when the class is initialized.
- password_manager::MigrationStatus initial_status_;
-
- DISALLOW_COPY_AND_ASSIGN(PasswordStoreMac);
-};
-
-#endif // CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_MAC_H_
diff --git a/chrome/browser/password_manager/password_store_mac_unittest.cc b/chrome/browser/password_manager/password_store_mac_unittest.cc
deleted file mode 100644
index ff81b2f2..0000000
--- a/chrome/browser/password_manager/password_store_mac_unittest.cc
+++ /dev/null
@@ -1,312 +0,0 @@
-// 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_mac.h"
-
-#include <utility>
-
-#include "base/files/scoped_temp_dir.h"
-#include "base/macros.h"
-#include "base/scoped_observer.h"
-#include "base/strings/utf_string_conversions.h"
-#include "base/test/metrics/histogram_tester.h"
-#include "base/test/scoped_task_environment.h"
-#include "chrome/browser/prefs/browser_prefs.h"
-#include "components/os_crypt/os_crypt_mocker.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 "components/sync_preferences/testing_pref_service_syncable.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace {
-
-using autofill::PasswordForm;
-using password_manager::MigrationStatus;
-using password_manager::PasswordStore;
-using password_manager::PasswordStoreChange;
-using password_manager::PasswordStoreChangeList;
-using testing::_;
-using testing::ElementsAre;
-using testing::IsEmpty;
-using testing::Pointee;
-
-class MockPasswordStoreConsumer
- : public password_manager::PasswordStoreConsumer {
- public:
- MockPasswordStoreConsumer() = default;
-
- const std::vector<std::unique_ptr<PasswordForm>>& forms() const {
- return forms_;
- }
-
- private:
- void OnGetPasswordStoreResults(
- std::vector<std::unique_ptr<PasswordForm>> results) override {
- forms_.swap(results);
- }
-
- std::vector<std::unique_ptr<PasswordForm>> forms_;
-
- DISALLOW_COPY_AND_ASSIGN(MockPasswordStoreConsumer);
-};
-
-class MockPasswordStoreObserver
- : public password_manager::PasswordStore::Observer {
- public:
- explicit MockPasswordStoreObserver(PasswordStoreMac* password_store)
- : guard_(this) {
- guard_.Add(password_store);
- }
- MOCK_METHOD1(OnLoginsChanged,
- void(const password_manager::PasswordStoreChangeList& changes));
-
- private:
- ScopedObserver<PasswordStoreMac, MockPasswordStoreObserver> guard_;
-
- DISALLOW_COPY_AND_ASSIGN(MockPasswordStoreObserver);
-};
-
-// A mock LoginDatabase that simulates a failing Init() method.
-class BadLoginDatabase : public password_manager::LoginDatabase {
- public:
- BadLoginDatabase() : password_manager::LoginDatabase(base::FilePath()) {}
- ~BadLoginDatabase() override {}
-
- // LoginDatabase:
- bool Init() override { return false; }
-
- private:
- DISALLOW_COPY_AND_ASSIGN(BadLoginDatabase);
-};
-
-class PasswordStoreMacTest : public testing::TestWithParam<MigrationStatus> {
- public:
- PasswordStoreMacTest();
- ~PasswordStoreMacTest() override;
-
- void CreateAndInitPasswordStore(
- std::unique_ptr<password_manager::LoginDatabase> login_db);
-
- void ClosePasswordStore();
-
- // Wait for all the previously enqueued operations to finish.
- void FinishAsyncProcessing();
-
- // Add/Update/Remove |form| and verify the operation succeeded.
- void AddForm(const PasswordForm& form);
- void UpdateForm(const PasswordForm& form);
- void RemoveForm(const PasswordForm& form);
-
- base::FilePath test_login_db_file_path() const;
-
- // Returns the expected migration status after the password store was inited.
- MigrationStatus GetTargetStatus() const;
-
- password_manager::LoginDatabase* login_db() const {
- return store_->login_metadata_db();
- }
-
- PasswordStoreMac* store() { return store_.get(); }
-
- protected:
- base::test::ScopedTaskEnvironment scoped_task_environment_;
- base::ScopedTempDir db_dir_;
- scoped_refptr<PasswordStoreMac> store_;
- sync_preferences::TestingPrefServiceSyncable testing_prefs_;
-};
-
-PasswordStoreMacTest::PasswordStoreMacTest() {
- EXPECT_TRUE(db_dir_.CreateUniqueTempDir());
- RegisterUserProfilePrefs(testing_prefs_.registry());
- testing_prefs_.SetInteger(password_manager::prefs::kKeychainMigrationStatus,
- static_cast<int>(GetParam()));
- // Ensure that LoginDatabase will use the mock keychain if it needs to
- // encrypt/decrypt a password.
- OSCryptMocker::SetUp();
-}
-
-PasswordStoreMacTest::~PasswordStoreMacTest() {
- ClosePasswordStore();
- OSCryptMocker::TearDown();
-}
-
-void PasswordStoreMacTest::CreateAndInitPasswordStore(
- std::unique_ptr<password_manager::LoginDatabase> login_db) {
- store_ = new PasswordStoreMac(std::move(login_db), &testing_prefs_);
- ASSERT_TRUE(store_->Init(syncer::SyncableService::StartSyncFlare(), nullptr));
-}
-
-void PasswordStoreMacTest::ClosePasswordStore() {
- if (!store_)
- return;
- store_->ShutdownOnUIThread();
- store_ = nullptr;
-}
-
-void PasswordStoreMacTest::FinishAsyncProcessing() {
- scoped_task_environment_.RunUntilIdle();
-}
-
-base::FilePath PasswordStoreMacTest::test_login_db_file_path() const {
- return db_dir_.GetPath().Append(FILE_PATH_LITERAL("login.db"));
-}
-
-MigrationStatus PasswordStoreMacTest::GetTargetStatus() const {
- if (GetParam() == MigrationStatus::NOT_STARTED ||
- GetParam() == MigrationStatus::FAILED_ONCE ||
- GetParam() == MigrationStatus::FAILED_TWICE) {
- return MigrationStatus::MIGRATION_STOPPED;
- }
- return GetParam();
-}
-
-void PasswordStoreMacTest::AddForm(const PasswordForm& form) {
- MockPasswordStoreObserver mock_observer(store());
-
- password_manager::PasswordStoreChangeList list;
- list.push_back(password_manager::PasswordStoreChange(
- password_manager::PasswordStoreChange::ADD, form));
- EXPECT_CALL(mock_observer, OnLoginsChanged(list));
- store()->AddLogin(form);
- FinishAsyncProcessing();
-}
-
-void PasswordStoreMacTest::UpdateForm(const PasswordForm& form) {
- MockPasswordStoreObserver mock_observer(store());
-
- password_manager::PasswordStoreChangeList list;
- list.push_back(password_manager::PasswordStoreChange(
- password_manager::PasswordStoreChange::UPDATE, form));
- EXPECT_CALL(mock_observer, OnLoginsChanged(list));
- store()->UpdateLogin(form);
- FinishAsyncProcessing();
-}
-
-void PasswordStoreMacTest::RemoveForm(const PasswordForm& form) {
- MockPasswordStoreObserver mock_observer(store());
-
- password_manager::PasswordStoreChangeList list;
- list.push_back(password_manager::PasswordStoreChange(
- password_manager::PasswordStoreChange::REMOVE, form));
- EXPECT_CALL(mock_observer, OnLoginsChanged(list));
- store()->RemoveLogin(form);
- FinishAsyncProcessing();
-}
-
-// ----------- Tests -------------
-
-TEST_P(PasswordStoreMacTest, Sanity) {
- base::HistogramTester histogram_tester;
-
- CreateAndInitPasswordStore(std::make_unique<password_manager::LoginDatabase>(
- test_login_db_file_path()));
- FinishAsyncProcessing();
- ClosePasswordStore();
-
- int status = testing_prefs_.GetInteger(
- password_manager::prefs::kKeychainMigrationStatus);
- EXPECT_EQ(static_cast<int>(GetTargetStatus()), status);
- histogram_tester.ExpectUniqueSample(
- "PasswordManager.KeychainMigration.Status", status, 1);
-}
-
-TEST_P(PasswordStoreMacTest, StartAndStop) {
- base::HistogramTester histogram_tester;
- // PasswordStore::ShutdownOnUIThread() immediately follows
- // PasswordStore::Init(). The message loop isn't running in between. Anyway,
- // PasswordStore should not collapse.
- CreateAndInitPasswordStore(std::make_unique<password_manager::LoginDatabase>(
- test_login_db_file_path()));
- ClosePasswordStore();
-
- FinishAsyncProcessing();
-
- histogram_tester.ExpectUniqueSample(
- "PasswordManager.KeychainMigration.Status",
- static_cast<int>(GetTargetStatus()), 1);
-}
-
-TEST_P(PasswordStoreMacTest, 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.
- CreateAndInitPasswordStore(std::make_unique<BadLoginDatabase>());
- FinishAsyncProcessing();
- EXPECT_FALSE(login_db());
-
- // The store should outlive the observer.
- scoped_refptr<PasswordStoreMac> store_refptr = store();
- MockPasswordStoreObserver mock_observer(store());
- EXPECT_CALL(mock_observer, OnLoginsChanged(_)).Times(0);
-
- // Add a new autofillable login + a blacklisted login.
- password_manager::PasswordFormData www_form_data = {
- PasswordForm::Scheme::kHtml,
- "http://www.facebook.com/",
- "http://www.facebook.com/index.html",
- "login",
- L"username",
- L"password",
- L"submit",
- L"not_joe_user",
- L"12345",
- true,
- 1};
- std::unique_ptr<PasswordForm> form = FillPasswordFormWithData(www_form_data);
- std::unique_ptr<PasswordForm> blacklisted_form(new PasswordForm(*form));
- blacklisted_form->signon_realm = "http://foo.example.com";
- blacklisted_form->origin = GURL("http://foo.example.com/origin");
- blacklisted_form->action = GURL("http://foo.example.com/action");
- blacklisted_form->blacklisted_by_user = true;
- store()->AddLogin(*form);
- store()->AddLogin(*blacklisted_form);
- FinishAsyncProcessing();
-
- // Get all logins; autofillable logins; blacklisted logins.
- MockPasswordStoreConsumer mock_consumer;
- store()->GetLogins(PasswordStore::FormDigest(*form), &mock_consumer);
- FinishAsyncProcessing();
- EXPECT_THAT(mock_consumer.forms(), IsEmpty());
-
- store()->GetAutofillableLogins(&mock_consumer);
- FinishAsyncProcessing();
- EXPECT_THAT(mock_consumer.forms(), IsEmpty());
-
- store()->GetAllLogins(&mock_consumer);
- FinishAsyncProcessing();
- EXPECT_THAT(mock_consumer.forms(), IsEmpty());
-
- // Report metrics.
- store()->ReportMetrics("Test Username", true, false);
- FinishAsyncProcessing();
-
- // Change the login.
- form->password_value = base::ASCIIToUTF16("a different password");
- store()->UpdateLogin(*form);
- FinishAsyncProcessing();
-
- // Delete one login; a range of logins.
- store()->RemoveLogin(*form);
- store()->RemoveLoginsCreatedBetween(base::Time(), base::Time::Max(),
- base::Closure());
- FinishAsyncProcessing();
-
- // Verify no notifications are fired during shutdown either.
- ClosePasswordStore();
-}
-
-INSTANTIATE_TEST_SUITE_P(,
- PasswordStoreMacTest,
- testing::Values(MigrationStatus::NOT_STARTED,
- MigrationStatus::MIGRATED,
- MigrationStatus::FAILED_ONCE,
- MigrationStatus::FAILED_TWICE,
- MigrationStatus::MIGRATED_DELETED,
- MigrationStatus::MIGRATED_PARTIALLY,
- MigrationStatus::MIGRATION_STOPPED));
-
-} // namespace
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index 516398a..1685c884 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -112,6 +112,7 @@
#include "components/optimization_guide/optimization_guide_prefs.h"
#include "components/password_manager/core/browser/password_bubble_experiment.h"
#include "components/password_manager/core/browser/password_manager.h"
+#include "components/password_manager/core/common/password_manager_pref_names.h"
#include "components/payments/core/payment_prefs.h"
#include "components/policy/core/browser/browser_policy_connector.h"
#include "components/policy/core/browser/url_blacklist_manager.h"
@@ -1109,4 +1110,7 @@
// Added 6/2019.
profile_prefs->ClearPref(kMediaCacheSize);
+#if defined(OS_MACOSX)
+ profile_prefs->ClearPref(password_manager::prefs::kKeychainMigrationStatus);
+#endif // defined(OS_MACOSX)
}
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index e42c12e..954961b 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -2909,7 +2909,6 @@
"../browser/page_load_metrics/resource_tracker_unittest.cc",
"../browser/password_manager/chrome_password_manager_client_unittest.cc",
"../browser/password_manager/password_manager_internals_service_unittest.cc",
- "../browser/password_manager/password_store_mac_unittest.cc",
"../browser/password_manager/password_store_x_unittest.cc",
"../browser/payments/payment_handler_permission_context_unittest.cc",
"../browser/performance_manager/decorators/frozen_frame_aggregator_unittest.cc",
