Index: chrome/browser/password_manager/password_store_mac_unittest.cc |
diff --git a/chrome/browser/password_manager/password_store_mac_unittest.cc b/chrome/browser/password_manager/password_store_mac_unittest.cc |
index 29a3f90f6c8b93e0ad1f891d6ce2f87c928d97a8..4e90d3ed2dff45d4dbe2351136d82bee778e62c1 100644 |
--- a/chrome/browser/password_manager/password_store_mac_unittest.cc |
+++ b/chrome/browser/password_manager/password_store_mac_unittest.cc |
@@ -14,6 +14,7 @@ |
#include "chrome/browser/password_manager/password_store_mac_internal.h" |
#include "chrome/common/chrome_paths.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_consumer.h" |
#include "content/public/test/test_browser_thread.h" |
#include "crypto/mock_apple_keychain.h" |
@@ -32,15 +33,13 @@ using password_manager::PasswordStore; |
using password_manager::PasswordStoreConsumer; |
using testing::_; |
using testing::DoAll; |
+using testing::ElementsAre; |
using testing::Invoke; |
+using testing::SizeIs; |
using testing::WithArg; |
namespace { |
-ACTION(STLDeleteElements0) { |
- STLDeleteContainerPointers(arg0.begin(), arg0.end()); |
-} |
- |
ACTION(QuitUIMessageLoop) { |
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); |
base::MessageLoop::current()->Quit(); |
@@ -48,28 +47,20 @@ ACTION(QuitUIMessageLoop) { |
class MockPasswordStoreConsumer : public PasswordStoreConsumer { |
public: |
- MOCK_METHOD1(OnGetPasswordStoreResults, |
- void(const std::vector<autofill::PasswordForm*>&)); |
+ MOCK_METHOD1(OnGetPasswordStoreResultsConstRef, |
+ void(const std::vector<PasswordForm*>&)); |
- void CopyElements(const std::vector<autofill::PasswordForm*>& forms) { |
- last_result.clear(); |
- for (size_t i = 0; i < forms.size(); ++i) { |
- last_result.push_back(*forms[i]); |
- } |
+ // GMock cannot mock methods with move-only args. |
+ void OnGetPasswordStoreResults(ScopedVector<PasswordForm> results) override { |
+ OnGetPasswordStoreResultsConstRef(results.get()); |
} |
- // Runs the current thread's message loop until OnGetPasswordStoreResults() |
- // is posted to it. This method should be called immediately after GetLogins, |
- // without pumping the message loop in-between. |
- void WaitOnGetPasswordStoreResults() { |
- EXPECT_CALL(*this, OnGetPasswordStoreResults(_)).WillOnce(DoAll( |
- WithArg<0>(Invoke(this, &MockPasswordStoreConsumer::CopyElements)), |
- WithArg<0>(STLDeleteElements0()), |
- QuitUIMessageLoop())); |
- base::MessageLoop::current()->Run(); |
+ void SaveACopyOfFirstForm(const std::vector<PasswordForm*>& result) { |
vasilii
2015/02/05 19:23:27
Turn it to the action taking an argument (Password
vabr (Chromium)
2015/02/06 14:16:05
Done.
|
+ DCHECK(!result.empty()); |
+ first_form = *result[0]; |
} |
- std::vector<PasswordForm> last_result; |
+ PasswordForm first_form; // First form of the returned result, if any. |
}; |
class MockPasswordStoreObserver : public PasswordStore::Observer { |
@@ -230,11 +221,10 @@ struct PasswordFormData { |
const double creation_time; |
}; |
-// Creates and returns a new PasswordForm built from form_data. Caller is |
-// responsible for deleting the object when finished with it. |
-static PasswordForm* CreatePasswordFormFromData( |
+// Creates and returns a new PasswordForm built from form_data. |
+static scoped_ptr<PasswordForm> CreatePasswordFormFromData( |
vasilii
2015/02/05 19:23:26
I'd put this method and it's neighbors to the name
vabr (Chromium)
2015/02/06 14:16:05
Done.
BTW., this method and the struct will be rem
|
const PasswordFormData& form_data) { |
- PasswordForm* form = new PasswordForm(); |
+ scoped_ptr<PasswordForm> form(new PasswordForm()); |
form->scheme = form_data.scheme; |
form->preferred = form_data.preferred; |
form->ssl_valid = form_data.ssl_valid; |
@@ -263,7 +253,7 @@ static PasswordForm* CreatePasswordFormFromData( |
} |
form->avatar_url = GURL("https://accounts.google.com/Avatar"); |
form->federation_url = GURL("https://accounts.google.com/login"); |
- return form; |
+ return form.Pass(); |
} |
// Macro to simplify calling CheckFormsAgainstExpectations with a useful label. |
@@ -477,8 +467,8 @@ TEST_F(PasswordStoreMacInternalsTest, TestKeychainSearch) { |
MacKeychainPasswordFormAdapter owned_keychain_adapter(keychain_); |
owned_keychain_adapter.SetFindsOnlyOwnedItems(true); |
for (unsigned int i = 0; i < arraysize(test_data); ++i) { |
- scoped_ptr<PasswordForm> query_form( |
- CreatePasswordFormFromData(test_data[i].data)); |
+ scoped_ptr<PasswordForm> query_form = |
+ CreatePasswordFormFromData(test_data[i].data); |
// Check matches treating the form as a fill target. |
ScopedVector<autofill::PasswordForm> matching_items = |
@@ -557,8 +547,8 @@ TEST_F(PasswordStoreMacInternalsTest, TestKeychainExactSearch) { |
for (unsigned int i = 0; i < arraysize(base_form_data); ++i) { |
// Create a base form and make sure we find a match. |
- scoped_ptr<PasswordForm> base_form(CreatePasswordFormFromData( |
- base_form_data[i])); |
+ scoped_ptr<PasswordForm> base_form = |
+ CreatePasswordFormFromData(base_form_data[i]); |
EXPECT_TRUE(keychain_adapter.HasPasswordsMergeableWithForm(*base_form)); |
EXPECT_TRUE(keychain_adapter.HasPasswordExactlyMatchingForm(*base_form)); |
@@ -631,8 +621,8 @@ TEST_F(PasswordStoreMacInternalsTest, TestKeychainAdd) { |
owned_keychain_adapter.SetFindsOnlyOwnedItems(true); |
for (unsigned int i = 0; i < arraysize(test_data); ++i) { |
- scoped_ptr<PasswordForm> in_form( |
- CreatePasswordFormFromData(test_data[i].data)); |
+ scoped_ptr<PasswordForm> in_form = |
+ CreatePasswordFormFromData(test_data[i].data); |
bool add_succeeded = owned_keychain_adapter.AddPassword(*in_form); |
EXPECT_EQ(test_data[i].should_succeed, add_succeeded); |
if (add_succeeded) { |
@@ -650,7 +640,7 @@ TEST_F(PasswordStoreMacInternalsTest, TestKeychainAdd) { |
"http://some.domain.com/insecure.html", NULL, |
NULL, NULL, NULL, L"joe_user", L"updated_password", false, false, 0 |
}; |
- scoped_ptr<PasswordForm> update_form(CreatePasswordFormFromData(data)); |
+ scoped_ptr<PasswordForm> update_form = CreatePasswordFormFromData(data); |
MacKeychainPasswordFormAdapter keychain_adapter(keychain_); |
EXPECT_TRUE(keychain_adapter.AddPassword(*update_form)); |
SecKeychainItemRef keychain_item = reinterpret_cast<SecKeychainItemRef>(2); |
@@ -683,13 +673,13 @@ TEST_F(PasswordStoreMacInternalsTest, TestKeychainRemove) { |
owned_keychain_adapter.SetFindsOnlyOwnedItems(true); |
// Add our test item so that we can delete it. |
- PasswordForm* add_form = CreatePasswordFormFromData(test_data[0].data); |
+ scoped_ptr<PasswordForm> add_form = |
+ CreatePasswordFormFromData(test_data[0].data); |
EXPECT_TRUE(owned_keychain_adapter.AddPassword(*add_form)); |
- delete add_form; |
for (unsigned int i = 0; i < arraysize(test_data); ++i) { |
- scoped_ptr<PasswordForm> form(CreatePasswordFormFromData( |
- test_data[i].data)); |
+ scoped_ptr<PasswordForm> form = |
+ CreatePasswordFormFromData(test_data[i].data); |
EXPECT_EQ(test_data[i].should_succeed, |
owned_keychain_adapter.RemovePassword(*form)); |
@@ -890,13 +880,13 @@ TEST_F(PasswordStoreMacInternalsTest, TestFormMerge) { |
for (std::vector<PasswordFormData*>::iterator i = |
test_data[KEYCHAIN_INPUT][test_case].begin(); |
i != test_data[KEYCHAIN_INPUT][test_case].end(); ++i) { |
- keychain_forms.push_back(CreatePasswordFormFromData(*(*i))); |
+ keychain_forms.push_back(CreatePasswordFormFromData(*(*i)).release()); |
} |
ScopedVector<autofill::PasswordForm> database_forms; |
for (std::vector<PasswordFormData*>::iterator i = |
test_data[DATABASE_INPUT][test_case].begin(); |
i != test_data[DATABASE_INPUT][test_case].end(); ++i) { |
- database_forms.push_back(CreatePasswordFormFromData(*(*i))); |
+ database_forms.push_back(CreatePasswordFormFromData(*(*i)).release()); |
} |
ScopedVector<autofill::PasswordForm> merged_forms; |
@@ -941,7 +931,7 @@ TEST_F(PasswordStoreMacInternalsTest, TestPasswordBulkLookup) { |
}; |
ScopedVector<autofill::PasswordForm> database_forms; |
for (unsigned int i = 0; i < arraysize(db_data); ++i) { |
- database_forms.push_back(CreatePasswordFormFromData(db_data[i])); |
+ database_forms.push_back(CreatePasswordFormFromData(db_data[i]).release()); |
} |
ScopedVector<autofill::PasswordForm> merged_forms; |
internal_keychain_helpers::GetPasswordsForForms(*keychain_, &database_forms, |
@@ -968,7 +958,7 @@ TEST_F(PasswordStoreMacInternalsTest, TestBlacklistedFiltering) { |
}; |
ScopedVector<autofill::PasswordForm> database_forms; |
for (unsigned int i = 0; i < arraysize(db_data); ++i) { |
- database_forms.push_back(CreatePasswordFormFromData(db_data[i])); |
+ database_forms.push_back(CreatePasswordFormFromData(db_data[i]).release()); |
} |
ScopedVector<autofill::PasswordForm> merged_forms; |
internal_keychain_helpers::GetPasswordsForForms(*keychain_, &database_forms, |
@@ -1069,8 +1059,8 @@ TEST_F(PasswordStoreMacInternalsTest, TestPasswordGetAll) { |
L"testname", L"testpass", false, false, 0 }, |
}; |
for (unsigned int i = 0; i < arraysize(owned_password_data); ++i) { |
- scoped_ptr<PasswordForm> form(CreatePasswordFormFromData( |
- owned_password_data[i])); |
+ scoped_ptr<PasswordForm> form = |
+ CreatePasswordFormFromData(owned_password_data[i]); |
owned_keychain_adapter.AddPassword(*form); |
} |
@@ -1137,7 +1127,9 @@ class PasswordStoreMacTest : public testing::Test { |
// to finish. |
MockPasswordStoreConsumer consumer; |
store_->GetLogins(PasswordForm(), PasswordStore::ALLOW_PROMPT, &consumer); |
- consumer.WaitOnGetPasswordStoreResults(); |
+ EXPECT_CALL(consumer, OnGetPasswordStoreResultsConstRef(_)) |
+ .WillOnce(QuitUIMessageLoop()); |
+ base::MessageLoop::current()->Run(); |
} |
TestPasswordStoreMac* store() { return store_.get(); } |
@@ -1161,7 +1153,7 @@ TEST_F(PasswordStoreMacTest, TestStoreUpdate) { |
"http://some.domain.com/insecure.html", "login.cgi", |
L"username", L"password", L"submit", L"joe_user", L"sekrit", true, false, 1 |
}; |
- scoped_ptr<PasswordForm> joint_form(CreatePasswordFormFromData(joint_data)); |
+ scoped_ptr<PasswordForm> joint_form = CreatePasswordFormFromData(joint_data); |
login_db()->AddLogin(*joint_form); |
MockAppleKeychain::KeychainTestData joint_keychain_data = { |
kSecAuthenticationTypeHTMLForm, "some.domain.com", |
@@ -1210,8 +1202,8 @@ TEST_F(PasswordStoreMacTest, TestStoreUpdate) { |
}, |
}; |
for (unsigned int i = 0; i < arraysize(updates); ++i) { |
- scoped_ptr<PasswordForm> form(CreatePasswordFormFromData( |
- updates[i].form_data)); |
+ scoped_ptr<PasswordForm> form = |
+ CreatePasswordFormFromData(updates[i].form_data); |
store_->UpdateLogin(*form); |
} |
@@ -1219,8 +1211,8 @@ TEST_F(PasswordStoreMacTest, TestStoreUpdate) { |
MacKeychainPasswordFormAdapter keychain_adapter(keychain()); |
for (unsigned int i = 0; i < arraysize(updates); ++i) { |
- scoped_ptr<PasswordForm> query_form( |
- CreatePasswordFormFromData(updates[i].form_data)); |
+ scoped_ptr<PasswordForm> query_form = |
+ CreatePasswordFormFromData(updates[i].form_data); |
ScopedVector<autofill::PasswordForm> matching_items = |
keychain_adapter.PasswordsFillingForm(query_form->signon_realm, |
@@ -1263,7 +1255,7 @@ TEST_F(PasswordStoreMacTest, TestDBKeychainAssociation) { |
"http://www.facebook.com/index.html", "login", |
L"username", L"password", L"submit", L"joe_user", L"sekrit", true, false, 1 |
}; |
- scoped_ptr<PasswordForm> www_form(CreatePasswordFormFromData(www_form_data)); |
+ scoped_ptr<PasswordForm> www_form = CreatePasswordFormFromData(www_form_data); |
login_db()->AddLogin(*www_form); |
MacKeychainPasswordFormAdapter owned_keychain_adapter(keychain()); |
owned_keychain_adapter.SetFindsOnlyOwnedItems(true); |
@@ -1276,11 +1268,15 @@ TEST_F(PasswordStoreMacTest, TestDBKeychainAssociation) { |
MockPasswordStoreConsumer consumer; |
store_->GetLogins(m_form, PasswordStore::ALLOW_PROMPT, &consumer); |
- consumer.WaitOnGetPasswordStoreResults(); |
- EXPECT_EQ(1u, consumer.last_result.size()); |
+ PasswordForm returned_form; |
vasilii
2015/02/05 19:23:26
Unused now, but to be used in your next PatchSet :
vabr (Chromium)
2015/02/06 14:16:05
Acknowledged. :)
|
+ EXPECT_CALL(consumer, OnGetPasswordStoreResultsConstRef(SizeIs(1u))) |
+ .WillOnce(DoAll( |
+ Invoke(&consumer, &MockPasswordStoreConsumer::SaveACopyOfFirstForm), |
+ QuitUIMessageLoop())); |
+ base::MessageLoop::current()->Run(); |
// 3. Add the returned password for m.facebook.com. |
- login_db()->AddLogin(consumer.last_result[0]); |
+ login_db()->AddLogin(consumer.first_form); |
owned_keychain_adapter.AddPassword(m_form); |
// 4. Remove both passwords. |
@@ -1349,12 +1345,12 @@ void CheckRemoveLoginsBetween(PasswordStoreMacTest* test, bool check_created) { |
PasswordForm::SCHEME_HTML, "http://different.com/", |
"http://different.com/index.html", "login", L"submit", L"username", |
L"password", L"different_joe_user", L"sekrit", true, false, 0 }; |
- scoped_ptr<PasswordForm> form_facebook( |
- CreatePasswordFormFromData(www_form_data_facebook)); |
- scoped_ptr<PasswordForm> form_facebook_old( |
- CreatePasswordFormFromData(www_form_data_facebook_old)); |
- scoped_ptr<PasswordForm> form_other( |
- CreatePasswordFormFromData(www_form_data_other)); |
+ scoped_ptr<PasswordForm> form_facebook = |
+ CreatePasswordFormFromData(www_form_data_facebook); |
+ scoped_ptr<PasswordForm> form_facebook_old = |
+ CreatePasswordFormFromData(www_form_data_facebook_old); |
+ scoped_ptr<PasswordForm> form_other = |
+ CreatePasswordFormFromData(www_form_data_other); |
base::Time now = base::Time::Now(); |
// TODO(vasilii): remove the next line once crbug/374132 is fixed. |
now = base::Time::FromTimeT(now.ToTimeT()); |
@@ -1453,7 +1449,8 @@ TEST_F(PasswordStoreMacTest, TestRemoveLoginsMultiProfile) { |
PasswordForm::SCHEME_HTML, "http://www.facebook.com/", |
"http://www.facebook.com/index.html", "login", L"username", L"password", |
L"submit", L"joe_user", L"sekrit", true, false, 1 }; |
- scoped_ptr<PasswordForm> www_form(CreatePasswordFormFromData(www_form_data1)); |
+ scoped_ptr<PasswordForm> www_form = |
+ CreatePasswordFormFromData(www_form_data1); |
EXPECT_TRUE(owned_keychain_adapter.AddPassword(*www_form)); |
// Add a password from the current profile. |
@@ -1461,7 +1458,7 @@ TEST_F(PasswordStoreMacTest, TestRemoveLoginsMultiProfile) { |
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.reset(CreatePasswordFormFromData(www_form_data2)); |
+ www_form = CreatePasswordFormFromData(www_form_data2); |
store_->AddLogin(*www_form); |
FinishAsyncProcessing(); |
@@ -1506,7 +1503,7 @@ TEST_F(PasswordStoreMacTest, StoreIsUsableImmediatelyAfterConstruction) { |
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}; |
- scoped_ptr<PasswordForm> form(CreatePasswordFormFromData(www_form_data)); |
+ scoped_ptr<PasswordForm> form = CreatePasswordFormFromData(www_form_data); |
store()->AddLogin(*form); |
MockPasswordStoreConsumer mock_consumer; |
@@ -1515,8 +1512,9 @@ TEST_F(PasswordStoreMacTest, StoreIsUsableImmediatelyAfterConstruction) { |
// Now the read/write tasks are scheduled, let the DB initialization proceed. |
event.Signal(); |
- mock_consumer.WaitOnGetPasswordStoreResults(); |
- EXPECT_EQ(1u, mock_consumer.last_result.size()); |
+ EXPECT_CALL(mock_consumer, OnGetPasswordStoreResultsConstRef(SizeIs(1u))) |
+ .WillOnce(QuitUIMessageLoop()); |
+ base::MessageLoop::current()->Run(); |
EXPECT_TRUE(login_db()); |
} |
@@ -1538,7 +1536,7 @@ TEST_F(PasswordStoreMacTest, OperationsOnABadDatabaseSilentlyFail) { |
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}; |
- scoped_ptr<PasswordForm> form(CreatePasswordFormFromData(www_form_data)); |
+ scoped_ptr<PasswordForm> form = CreatePasswordFormFromData(www_form_data); |
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"); |
@@ -1551,14 +1549,19 @@ TEST_F(PasswordStoreMacTest, OperationsOnABadDatabaseSilentlyFail) { |
// Get all logins; autofillable logins; blacklisted logins. |
MockPasswordStoreConsumer mock_consumer; |
store()->GetLogins(*form, PasswordStore::DISALLOW_PROMPT, &mock_consumer); |
- mock_consumer.WaitOnGetPasswordStoreResults(); |
- EXPECT_TRUE(mock_consumer.last_result.empty()); |
+ EXPECT_CALL(mock_consumer, OnGetPasswordStoreResultsConstRef(ElementsAre())) |
vasilii
2015/02/05 19:23:26
IsEmpty()
vabr (Chromium)
2015/02/06 14:16:04
Done.
|
+ .WillOnce(QuitUIMessageLoop()); |
+ base::MessageLoop::current()->Run(); |
+ |
store()->GetAutofillableLogins(&mock_consumer); |
- mock_consumer.WaitOnGetPasswordStoreResults(); |
- EXPECT_TRUE(mock_consumer.last_result.empty()); |
+ EXPECT_CALL(mock_consumer, OnGetPasswordStoreResultsConstRef(ElementsAre())) |
+ .WillOnce(QuitUIMessageLoop()); |
+ base::MessageLoop::current()->Run(); |
+ |
store()->GetBlacklistLogins(&mock_consumer); |
- mock_consumer.WaitOnGetPasswordStoreResults(); |
- EXPECT_TRUE(mock_consumer.last_result.empty()); |
+ EXPECT_CALL(mock_consumer, OnGetPasswordStoreResultsConstRef(ElementsAre())) |
+ .WillOnce(QuitUIMessageLoop()); |
vasilii
2015/02/05 19:23:26
This is repeated many times here. Sounds like a ca
vabr (Chromium)
2015/02/06 14:16:04
Done.
|
+ base::MessageLoop::current()->Run(); |
// Report metrics. |
store()->ReportMetrics("Test Username", true); |