Index: components/password_manager/core/browser/form_fetcher_impl_unittest.cc |
diff --git a/components/password_manager/core/browser/form_fetcher_impl_unittest.cc b/components/password_manager/core/browser/form_fetcher_impl_unittest.cc |
index 90c36a90dbf9ad13e8a29391c28639b217517182..83a0290900cc96f8a41100496a2911fad61a2518 100644 |
--- a/components/password_manager/core/browser/form_fetcher_impl_unittest.cc |
+++ b/components/password_manager/core/browser/form_fetcher_impl_unittest.cc |
@@ -44,6 +44,12 @@ namespace password_manager { |
namespace { |
+const char kTestHttpRealm[] = "http://accounts.google.com/"; |
+const char kTestHttpURL[] = "http://accounts.google.com/a/LoginAuth"; |
+ |
+const char kTestHttpsRealm[] = "https://accounts.google.com/"; |
+const char kTestHttpsURL[] = "https://accounts.google.com/a/LoginAuth"; |
+ |
class MockConsumer : public FormFetcher::Consumer { |
public: |
MOCK_METHOD2(ProcessMatches, |
@@ -128,7 +134,7 @@ PasswordForm CreateHTTPNonFederated() { |
PasswordForm CreateFederated() { |
PasswordForm form = CreateNonFederated(); |
form.password_value.clear(); |
- form.federation_origin = url::Origin(GURL("https://accounts.google.com/")); |
+ form.federation_origin = url::Origin(GURL(kTestHttpsRealm)); |
return form; |
} |
@@ -162,13 +168,14 @@ class FormFetcherImplTest : public testing::Test { |
public: |
FormFetcherImplTest() |
: form_digest_(PasswordForm::SCHEME_HTML, |
- "http://accounts.google.com", |
- GURL("http://accounts.google.com/a/LoginAuth")) { |
+ kTestHttpRealm, |
+ GURL(kTestHttpURL)) { |
mock_store_ = new MockPasswordStore(); |
client_.set_store(mock_store_.get()); |
form_fetcher_ = base::MakeUnique<FormFetcherImpl>( |
- form_digest_, &client_, /* should_migrate_http_passwords */ false); |
+ form_digest_, &client_, false /* should_migrate_http_passwords */, |
+ false /* should_query_suppressed_https_forms */); |
} |
~FormFetcherImplTest() override { mock_store_->ShutdownOnUIThread(); } |
@@ -399,7 +406,8 @@ TEST_F(FormFetcherImplTest, DoNotTryToMigrateHTTPPasswordsOnHTTPSites) { |
// A new form fetcher is created to be able to set the form digest and |
// migration flag. |
form_fetcher_ = base::MakeUnique<FormFetcherImpl>( |
- form_digest_, &client_, /* should_migrate_http_passwords */ true); |
+ form_digest_, &client_, true /* should_migrate_http_passwords */, |
+ false /* should_query_suppressed_https_forms */); |
EXPECT_CALL(consumer_, ProcessMatches(IsEmpty(), 0u)); |
form_fetcher_->AddConsumer(&consumer_); |
@@ -441,7 +449,8 @@ TEST_F(FormFetcherImplTest, TryToMigrateHTTPPasswordsOnHTTPSSites) { |
// A new form fetcher is created to be able to set the form digest and |
// migration flag. |
form_fetcher_ = base::MakeUnique<FormFetcherImpl>( |
- form_digest_, &client_, /* should_migrate_http_passwords */ true); |
+ form_digest_, &client_, true /* should_migrate_http_passwords */, |
+ false /* should_query_suppressed_https_forms */); |
EXPECT_CALL(consumer_, ProcessMatches(IsEmpty(), 0u)); |
form_fetcher_->AddConsumer(&consumer_); |
@@ -510,7 +519,8 @@ TEST_F(FormFetcherImplTest, StateIsWaitingDuringMigration) { |
// A new form fetcher is created to be able to set the form digest and |
// migration flag. |
form_fetcher_ = base::MakeUnique<FormFetcherImpl>( |
- form_digest_, &client_, /* should_migrate_http_passwords */ true); |
+ form_digest_, &client_, true /* should_migrate_http_passwords */, |
+ false /* should_query_suppressed_https_forms */); |
PasswordForm https_form = CreateNonFederated(); |
@@ -551,6 +561,181 @@ TEST_F(FormFetcherImplTest, StateIsWaitingDuringMigration) { |
EXPECT_EQ(FormFetcher::State::NOT_WAITING, form_fetcher_->GetState()); |
} |
+TEST_F(FormFetcherImplTest, SuppressedHTTPSForms_QueriedForHTTPOrigins) { |
+ form_fetcher_ = base::MakeUnique<FormFetcherImpl>( |
+ form_digest_, &client_, false /* should_migrate_http_passwords */, |
+ true /* should_query_suppressed_https_forms */); |
+ EXPECT_CALL(consumer_, ProcessMatches(IsEmpty(), 0u)); |
+ form_fetcher_->AddConsumer(&consumer_); |
+ testing::Mock::VerifyAndClearExpectations(&consumer_); |
+ |
+ Fetch(); |
+ |
+ // The matching PasswordStore results coming in should trigger another |
+ // GetLogins request to fetcht the suppressed HTTPS forms. |
+ |
+ const PasswordStore::FormDigest https_version_of_form_digest( |
+ PasswordForm::SCHEME_HTML, kTestHttpsRealm, GURL(kTestHttpsURL)); |
+ PasswordStoreConsumer* https_form_fetcher_ptr = nullptr; |
+ EXPECT_CALL(*mock_store_, GetLogins(https_version_of_form_digest, _)) |
+ .WillOnce(::testing::SaveArg<1>(&https_form_fetcher_ptr)); |
+ EXPECT_CALL(consumer_, ProcessMatches(IsEmpty(), 0u)); |
+ |
+ form_fetcher_->OnGetPasswordStoreResults( |
+ MakeResults(std::vector<PasswordForm>())); |
+ |
+ testing::Mock::VerifyAndClearExpectations(&consumer_); |
+ testing::Mock::VerifyAndClearExpectations(mock_store_.get()); |
+ EXPECT_EQ(FormFetcher::State::NOT_WAITING, form_fetcher_->GetState()); |
+ EXPECT_FALSE(form_fetcher_->DidCompleteQueryingSuppressedHTTPSForms()); |
+ ASSERT_TRUE(https_form_fetcher_ptr); |
+ |
+ https_form_fetcher_ptr->OnGetPasswordStoreResults( |
+ MakeResults(std::vector<PasswordForm>())); |
+ |
+ EXPECT_EQ(FormFetcher::State::NOT_WAITING, form_fetcher_->GetState()); |
+ EXPECT_TRUE(form_fetcher_->DidCompleteQueryingSuppressedHTTPSForms()); |
+ EXPECT_THAT(form_fetcher_->GetSuppressedHTTPSForms(), IsEmpty()); |
+ |
kolos1
2017/05/22 12:35:07
Could we split this test into 3 test?
engedy
2017/05/22 20:20:19
Done. I also introduced two helper methods to cut
|
+ // Another call to Fetch() should refetch the list of suppressed HTTPS |
+ // credentials too. It should not matter that there are matching HTTP forms. |
+ |
+ Fetch(); |
+ |
+ const PasswordForm matching_http_form = CreateHTTPNonFederated(); |
+ const PasswordForm suppressed_https_form1 = CreateNonFederated(); |
+ const PasswordForm suppressed_https_form2 = CreateFederated(); |
+ https_form_fetcher_ptr = nullptr; |
+ EXPECT_CALL(*mock_store_, GetLogins(https_version_of_form_digest, _)) |
+ .WillOnce(::testing::SaveArg<1>(&https_form_fetcher_ptr)); |
+ EXPECT_CALL( |
+ consumer_, |
+ ProcessMatches(UnorderedElementsAre(Pointee(matching_http_form)), 0u)); |
+ |
+ form_fetcher_->OnGetPasswordStoreResults(MakeResults({matching_http_form})); |
+ |
+ testing::Mock::VerifyAndClearExpectations(&consumer_); |
+ testing::Mock::VerifyAndClearExpectations(mock_store_.get()); |
+ EXPECT_EQ(FormFetcher::State::NOT_WAITING, form_fetcher_->GetState()); |
+ EXPECT_TRUE(form_fetcher_->DidCompleteQueryingSuppressedHTTPSForms()); |
+ ASSERT_TRUE(https_form_fetcher_ptr); |
+ |
+ https_form_fetcher_ptr->OnGetPasswordStoreResults( |
+ MakeResults({suppressed_https_form1, suppressed_https_form2})); |
+ |
+ EXPECT_EQ(FormFetcher::State::NOT_WAITING, form_fetcher_->GetState()); |
+ EXPECT_TRUE(form_fetcher_->DidCompleteQueryingSuppressedHTTPSForms()); |
+ EXPECT_THAT(form_fetcher_->GetSuppressedHTTPSForms(), |
+ UnorderedElementsAre(Pointee(suppressed_https_form1), |
+ Pointee(suppressed_https_form2))); |
+ |
+ // Ensure that calling Fetch() does not wipe (even temporarily) the previously |
+ // fetched list of suppressed HTTPS credentials. Stale is better than nothing. |
+ |
+ Fetch(); |
+ |
+ EXPECT_TRUE(form_fetcher_->DidCompleteQueryingSuppressedHTTPSForms()); |
+ EXPECT_EQ(FormFetcher::State::WAITING, form_fetcher_->GetState()); |
+ EXPECT_THAT(form_fetcher_->GetSuppressedHTTPSForms(), |
+ UnorderedElementsAre(Pointee(suppressed_https_form1), |
+ Pointee(suppressed_https_form2))); |
+ |
+ EXPECT_CALL(*mock_store_, GetLogins(https_version_of_form_digest, _)); |
+ EXPECT_CALL(consumer_, ProcessMatches(IsEmpty(), 0u)); |
+ |
+ form_fetcher_->OnGetPasswordStoreResults( |
+ MakeResults(std::vector<PasswordForm>())); |
+ |
+ EXPECT_TRUE(form_fetcher_->DidCompleteQueryingSuppressedHTTPSForms()); |
+ EXPECT_THAT(form_fetcher_->GetSuppressedHTTPSForms(), |
+ UnorderedElementsAre(Pointee(suppressed_https_form1), |
+ Pointee(suppressed_https_form2))); |
+ |
+ testing::Mock::VerifyAndClearExpectations(&consumer_); |
+ testing::Mock::VerifyAndClearExpectations(mock_store_.get()); |
+ |
+ // Destroy FormFetcher while SuppressedHTTPSFormFetcher is busy. |
+ form_fetcher_.reset(); |
+} |
+ |
+// Exercise the scenario where querying the suppressed HTTPS logins takes so |
+// long that in the meantime there is another call to Fetch(), which completes, |
+// and triggers fetching HTTPS logins yet again. In this case, the first |
+// SuppressedHTTPSFormFetcher is destroyed and its query cancelled. |
+TEST_F(FormFetcherImplTest, SuppressedHTTPSForms_SimultaneousQueries) { |
+ form_fetcher_ = base::MakeUnique<FormFetcherImpl>( |
+ form_digest_, &client_, false /* should_migrate_http_passwords */, |
+ true /* should_query_suppressed_https_forms */); |
+ EXPECT_CALL(consumer_, ProcessMatches(IsEmpty(), 0u)); |
+ form_fetcher_->AddConsumer(&consumer_); |
+ testing::Mock::VerifyAndClearExpectations(&consumer_); |
+ |
+ Fetch(); |
+ |
+ const PasswordStore::FormDigest https_version_of_form_digest( |
+ PasswordForm::SCHEME_HTML, kTestHttpsRealm, GURL(kTestHttpsURL)); |
+ base::WeakPtr<PasswordStoreConsumer> https_form_fetcher_ptr1; |
+ EXPECT_CALL(*mock_store_, GetLogins(https_version_of_form_digest, _)) |
+ .WillOnce(::WithArg<1>(GetAndAssignWeakPtr(&https_form_fetcher_ptr1))); |
+ EXPECT_CALL(consumer_, ProcessMatches(IsEmpty(), 0u)); |
+ |
+ form_fetcher_->OnGetPasswordStoreResults( |
+ MakeResults(std::vector<PasswordForm>())); |
+ |
+ testing::Mock::VerifyAndClearExpectations(&consumer_); |
+ testing::Mock::VerifyAndClearExpectations(mock_store_.get()); |
+ EXPECT_TRUE(https_form_fetcher_ptr1); |
+ |
+ Fetch(); |
+ |
+ PasswordStoreConsumer* https_form_fetcher_ptr2 = nullptr; |
+ EXPECT_CALL(*mock_store_, GetLogins(https_version_of_form_digest, _)) |
+ .WillOnce(::testing::SaveArg<1>(&https_form_fetcher_ptr2)); |
+ EXPECT_CALL(consumer_, ProcessMatches(IsEmpty(), 0u)); |
+ |
+ form_fetcher_->OnGetPasswordStoreResults( |
+ MakeResults(std::vector<PasswordForm>())); |
+ |
+ testing::Mock::VerifyAndClearExpectations(&consumer_); |
+ testing::Mock::VerifyAndClearExpectations(mock_store_.get()); |
+ EXPECT_FALSE(https_form_fetcher_ptr1); |
+ ASSERT_TRUE(https_form_fetcher_ptr2); |
+ EXPECT_EQ(FormFetcher::State::NOT_WAITING, form_fetcher_->GetState()); |
+ |
+ EXPECT_FALSE(form_fetcher_->DidCompleteQueryingSuppressedHTTPSForms()); |
+ EXPECT_THAT(form_fetcher_->GetSuppressedHTTPSForms(), IsEmpty()); |
+ |
+ const PasswordForm suppressed_https_form = CreateNonFederated(); |
+ https_form_fetcher_ptr2->OnGetPasswordStoreResults( |
+ MakeResults({suppressed_https_form})); |
+ |
+ EXPECT_TRUE(form_fetcher_->DidCompleteQueryingSuppressedHTTPSForms()); |
+ EXPECT_THAT(form_fetcher_->GetSuppressedHTTPSForms(), |
+ UnorderedElementsAre(Pointee(suppressed_https_form))); |
+} |
+ |
+TEST_F(FormFetcherImplTest, SuppressedHTTPSForms_NotQueriedForHTTPSOrigins) { |
+ form_digest_ = PasswordStore::FormDigest( |
+ PasswordForm::SCHEME_HTML, kTestHttpsRealm, GURL(kTestHttpsURL)); |
+ form_fetcher_ = base::MakeUnique<FormFetcherImpl>( |
+ form_digest_, &client_, false /* should_migrate_http_passwords */, |
+ true /* should_query_suppressed_https_forms */); |
+ EXPECT_CALL(consumer_, ProcessMatches(IsEmpty(), 0u)); |
+ form_fetcher_->AddConsumer(&consumer_); |
+ testing::Mock::VerifyAndClearExpectations(&consumer_); |
+ |
+ Fetch(); |
+ |
+ EXPECT_CALL(*mock_store_, GetLogins(_, _)).Times(0); |
+ EXPECT_CALL(consumer_, ProcessMatches(IsEmpty(), 0u)); |
+ |
+ form_fetcher_->OnGetPasswordStoreResults( |
+ MakeResults(std::vector<PasswordForm>())); |
+ |
+ EXPECT_EQ(FormFetcher::State::NOT_WAITING, form_fetcher_->GetState()); |
+ EXPECT_FALSE(form_fetcher_->DidCompleteQueryingSuppressedHTTPSForms()); |
+} |
+ |
// Cloning a FormFetcherImpl with empty results should result in an |
// instance with empty results. |
TEST_F(FormFetcherImplTest, Clone_EmptyResults) { |
@@ -564,6 +749,7 @@ TEST_F(FormFetcherImplTest, Clone_EmptyResults) { |
EXPECT_EQ(FormFetcher::State::NOT_WAITING, clone->GetState()); |
EXPECT_THAT(clone->GetInteractionsStats(), IsEmpty()); |
EXPECT_THAT(clone->GetFederatedMatches(), IsEmpty()); |
+ EXPECT_THAT(clone->GetSuppressedHTTPSForms(), IsEmpty()); |
MockConsumer consumer; |
EXPECT_CALL(consumer, ProcessMatches(IsEmpty(), 0u)); |
clone->AddConsumer(&consumer); |
@@ -615,6 +801,19 @@ TEST_F(FormFetcherImplTest, Clone_Stats) { |
EXPECT_EQ(1u, clone->GetInteractionsStats().size()); |
} |
+// Cloning a FormFetcherImpl with some suppressed HTTPS credentials should |
+// result in an instance with the same suppressed credentials. |
+TEST_F(FormFetcherImplTest, Clone_SuppressedHTTPSCredentials) { |
+ Fetch(); |
+ form_fetcher_->OnGetPasswordStoreResults( |
+ std::vector<std::unique_ptr<PasswordForm>>()); |
+ form_fetcher_->ProcessSuppressedHTTPSForms( |
+ MakeResults({CreateNonFederated()})); |
+ |
+ auto clone = form_fetcher_->Clone(); |
+ EXPECT_EQ(1u, clone->GetSuppressedHTTPSForms().size()); |
+} |
+ |
// Check that removing consumers stops them from receiving store updates. |
TEST_F(FormFetcherImplTest, RemoveConsumer) { |
Fetch(); |