Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(403)

Unified Diff: components/password_manager/core/browser/password_store_default_unittest.cc

Issue 838453003: Open the LoginDatabase on the DB thread, not the UI thread. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fix nits from vabr@. Created 5 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/password_manager/core/browser/password_store_default_unittest.cc
diff --git a/components/password_manager/core/browser/password_store_default_unittest.cc b/components/password_manager/core/browser/password_store_default_unittest.cc
index 7b3bcd263a73d196ab688cf6d8bda8e7ab245612..b4d87c35839f8eeb074149f2f2fb542748960c54 100644
--- a/components/password_manager/core/browser/password_store_default_unittest.cc
+++ b/components/password_manager/core/browser/password_store_default_unittest.cc
@@ -5,12 +5,14 @@
#include "base/basictypes.h"
#include "base/bind.h"
#include "base/bind_helpers.h"
+#include "base/files/file_util.h"
#include "base/files/scoped_temp_dir.h"
#include "base/prefs/pref_service.h"
#include "base/stl_util.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "base/time/time.h"
+#include "components/password_manager/core/browser/login_database.h"
#include "components/password_manager/core/browser/password_form_data.h"
#include "components/password_manager/core/browser/password_store_change.h"
#include "components/password_manager/core/browser/password_store_consumer.h"
@@ -38,21 +40,52 @@ class MockPasswordStoreObserver : public PasswordStore::Observer {
MOCK_METHOD1(OnLoginsChanged, void(const PasswordStoreChangeList& changes));
};
+// A mock LoginDatabase that simulates a failing Init() method.
+class BadLoginDatabase : public LoginDatabase {
+ public:
+ BadLoginDatabase() : LoginDatabase(base::FilePath()) {}
+ ~BadLoginDatabase() override {}
+
+ // LoginDatabase:
+ bool Init() override { return false; }
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(BadLoginDatabase);
+};
+
+PasswordFormData CreateTestPasswordFormData() {
+ PasswordFormData data = {
+ PasswordForm::SCHEME_HTML,
+ "http://bar.example.com",
+ "http://bar.example.com/origin",
+ "http://bar.example.com/action",
+ L"submit_element",
+ L"username_element",
+ L"password_element",
+ L"username_value",
+ L"password_value",
+ true,
+ false,
+ 1
+ };
+ return data;
+}
+
} // anonymous namespace
class PasswordStoreDefaultTest : public testing::Test {
protected:
void SetUp() override {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
- login_db_.reset(new LoginDatabase());
- ASSERT_TRUE(login_db_->Init(
- temp_dir_.path().Append(FILE_PATH_LITERAL("login_test"))));
}
void TearDown() override { ASSERT_TRUE(temp_dir_.Delete()); }
+ base::FilePath test_login_db_file_path() const {
+ return temp_dir_.path().Append(FILE_PATH_LITERAL("login_test"));
+ }
+
base::MessageLoopForUI message_loop_;
- scoped_ptr<LoginDatabase> login_db_;
base::ScopedTempDir temp_dir_;
};
@@ -62,9 +95,8 @@ ACTION(STLDeleteElements0) {
TEST_F(PasswordStoreDefaultTest, NonASCIIData) {
scoped_refptr<PasswordStoreDefault> store(new PasswordStoreDefault(
- base::MessageLoopProxy::current(),
- base::MessageLoopProxy::current(),
- login_db_.release()));
+ base::MessageLoopProxy::current(), base::MessageLoopProxy::current(),
+ make_scoped_ptr(new LoginDatabase(test_login_db_file_path()))));
store->Init(syncer::SyncableService::StartSyncFlare());
// Some non-ASCII password form data.
@@ -108,23 +140,12 @@ TEST_F(PasswordStoreDefaultTest, NonASCIIData) {
TEST_F(PasswordStoreDefaultTest, Notifications) {
scoped_refptr<PasswordStoreDefault> store(new PasswordStoreDefault(
- base::MessageLoopProxy::current(),
- base::MessageLoopProxy::current(),
- login_db_.release()));
+ base::MessageLoopProxy::current(), base::MessageLoopProxy::current(),
+ make_scoped_ptr(new LoginDatabase(test_login_db_file_path()))));
store->Init(syncer::SyncableService::StartSyncFlare());
- PasswordFormData form_data =
- { PasswordForm::SCHEME_HTML,
- "http://bar.example.com",
- "http://bar.example.com/origin",
- "http://bar.example.com/action",
- L"submit_element",
- L"username_element",
- L"password_element",
- L"username_value",
- L"password_value",
- true, false, 1 };
- scoped_ptr<PasswordForm> form(CreatePasswordFormFromData(form_data));
+ scoped_ptr<PasswordForm> form(
+ CreatePasswordFormFromData(CreateTestPasswordFormData()));
MockPasswordStoreObserver observer;
store->AddObserver(&observer);
@@ -170,4 +191,68 @@ TEST_F(PasswordStoreDefaultTest, Notifications) {
base::MessageLoop::current()->RunUntilIdle();
}
+// 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.
+TEST_F(PasswordStoreDefaultTest, OperationsOnABadDatabaseSilentlyFail) {
+ scoped_refptr<PasswordStoreDefault> bad_store(new PasswordStoreDefault(
+ base::MessageLoopProxy::current(), base::MessageLoopProxy::current(),
+ make_scoped_ptr<LoginDatabase>(new BadLoginDatabase)));
+
+ bad_store->Init(syncer::SyncableService::StartSyncFlare());
+ base::MessageLoop::current()->RunUntilIdle();
+ ASSERT_EQ(nullptr, bad_store->login_db());
+
+ testing::StrictMock<MockPasswordStoreObserver> mock_observer;
+ bad_store->AddObserver(&mock_observer);
+
+ // Add a new autofillable login + a blacklisted login.
+ scoped_ptr<PasswordForm> form(
+ CreatePasswordFormFromData(CreateTestPasswordFormData()));
+ scoped_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;
+ bad_store->AddLogin(*form);
+ bad_store->AddLogin(*blacklisted_form);
+ base::MessageLoop::current()->RunUntilIdle();
+
+ // Get all logins; autofillable logins; blacklisted logins.
+ testing::StrictMock<MockPasswordStoreConsumer> mock_consumer;
+ EXPECT_CALL(mock_consumer, OnGetPasswordStoreResults(testing::ElementsAre()));
+ bad_store->GetLogins(*form, PasswordStore::DISALLOW_PROMPT, &mock_consumer);
+ base::MessageLoop::current()->RunUntilIdle();
+ testing::Mock::VerifyAndClearExpectations(&mock_consumer);
+ EXPECT_CALL(mock_consumer, OnGetPasswordStoreResults(testing::ElementsAre()));
+ bad_store->GetAutofillableLogins(&mock_consumer);
+ base::MessageLoop::current()->RunUntilIdle();
+ testing::Mock::VerifyAndClearExpectations(&mock_consumer);
+ EXPECT_CALL(mock_consumer, OnGetPasswordStoreResults(testing::ElementsAre()));
+ bad_store->GetBlacklistLogins(&mock_consumer);
+ base::MessageLoop::current()->RunUntilIdle();
+
+ // Report metrics.
+ bad_store->ReportMetrics("Test Username", true);
+ base::MessageLoop::current()->RunUntilIdle();
+
+ // Change the login.
+ form->password_value = base::ASCIIToUTF16("a different password");
+ bad_store->UpdateLogin(*form);
+ base::MessageLoop::current()->RunUntilIdle();
+
+ // Delete one login; a range of logins.
+ bad_store->RemoveLogin(*form);
+ base::MessageLoop::current()->RunUntilIdle();
+ bad_store->RemoveLoginsCreatedBetween(base::Time(), base::Time::Max());
+ base::MessageLoop::current()->RunUntilIdle();
+ bad_store->RemoveLoginsSyncedBetween(base::Time(), base::Time::Max());
+ base::MessageLoop::current()->RunUntilIdle();
+
+ // Ensure no notifications and no explosions during shutdown either.
+ bad_store->RemoveObserver(&mock_observer);
+ bad_store->Shutdown();
+ base::MessageLoop::current()->RunUntilIdle();
+}
+
} // namespace password_manager

Powered by Google App Engine
This is Rietveld 408576698