Removing metrics PasswordManager.SuppressedAccount*
Bug: 947058
Change-Id: Ia58ce331fb40146aaf5877809a26ec4528daadb9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1543630
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#646755}diff --git a/chrome/browser/password_manager/password_store_x.cc b/chrome/browser/password_manager/password_store_x.cc
index f34a00c..a13873578 100644
--- a/chrome/browser/password_manager/password_store_x.cc
+++ b/chrome/browser/password_manager/password_store_x.cc
@@ -307,13 +307,6 @@
return std::vector<std::unique_ptr<PasswordForm>>();
}
-std::vector<std::unique_ptr<PasswordForm>>
-PasswordStoreX::FillLoginsForSameOrganizationName(
- const std::string& signon_realm) {
- // Not available on X.
- return std::vector<std::unique_ptr<PasswordForm>>();
-}
-
bool PasswordStoreX::FillAutofillableLogins(
std::vector<std::unique_ptr<PasswordForm>>* forms) {
CheckMigration();
diff --git a/chrome/browser/password_manager/password_store_x.h b/chrome/browser/password_manager/password_store_x.h
index a783fef..6dc58132 100644
--- a/chrome/browser/password_manager/password_store_x.h
+++ b/chrome/browser/password_manager/password_store_x.h
@@ -174,8 +174,6 @@
const base::Callback<bool(const GURL&)>& origin_filter) override;
std::vector<std::unique_ptr<autofill::PasswordForm>> FillMatchingLogins(
const FormDigest& form) override;
- std::vector<std::unique_ptr<autofill::PasswordForm>>
- FillLoginsForSameOrganizationName(const std::string& signon_realm) override;
bool FillAutofillableLogins(
std::vector<std::unique_ptr<autofill::PasswordForm>>* forms) override;
bool FillBlacklistLogins(
diff --git a/components/password_manager/core/browser/BUILD.gn b/components/password_manager/core/browser/BUILD.gn
index c8e6692..66729db 100644
--- a/components/password_manager/core/browser/BUILD.gn
+++ b/components/password_manager/core/browser/BUILD.gn
@@ -165,8 +165,6 @@
"statistics_table.h",
"store_metrics_reporter.cc",
"store_metrics_reporter.h",
- "suppressed_form_fetcher.cc",
- "suppressed_form_fetcher.h",
"sync/password_data_type_controller.cc",
"sync/password_data_type_controller.h",
"sync/password_model_type_controller.cc",
@@ -467,7 +465,6 @@
"sql_table_builder_unittest.cc",
"statistics_table_unittest.cc",
"store_metrics_reporter_unittest.cc",
- "suppressed_form_fetcher_unittest.cc",
"sync/password_sync_bridge_unittest.cc",
"sync/password_syncable_service_unittest.cc",
"sync_credentials_filter_unittest.cc",
diff --git a/components/password_manager/core/browser/credential_manager_impl.cc b/components/password_manager/core/browser/credential_manager_impl.cc
index 363f5fc..ab96f76 100644
--- a/components/password_manager/core/browser/credential_manager_impl.cc
+++ b/components/password_manager/core/browser/credential_manager_impl.cc
@@ -62,7 +62,7 @@
// without fetching of suppressed HTTPS credentials on HTTP origins as the API
// is only available on HTTPS origins.
auto form_fetcher = std::make_unique<FormFetcherImpl>(
- PasswordStore::FormDigest(*observed_form), client_, false, false);
+ PasswordStore::FormDigest(*observed_form), client_, false);
form_manager_ = std::make_unique<CredentialManagerPasswordFormManager>(
client_, *observed_form, std::move(form), this, nullptr,
std::move(form_fetcher));
diff --git a/components/password_manager/core/browser/fake_form_fetcher.cc b/components/password_manager/core/browser/fake_form_fetcher.cc
index f7507c2..a6421ad3 100644
--- a/components/password_manager/core/browser/fake_form_fetcher.cc
+++ b/components/password_manager/core/browser/fake_form_fetcher.cc
@@ -49,25 +49,6 @@
return blacklisted_;
}
-const std::vector<const PasswordForm*>&
-FakeFormFetcher::GetSuppressedHTTPSForms() const {
- return suppressed_https_forms_;
-}
-
-const std::vector<const autofill::PasswordForm*>&
-FakeFormFetcher::GetSuppressedPSLMatchingForms() const {
- return suppressed_psl_matching_forms_;
-}
-
-const std::vector<const autofill::PasswordForm*>&
-FakeFormFetcher::GetSuppressedSameOrganizationNameForms() const {
- return suppressed_same_organization_name_forms_;
-}
-
-bool FakeFormFetcher::DidCompleteQueryingSuppressedForms() const {
- return did_complete_querying_suppressed_forms_;
-}
-
void FakeFormFetcher::SetNonFederated(
const std::vector<const autofill::PasswordForm*>& non_federated) {
non_federated_ = non_federated;
diff --git a/components/password_manager/core/browser/fake_form_fetcher.h b/components/password_manager/core/browser/fake_form_fetcher.h
index 4367187..4a36222 100644
--- a/components/password_manager/core/browser/fake_form_fetcher.h
+++ b/components/password_manager/core/browser/fake_form_fetcher.h
@@ -63,39 +63,6 @@
federated_ = federated;
}
- const std::vector<const autofill::PasswordForm*>& GetSuppressedHTTPSForms()
- const override;
-
- // The pointees in |suppressed_forms| must outlive the fetcher.
- void set_suppressed_https_forms(
- const std::vector<const autofill::PasswordForm*>& suppressed_forms) {
- suppressed_https_forms_ = suppressed_forms;
- }
-
- const std::vector<const autofill::PasswordForm*>&
- GetSuppressedPSLMatchingForms() const override;
-
- // The pointees in |suppressed_forms| must outlive the fetcher.
- void set_suppressed_psl_matching_forms(
- const std::vector<const autofill::PasswordForm*>& suppressed_forms) {
- suppressed_psl_matching_forms_ = suppressed_forms;
- }
-
- const std::vector<const autofill::PasswordForm*>&
- GetSuppressedSameOrganizationNameForms() const override;
-
- // The pointees in |suppressed_forms| must outlive the fetcher.
- void set_suppressed_same_organization_name_forms(
- const std::vector<const autofill::PasswordForm*>& suppressed_forms) {
- suppressed_same_organization_name_forms_ = suppressed_forms;
- }
-
- bool DidCompleteQueryingSuppressedForms() const override;
-
- void set_did_complete_querying_suppressed_forms(bool value) {
- did_complete_querying_suppressed_forms_ = value;
- }
-
void SetNonFederated(
const std::vector<const autofill::PasswordForm*>& non_federated);
@@ -117,11 +84,6 @@
std::vector<const autofill::PasswordForm*> non_federated_;
std::vector<const autofill::PasswordForm*> federated_;
std::vector<const autofill::PasswordForm*> blacklisted_;
- std::vector<const autofill::PasswordForm*> suppressed_https_forms_;
- std::vector<const autofill::PasswordForm*> suppressed_psl_matching_forms_;
- std::vector<const autofill::PasswordForm*>
- suppressed_same_organization_name_forms_;
- bool did_complete_querying_suppressed_forms_ = false;
DISALLOW_COPY_AND_ASSIGN(FakeFormFetcher);
};
diff --git a/components/password_manager/core/browser/form_fetcher.h b/components/password_manager/core/browser/form_fetcher.h
index 25a0ad1..32d43984 100644
--- a/components/password_manager/core/browser/form_fetcher.h
+++ b/components/password_manager/core/browser/form_fetcher.h
@@ -76,42 +76,6 @@
virtual const std::vector<const autofill::PasswordForm*>&
GetBlacklistedMatches() const = 0;
- // The following accessors return various kinds of `suppressed` credentials.
- // These are stored credentials that are not (auto-)filled, because they are
- // for an origin that is similar to, but not exactly matching the origin that
- // this FormFetcher was created for. They are used for recording metrics on
- // how often such -- potentially, but not necessarily related -- credentials
- // are not offered to the user, unduly increasing log-in friction.
- //
- // There are currently three kinds of suppressed credentials:
- // 1.) HTTPS credentials not filled on the HTTP version of the origin.
- // 2.) PSL-matches that are not auto-filled (but filled on account select).
- // 3.) Same-organization name credentials, not filled.
- //
- // Results below are queried on a best-effort basis, might be somewhat stale,
- // and are available shortly after the Consumer::OnFetchCompleted callback.
-
- // When this instance fetches forms for an HTTP origin: Returns saved
- // credentials, if any, found for the HTTPS version of that origin. Empty
- // otherwise.
- virtual const std::vector<const autofill::PasswordForm*>&
- GetSuppressedHTTPSForms() const = 0;
-
- // Returns saved credentials, if any, for PSL-matching origins. Autofilling
- // these is suppressed, however, they *can be* filled on account select.
- virtual const std::vector<const autofill::PasswordForm*>&
- GetSuppressedPSLMatchingForms() const = 0;
-
- // Returns saved credentials, if any, found for HTTP/HTTPS origins with the
- // same organization name as the origin this FormFetcher was created for.
- virtual const std::vector<const autofill::PasswordForm*>&
- GetSuppressedSameOrganizationNameForms() const = 0;
-
- // Whether querying suppressed forms (of all flavors) was attempted and did
- // complete at least once during the lifetime of this instance, regardless of
- // whether there have been any results.
- virtual bool DidCompleteQueryingSuppressedForms() const = 0;
-
// Fetches stored matching logins. In addition the statistics is fetched on
// platforms with the password bubble. This is called automatically during
// construction and can be called manually later as well to cause an update
diff --git a/components/password_manager/core/browser/form_fetcher_impl.cc b/components/password_manager/core/browser/form_fetcher_impl.cc
index 83f810e5..964028bb 100644
--- a/components/password_manager/core/browser/form_fetcher_impl.cc
+++ b/components/password_manager/core/browser/form_fetcher_impl.cc
@@ -62,43 +62,6 @@
return matches;
}
-void SplitSuppressedFormsAndAssignTo(
- const PasswordStore::FormDigest& observed_form_digest,
- std::vector<std::unique_ptr<PasswordForm>> suppressed_forms,
- std::vector<std::unique_ptr<PasswordForm>>* same_origin_https_forms,
- std::vector<std::unique_ptr<PasswordForm>>* psl_matching_forms,
- std::vector<std::unique_ptr<PasswordForm>>* same_organization_name_forms) {
- DCHECK(same_origin_https_forms);
- DCHECK(psl_matching_forms);
- DCHECK(same_organization_name_forms);
- same_origin_https_forms->clear();
- psl_matching_forms->clear();
- same_organization_name_forms->clear();
- for (auto& form : suppressed_forms) {
- switch (GetMatchResult(*form, observed_form_digest)) {
- case MatchResult::PSL_MATCH:
- psl_matching_forms->push_back(std::move(form));
- break;
- case MatchResult::NO_MATCH:
- if (form->origin.host() != observed_form_digest.origin.host()) {
- same_organization_name_forms->push_back(std::move(form));
- } else if (form->origin.SchemeIs(url::kHttpsScheme) &&
- observed_form_digest.origin.SchemeIs(url::kHttpScheme)) {
- same_origin_https_forms->push_back(std::move(form));
- } else {
- // HTTP form suppressed on HTTPS observed page: The HTTP->HTTPS
- // migration can leave tons of such HTTP forms behind, ignore these.
- }
- break;
- case MatchResult::EXACT_MATCH:
- case MatchResult::FEDERATED_MATCH:
- case MatchResult::FEDERATED_PSL_MATCH:
- NOTREACHED() << "Suppressed match cannot be exact or federated.";
- break;
- }
- }
-}
-
// Create a vector of const PasswordForm from a vector of
// unique_ptr<PasswordForm> by applying get() item-wise.
std::vector<const PasswordForm*> MakeWeakCopies(
@@ -126,12 +89,10 @@
FormFetcherImpl::FormFetcherImpl(PasswordStore::FormDigest form_digest,
const PasswordManagerClient* client,
- bool should_migrate_http_passwords,
- bool should_query_suppressed_forms)
+ bool should_migrate_http_passwords)
: form_digest_(std::move(form_digest)),
client_(client),
- should_migrate_http_passwords_(should_migrate_http_passwords),
- should_query_suppressed_forms_(should_query_suppressed_forms) {}
+ should_migrate_http_passwords_(should_migrate_http_passwords) {}
FormFetcherImpl::~FormFetcherImpl() = default;
@@ -171,25 +132,6 @@
return weak_blacklisted_;
}
-const std::vector<const PasswordForm*>&
-FormFetcherImpl::GetSuppressedHTTPSForms() const {
- return weak_suppressed_same_origin_https_forms_;
-}
-
-const std::vector<const PasswordForm*>&
-FormFetcherImpl::GetSuppressedPSLMatchingForms() const {
- return weak_suppressed_psl_matching_forms_;
-}
-
-const std::vector<const PasswordForm*>&
-FormFetcherImpl::GetSuppressedSameOrganizationNameForms() const {
- return weak_suppressed_same_organization_name_forms_;
-}
-
-bool FormFetcherImpl::DidCompleteQueryingSuppressedForms() const {
- return did_complete_querying_suppressed_forms_;
-}
-
void FormFetcherImpl::OnGetPasswordStoreResults(
std::vector<std::unique_ptr<PasswordForm>> results) {
DCHECK_EQ(State::WAITING, state_);
@@ -210,17 +152,6 @@
logger->LogNumber(Logger::STRING_NUMBER_RESULTS, results.size());
}
- // Kick off the discovery of suppressed credentials, regardless of whether
- // there are some precisely matching |results|. These results are used only
- // for recording metrics at PasswordFormManager desctruction time, this is why
- // they are requested this late.
- if (should_query_suppressed_forms_ &&
- form_digest_.scheme == PasswordForm::SCHEME_HTML &&
- GURL(form_digest_.signon_realm).SchemeIsHTTPOrHTTPS()) {
- suppressed_form_fetcher_ = std::make_unique<SuppressedFormFetcher>(
- form_digest_.signon_realm, client_, this);
- }
-
if (should_migrate_http_passwords_ && results.empty() &&
form_digest_.origin.SchemeIs(url::kHttpsScheme)) {
http_migrator_ = std::make_unique<HttpPasswordStoreMigrator>(
@@ -243,21 +174,6 @@
ProcessPasswordStoreResults(std::move(forms));
}
-void FormFetcherImpl::ProcessSuppressedForms(
- std::vector<std::unique_ptr<autofill::PasswordForm>> forms) {
- did_complete_querying_suppressed_forms_ = true;
- SplitSuppressedFormsAndAssignTo(form_digest_, std::move(forms),
- &suppressed_same_origin_https_forms_,
- &suppressed_psl_matching_forms_,
- &suppressed_same_organization_name_forms_);
- weak_suppressed_same_origin_https_forms_ =
- MakeWeakCopies(suppressed_same_origin_https_forms_);
- weak_suppressed_psl_matching_forms_ =
- MakeWeakCopies(suppressed_psl_matching_forms_);
- weak_suppressed_same_organization_name_forms_ =
- MakeWeakCopies(suppressed_same_organization_name_forms_);
-}
-
void FormFetcherImpl::Fetch() {
std::unique_ptr<BrowserSavePasswordProgressLogger> logger;
if (password_manager_util::IsLoggingActive(client_)) {
@@ -296,8 +212,7 @@
std::unique_ptr<FormFetcher> FormFetcherImpl::Clone() {
// Create the copy without the "HTTPS migration" activated. If it was needed,
// then it was done by |this| already.
- auto result = std::make_unique<FormFetcherImpl>(
- form_digest_, client_, false, should_query_suppressed_forms_);
+ auto result = std::make_unique<FormFetcherImpl>(form_digest_, client_, false);
if (state_ != State::NOT_WAITING) {
// There are no store results to copy, trigger a Fetch on the clone instead.
@@ -309,22 +224,10 @@
result->federated_ = MakeCopies(this->federated_);
result->blacklisted_ = MakeCopies(this->blacklisted_);
result->interactions_stats_ = this->interactions_stats_;
- result->suppressed_same_origin_https_forms_ =
- MakeCopies(this->suppressed_same_origin_https_forms_);
- result->suppressed_psl_matching_forms_ =
- MakeCopies(this->suppressed_psl_matching_forms_);
- result->suppressed_same_organization_name_forms_ =
- MakeCopies(this->suppressed_same_organization_name_forms_);
result->weak_non_federated_ = MakeWeakCopies(result->non_federated_);
result->weak_federated_ = MakeWeakCopies(result->federated_);
result->weak_blacklisted_ = MakeWeakCopies(result->blacklisted_);
- result->weak_suppressed_same_origin_https_forms_ =
- MakeWeakCopies(result->suppressed_same_origin_https_forms_);
- result->weak_suppressed_psl_matching_forms_ =
- MakeWeakCopies(result->suppressed_psl_matching_forms_);
- result->weak_suppressed_same_organization_name_forms_ =
- MakeWeakCopies(result->suppressed_same_organization_name_forms_);
result->state_ = this->state_;
result->need_to_refetch_ = this->need_to_refetch_;
diff --git a/components/password_manager/core/browser/form_fetcher_impl.h b/components/password_manager/core/browser/form_fetcher_impl.h
index b9ac4b2..c3d7d7e 100644
--- a/components/password_manager/core/browser/form_fetcher_impl.h
+++ b/components/password_manager/core/browser/form_fetcher_impl.h
@@ -14,7 +14,6 @@
#include "components/password_manager/core/browser/http_password_store_migrator.h"
#include "components/password_manager/core/browser/password_store.h"
#include "components/password_manager/core/browser/password_store_consumer.h"
-#include "components/password_manager/core/browser/suppressed_form_fetcher.h"
namespace password_manager {
@@ -24,15 +23,13 @@
// with a particular origin.
class FormFetcherImpl : public FormFetcher,
public PasswordStoreConsumer,
- public HttpPasswordStoreMigrator::Consumer,
- public SuppressedFormFetcher::Consumer {
+ public HttpPasswordStoreMigrator::Consumer {
public:
// |form_digest| describes what credentials need to be retrieved and
// |client| serves the PasswordStore, the logging information etc.
FormFetcherImpl(PasswordStore::FormDigest form_digest,
const PasswordManagerClient* client,
- bool should_migrate_http_passwords,
- bool should_query_suppressed_forms);
+ bool should_migrate_http_passwords);
~FormFetcherImpl() override;
@@ -47,13 +44,6 @@
const override;
const std::vector<const autofill::PasswordForm*>& GetBlacklistedMatches()
const override;
- const std::vector<const autofill::PasswordForm*>& GetSuppressedHTTPSForms()
- const override;
- const std::vector<const autofill::PasswordForm*>&
- GetSuppressedPSLMatchingForms() const override;
- const std::vector<const autofill::PasswordForm*>&
- GetSuppressedSameOrganizationNameForms() const override;
- bool DidCompleteQueryingSuppressedForms() const override;
void Fetch() override;
std::unique_ptr<FormFetcher> Clone() override;
@@ -66,10 +56,6 @@
void ProcessMigratedForms(
std::vector<std::unique_ptr<autofill::PasswordForm>> forms) override;
- // SuppressedFormFetcher::Consumer:
- void ProcessSuppressedForms(
- std::vector<std::unique_ptr<autofill::PasswordForm>> forms) override;
-
private:
// Processes password form results and forwards them to the |consumers_|.
void ProcessPasswordStoreResults(
@@ -92,29 +78,11 @@
// Statistics for the current domain.
std::vector<InteractionsStats> interactions_stats_;
- std::vector<std::unique_ptr<autofill::PasswordForm>>
- suppressed_same_origin_https_forms_;
- std::vector<std::unique_ptr<autofill::PasswordForm>>
- suppressed_psl_matching_forms_;
- std::vector<std::unique_ptr<autofill::PasswordForm>>
- suppressed_same_organization_name_forms_;
-
- // Whether querying |suppressed_https_forms_| was attempted and did complete
- // at least once during the lifetime of this instance, regardless of whether
- // there have been any results.
- bool did_complete_querying_suppressed_forms_ = false;
-
// Non-owning copies of the vectors above.
// TODO(https://crbug.com/945864): Clean this up.
std::vector<const autofill::PasswordForm*> weak_non_federated_;
std::vector<const autofill::PasswordForm*> weak_federated_;
std::vector<const autofill::PasswordForm*> weak_blacklisted_;
- std::vector<const autofill::PasswordForm*>
- weak_suppressed_same_origin_https_forms_;
- std::vector<const autofill::PasswordForm*>
- weak_suppressed_psl_matching_forms_;
- std::vector<const autofill::PasswordForm*>
- weak_suppressed_same_organization_name_forms_;
// Consumers of the fetcher, all are assumed to outlive |this|.
std::set<FormFetcher::Consumer*> consumers_;
@@ -132,18 +100,9 @@
// Indicates whether HTTP passwords should be migrated to HTTPS.
const bool should_migrate_http_passwords_;
- // Indicates whether to query suppressed forms.
- const bool should_query_suppressed_forms_;
-
// Does the actual migration.
std::unique_ptr<HttpPasswordStoreMigrator> http_migrator_;
- // Responsible for looking up `suppressed` credentials. These are stored
- // credentials that were not filled, even though they might be related to the
- // origin that this instance was created for. Look-up happens asynchronously,
- // without blocking Consumer::OnFetchCompleted.
- std::unique_ptr<SuppressedFormFetcher> suppressed_form_fetcher_;
-
DISALLOW_COPY_AND_ASSIGN(FormFetcherImpl);
};
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 3c22edfd..7850558 100644
--- a/components/password_manager/core/browser/form_fetcher_impl_unittest.cc
+++ b/components/password_manager/core/browser/form_fetcher_impl_unittest.cc
@@ -50,12 +50,6 @@
constexpr const char kTestHttpsURL[] = "https://example.in/";
constexpr const char kTestHttpsActionURL[] = "https://login.example.org/";
-constexpr const char kTestPSLMatchingHttpURL[] = "http://psl.example.in/";
-constexpr const char kTestPSLMatchingHttpsURL[] = "https://psl.example.in/";
-
-constexpr const char kTestHttpSameOrgNameURL[] = "http://sub.example.com/";
-constexpr const char kTestHttpsSameOrgNameURL[] = "https://sub.example.com/";
-
constexpr const char kTestFederatedRealm[] =
"federation://example.in/accounts.google.com";
constexpr const char kTestFederationURL[] = "https://accounts.google.com/";
@@ -175,15 +169,6 @@
return results;
}
-std::vector<PasswordForm> PointeeValues(
- const std::vector<const PasswordForm*> forms) {
- std::vector<PasswordForm> result;
- result.reserve(forms.size());
- for (const PasswordForm* form : forms)
- result.push_back(*form);
- return result;
-}
-
ACTION_P(GetAndAssignWeakPtr, ptr) {
*ptr = arg0->GetWeakPtr();
}
@@ -201,8 +186,7 @@
client_.set_store(mock_store_.get());
form_fetcher_ = std::make_unique<FormFetcherImpl>(
- form_digest_, &client_, false /* should_migrate_http_passwords */,
- false /* should_query_suppressed_https_forms */);
+ form_digest_, &client_, false /* should_migrate_http_passwords */);
}
~FormFetcherImplTest() override { mock_store_->ShutdownOnUIThread(); }
@@ -220,57 +204,6 @@
testing::Mock::VerifyAndClearExpectations(mock_store_.get());
}
- void RecreateFormFetcherWithQueryingSuppressedForms() {
- form_fetcher_ = std::make_unique<FormFetcherImpl>(
- form_digest_, &client_, false /* should_migrate_http_passwords */,
- true /* should_query_suppressed_https_forms */);
- EXPECT_CALL(consumer_, OnFetchCompleted);
- form_fetcher_->AddConsumer(&consumer_);
- testing::Mock::VerifyAndClearExpectations(&consumer_);
- }
-
- // Simulates a call to Fetch(), and supplies |simulated_matches| as the
- // PasswordStore results. Expects that this will trigger the querying of
- // suppressed forms by means of a GetLoginsForSameOrganizationName call
- // being issued against the |expected_signon_realm|.
- //
- // Call CompleteQueryingSuppressedForms with the emitted |consumer_ptr|
- // to complete the query.
- void SimulateFetchAndExpectQueryingSuppressedForms(
- const std::vector<PasswordForm>& simulated_get_logins_matches,
- const std::string& expected_signon_realm,
- base::WeakPtr<PasswordStoreConsumer>* consumer_ptr /* out */) {
- ASSERT_EQ(FormFetcher::State::NOT_WAITING, form_fetcher_->GetState());
-
- Fetch();
-
- EXPECT_CALL(*mock_store_,
- GetLoginsForSameOrganizationName(expected_signon_realm, _))
- .WillOnce(::testing::WithArg<1>(GetAndAssignWeakPtr(consumer_ptr)));
- const size_t num_matches = simulated_get_logins_matches.size();
- EXPECT_CALL(consumer_, OnFetchCompleted);
-
- form_fetcher_->OnGetPasswordStoreResults(
- MakeResults(simulated_get_logins_matches));
- EXPECT_THAT(form_fetcher_->GetNonFederatedMatches(),
- ::testing::SizeIs(num_matches));
-
- ASSERT_TRUE(testing::Mock::VerifyAndClearExpectations(&consumer_));
- ASSERT_TRUE(testing::Mock::VerifyAndClearExpectations(mock_store_.get()));
- ASSERT_EQ(FormFetcher::State::NOT_WAITING, form_fetcher_->GetState());
- ASSERT_TRUE(*consumer_ptr);
- }
-
- void CompleteQueryingSuppressedForms(
- const std::vector<PasswordForm>& simulated_suppressed_forms,
- base::WeakPtr<PasswordStoreConsumer> consumer_ptr) {
- ASSERT_TRUE(consumer_ptr);
- ASSERT_EQ(FormFetcher::State::NOT_WAITING, form_fetcher_->GetState());
- consumer_ptr->OnGetPasswordStoreResults(
- MakeResults(simulated_suppressed_forms));
- ASSERT_EQ(FormFetcher::State::NOT_WAITING, form_fetcher_->GetState());
- }
-
base::test::ScopedTaskEnvironment scoped_task_environment_;
PasswordStore::FormDigest form_digest_;
std::unique_ptr<FormFetcherImpl> form_fetcher_;
@@ -514,8 +447,7 @@
// A new form fetcher is created to be able to set the form digest and
// migration flag.
form_fetcher_ = std::make_unique<FormFetcherImpl>(
- form_digest_, &client_, true /* should_migrate_http_passwords */,
- false /* should_query_suppressed_https_forms */);
+ form_digest_, &client_, true /* should_migrate_http_passwords */);
EXPECT_CALL(consumer_, OnFetchCompleted);
form_fetcher_->AddConsumer(&consumer_);
@@ -563,8 +495,7 @@
// A new form fetcher is created to be able to set the form digest and
// migration flag.
form_fetcher_ = std::make_unique<FormFetcherImpl>(
- form_digest_, &client_, true /* should_migrate_http_passwords */,
- false /* should_query_suppressed_https_forms */);
+ form_digest_, &client_, true /* should_migrate_http_passwords */);
EXPECT_CALL(consumer_, OnFetchCompleted);
form_fetcher_->AddConsumer(&consumer_);
@@ -639,8 +570,7 @@
// A new form fetcher is created to be able to set the form digest and
// migration flag.
form_fetcher_ = std::make_unique<FormFetcherImpl>(
- form_digest_, &client_, true /* should_migrate_http_passwords */,
- false /* should_query_suppressed_https_forms */);
+ form_digest_, &client_, true /* should_migrate_http_passwords */);
PasswordForm https_form = CreateNonFederated();
@@ -681,229 +611,20 @@
EXPECT_EQ(FormFetcher::State::NOT_WAITING, form_fetcher_->GetState());
}
-TEST_F(FormFetcherImplTest, SuppressedForms_QueriedForHTTPAndHTTPSOrigins) {
- static const PasswordStore::FormDigest kObservedHTTPSFormDigest(
- PasswordForm::SCHEME_HTML, kTestHttpsURL, GURL(kTestHttpsURL));
-
- static const PasswordForm kFormHttpSameHost =
- CreateHTMLForm(kTestHttpURL, "user_1", "pass_1");
- static const PasswordForm kFormHttpsSameHost =
- CreateHTMLForm(kTestHttpsURL, "user_2", "pass_2");
- static const PasswordForm kFormHttpPSLMatchingHost =
- CreateHTMLForm(kTestPSLMatchingHttpURL, "user_3", "pass_3");
- static const PasswordForm kFormHttpsPSLMatchingHost =
- CreateHTMLForm(kTestPSLMatchingHttpsURL, "user_4", "pass_4");
- static const PasswordForm kFormHttpSameOrgNameHost =
- CreateHTMLForm(kTestHttpSameOrgNameURL, "user_5", "pass_5");
- static const PasswordForm kFormHttpsSameOrgNameHost =
- CreateHTMLForm(kTestHttpsSameOrgNameURL, "user_6", "pass_6");
-
- static const struct {
- const char* observed_form_origin;
- const char* observed_form_realm;
- std::vector<PasswordForm> matching_forms;
- std::vector<PasswordForm> all_suppressed_forms;
- std::vector<PasswordForm> expected_suppressed_https_forms;
- std::vector<PasswordForm> expected_suppressed_psl_forms;
- std::vector<PasswordForm> expected_suppressed_same_org_name_forms;
- } kTestCases[] = {
- {kTestHttpURL,
- kTestHttpURL,
- {kFormHttpSameHost},
- {kFormHttpsSameHost, kFormHttpPSLMatchingHost, kFormHttpsPSLMatchingHost,
- kFormHttpSameOrgNameHost, kFormHttpsSameOrgNameHost},
- {kFormHttpsSameHost},
- {kFormHttpPSLMatchingHost},
- {kFormHttpsPSLMatchingHost, kFormHttpSameOrgNameHost,
- kFormHttpsSameOrgNameHost}},
-
- {kTestHttpsURL,
- kTestHttpsURL,
- {kFormHttpsSameHost},
- {kFormHttpSameHost, kFormHttpPSLMatchingHost, kFormHttpsPSLMatchingHost,
- kFormHttpSameOrgNameHost, kFormHttpsSameOrgNameHost},
- std::vector<PasswordForm>(),
- {kFormHttpsPSLMatchingHost},
- {kFormHttpPSLMatchingHost, kFormHttpSameOrgNameHost,
- kFormHttpsSameOrgNameHost}},
- };
-
- for (const auto& test_case : kTestCases) {
- SCOPED_TRACE(test_case.observed_form_origin);
-
- form_digest_ = PasswordStore::FormDigest(
- PasswordForm::SCHEME_HTML, test_case.observed_form_origin,
- GURL(test_case.observed_form_origin));
- RecreateFormFetcherWithQueryingSuppressedForms();
-
- // The matching PasswordStore results coming in should trigger another
- // GetLogins request to fetcht the suppressed forms.
- base::WeakPtr<PasswordStoreConsumer> suppressed_form_fetcher_ptr = nullptr;
- ASSERT_NO_FATAL_FAILURE(SimulateFetchAndExpectQueryingSuppressedForms(
- test_case.matching_forms, test_case.observed_form_realm,
- &suppressed_form_fetcher_ptr));
-
- EXPECT_FALSE(form_fetcher_->DidCompleteQueryingSuppressedForms());
- EXPECT_THAT(form_fetcher_->GetSuppressedHTTPSForms(), IsEmpty());
-
- ASSERT_NO_FATAL_FAILURE(CompleteQueryingSuppressedForms(
- test_case.all_suppressed_forms, suppressed_form_fetcher_ptr));
-
- EXPECT_TRUE(form_fetcher_->DidCompleteQueryingSuppressedForms());
- EXPECT_THAT(
- PointeeValues(form_fetcher_->GetSuppressedHTTPSForms()),
- UnorderedElementsAreArray(test_case.expected_suppressed_https_forms));
- EXPECT_THAT(
- PointeeValues(form_fetcher_->GetSuppressedPSLMatchingForms()),
- UnorderedElementsAreArray(test_case.expected_suppressed_psl_forms));
- EXPECT_THAT(
- PointeeValues(form_fetcher_->GetSuppressedSameOrganizationNameForms()),
- UnorderedElementsAreArray(
- test_case.expected_suppressed_same_org_name_forms));
- }
-}
-
-TEST_F(FormFetcherImplTest, SuppressedForms_RequeriedOnRefetch) {
- RecreateFormFetcherWithQueryingSuppressedForms();
-
- base::WeakPtr<PasswordStoreConsumer> https_form_fetcher_ptr = nullptr;
- ASSERT_NO_FATAL_FAILURE(SimulateFetchAndExpectQueryingSuppressedForms(
- std::vector<PasswordForm>(), kTestHttpURL, &https_form_fetcher_ptr));
- ASSERT_NO_FATAL_FAILURE(CompleteQueryingSuppressedForms(
- std::vector<PasswordForm>(), https_form_fetcher_ptr));
-
- // Another call to Fetch() should refetch the list of suppressed credentials.
- const PasswordForm suppressed_https_form = CreateNonFederated();
- ASSERT_NO_FATAL_FAILURE(SimulateFetchAndExpectQueryingSuppressedForms(
- std::vector<PasswordForm>(), kTestHttpURL, &https_form_fetcher_ptr));
- ASSERT_NO_FATAL_FAILURE(CompleteQueryingSuppressedForms(
- {suppressed_https_form}, https_form_fetcher_ptr));
-
- EXPECT_THAT(form_fetcher_->GetSuppressedHTTPSForms(),
- UnorderedElementsAre(Pointee(suppressed_https_form)));
-}
-
-TEST_F(FormFetcherImplTest, SuppressedForms_NeverWiped) {
- RecreateFormFetcherWithQueryingSuppressedForms();
-
- static const PasswordForm kFormHttpsSameHost =
- CreateHTMLForm(kTestHttpsURL, "user_1", "pass_1");
- static const PasswordForm kFormHttpPSLMatchingHost =
- CreateHTMLForm(kTestPSLMatchingHttpURL, "user_2", "pass_2");
- static const PasswordForm kFormHttpSameOrgNameHost =
- CreateHTMLForm(kTestHttpSameOrgNameURL, "user_3", "pass_3");
-
- base::WeakPtr<PasswordStoreConsumer> https_form_fetcher_ptr = nullptr;
- ASSERT_NO_FATAL_FAILURE(SimulateFetchAndExpectQueryingSuppressedForms(
- std::vector<PasswordForm>(), kTestHttpURL, &https_form_fetcher_ptr));
- ASSERT_NO_FATAL_FAILURE(CompleteQueryingSuppressedForms(
- {kFormHttpsSameHost, kFormHttpPSLMatchingHost, kFormHttpSameOrgNameHost},
- https_form_fetcher_ptr));
-
- // Ensure that calling Fetch() does not wipe (even temporarily) the previously
- // fetched list of suppressed HTTPS credentials. Stale is better than nothing.
- ASSERT_NO_FATAL_FAILURE(SimulateFetchAndExpectQueryingSuppressedForms(
- std::vector<PasswordForm>(), kTestHttpURL, &https_form_fetcher_ptr));
-
- EXPECT_TRUE(form_fetcher_->DidCompleteQueryingSuppressedForms());
- EXPECT_THAT(form_fetcher_->GetSuppressedHTTPSForms(),
- UnorderedElementsAre(Pointee(kFormHttpsSameHost)));
- EXPECT_THAT(form_fetcher_->GetSuppressedPSLMatchingForms(),
- UnorderedElementsAre(Pointee(kFormHttpPSLMatchingHost)));
- EXPECT_THAT(form_fetcher_->GetSuppressedSameOrganizationNameForms(),
- UnorderedElementsAre(Pointee(kFormHttpSameOrgNameHost)));
-}
-
-TEST_F(FormFetcherImplTest, SuppressedForms_FormFetcherDestroyedWhileQuerying) {
- RecreateFormFetcherWithQueryingSuppressedForms();
-
- base::WeakPtr<PasswordStoreConsumer> https_form_fetcher_ptr = nullptr;
- ASSERT_NO_FATAL_FAILURE(SimulateFetchAndExpectQueryingSuppressedForms(
- std::vector<PasswordForm>(), kTestHttpURL, &https_form_fetcher_ptr));
-
- EXPECT_FALSE(form_fetcher_->DidCompleteQueryingSuppressedForms());
-
- // 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 suppressed forms yet again. In this case, the
-// first SuppressedHTTPSFormFetcher is destroyed and its query cancelled.
-TEST_F(FormFetcherImplTest, SuppressedForms_SimultaneousQueries) {
- RecreateFormFetcherWithQueryingSuppressedForms();
-
- base::WeakPtr<PasswordStoreConsumer> https_form_fetcher_ptr1;
- ASSERT_NO_FATAL_FAILURE(SimulateFetchAndExpectQueryingSuppressedForms(
- std::vector<PasswordForm>(), kTestHttpURL, &https_form_fetcher_ptr1));
-
- base::WeakPtr<PasswordStoreConsumer> https_form_fetcher_ptr2;
- ASSERT_NO_FATAL_FAILURE(SimulateFetchAndExpectQueryingSuppressedForms(
- std::vector<PasswordForm>(), kTestHttpURL, &https_form_fetcher_ptr2));
-
- EXPECT_FALSE(form_fetcher_->DidCompleteQueryingSuppressedForms());
- EXPECT_THAT(form_fetcher_->GetSuppressedHTTPSForms(), IsEmpty());
- EXPECT_FALSE(https_form_fetcher_ptr1);
- ASSERT_TRUE(https_form_fetcher_ptr2);
-
- static const PasswordForm kFormHttpsSameHost =
- CreateHTMLForm(kTestHttpsURL, "user_1", "pass_1");
- static const PasswordForm kFormHttpPSLMatchingHost =
- CreateHTMLForm(kTestPSLMatchingHttpURL, "user_2", "pass_2");
- static const PasswordForm kFormHttpSameOrgNameHost =
- CreateHTMLForm(kTestHttpSameOrgNameURL, "user_3", "pass_3");
-
- ASSERT_NO_FATAL_FAILURE(CompleteQueryingSuppressedForms(
- {kFormHttpsSameHost, kFormHttpPSLMatchingHost, kFormHttpSameOrgNameHost},
- https_form_fetcher_ptr2));
-
- EXPECT_TRUE(form_fetcher_->DidCompleteQueryingSuppressedForms());
-
- EXPECT_TRUE(form_fetcher_->DidCompleteQueryingSuppressedForms());
- EXPECT_THAT(form_fetcher_->GetSuppressedHTTPSForms(),
- UnorderedElementsAre(Pointee(kFormHttpsSameHost)));
- EXPECT_THAT(form_fetcher_->GetSuppressedPSLMatchingForms(),
- UnorderedElementsAre(Pointee(kFormHttpPSLMatchingHost)));
- EXPECT_THAT(form_fetcher_->GetSuppressedSameOrganizationNameForms(),
- UnorderedElementsAre(Pointee(kFormHttpSameOrgNameHost)));
-}
-
-TEST_F(FormFetcherImplTest, SuppressedForms_NotQueriedForFederatedRealms) {
- form_digest_ = PasswordStore::FormDigest(
- PasswordForm::SCHEME_HTML, kTestFederatedRealm, GURL(kTestFederationURL));
- RecreateFormFetcherWithQueryingSuppressedForms();
- Fetch();
-
- EXPECT_CALL(*mock_store_, GetLogins(_, _)).Times(0);
- EXPECT_CALL(consumer_, OnFetchCompleted);
-
- form_fetcher_->OnGetPasswordStoreResults(
- MakeResults(std::vector<PasswordForm>()));
-
- EXPECT_EQ(FormFetcher::State::NOT_WAITING, form_fetcher_->GetState());
- EXPECT_FALSE(form_fetcher_->DidCompleteQueryingSuppressedForms());
-}
-
// Cloning a FormFetcherImpl with empty results should result in an
// instance with empty results.
TEST_F(FormFetcherImplTest, Clone_EmptyResults) {
- RecreateFormFetcherWithQueryingSuppressedForms();
Fetch();
- EXPECT_CALL(consumer_, OnFetchCompleted);
- EXPECT_CALL(*mock_store_, GetLoginsForSameOrganizationName(_, _));
form_fetcher_->OnGetPasswordStoreResults(
std::vector<std::unique_ptr<PasswordForm>>());
ASSERT_TRUE(::testing::Mock::VerifyAndClearExpectations(mock_store_.get()));
// Clone() should not cause re-fetching from PasswordStore.
EXPECT_CALL(*mock_store_, GetLogins(_, _)).Times(0);
- EXPECT_CALL(*mock_store_, GetLoginsForSameOrganizationName(_, _)).Times(0);
auto clone = form_fetcher_->Clone();
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, OnFetchCompleted);
clone->AddConsumer(&consumer);
@@ -912,7 +633,6 @@
// Cloning a FormFetcherImpl with non-empty results should result in an
// instance with the same results.
TEST_F(FormFetcherImplTest, Clone_NonEmptyResults) {
- RecreateFormFetcherWithQueryingSuppressedForms();
Fetch();
PasswordForm non_federated = CreateNonFederated();
PasswordForm federated = CreateFederated();
@@ -922,8 +642,6 @@
results.push_back(std::make_unique<PasswordForm>(federated));
results.push_back(std::make_unique<PasswordForm>(android_federated));
- EXPECT_CALL(consumer_, OnFetchCompleted);
- EXPECT_CALL(*mock_store_, GetLoginsForSameOrganizationName(_, _));
form_fetcher_->OnGetPasswordStoreResults(std::move(results));
EXPECT_THAT(form_fetcher_->GetNonFederatedMatches(),
UnorderedElementsAre(Pointee(non_federated)));
@@ -936,7 +654,6 @@
// Clone() should not cause re-fetching from PasswordStore.
EXPECT_CALL(*mock_store_, GetLogins(_, _)).Times(0);
- EXPECT_CALL(*mock_store_, GetLoginsForSameOrganizationName(_, _)).Times(0);
auto clone = form_fetcher_->Clone();
// Additionally, destroy the original FormFetcher. This should not invalidate
@@ -969,33 +686,6 @@
EXPECT_EQ(1u, clone->GetInteractionsStats().size());
}
-// Cloning a FormFetcherImpl with some suppressed credentials should
-// result in an instance with the same suppressed credentials.
-TEST_F(FormFetcherImplTest, Clone_SuppressedCredentials) {
- Fetch();
- form_fetcher_->OnGetPasswordStoreResults(
- std::vector<std::unique_ptr<PasswordForm>>());
-
- static const PasswordForm kFormHttpsSameHost =
- CreateHTMLForm(kTestHttpsURL, "user_1", "pass_1");
- static const PasswordForm kFormHttpPSLMatchingHost =
- CreateHTMLForm(kTestPSLMatchingHttpURL, "user_2", "pass_2");
- static const PasswordForm kFormHttpSameOrgNameHost =
- CreateHTMLForm(kTestHttpSameOrgNameURL, "user_3", "pass_3");
-
- form_fetcher_->ProcessSuppressedForms(
- MakeResults({kFormHttpsSameHost, kFormHttpPSLMatchingHost,
- kFormHttpSameOrgNameHost}));
-
- auto clone = form_fetcher_->Clone();
- EXPECT_THAT(PointeeValues(clone->GetSuppressedHTTPSForms()),
- UnorderedElementsAre(kFormHttpsSameHost));
- EXPECT_THAT(PointeeValues(clone->GetSuppressedPSLMatchingForms()),
- UnorderedElementsAre(kFormHttpPSLMatchingHost));
- EXPECT_THAT(PointeeValues(clone->GetSuppressedSameOrganizationNameForms()),
- UnorderedElementsAre(kFormHttpSameOrgNameHost));
-}
-
// Check that removing consumers stops them from receiving store updates.
TEST_F(FormFetcherImplTest, RemoveConsumer) {
Fetch();
diff --git a/components/password_manager/core/browser/login_database.cc b/components/password_manager/core/browser/login_database.cc
index de4f10e..5609806 100644
--- a/components/password_manager/core/browser/login_database.cc
+++ b/components/password_manager/core/browser/login_database.cc
@@ -1395,52 +1395,6 @@
return true;
}
-bool LoginDatabase::GetLoginsForSameOrganizationName(
- const std::string& signon_realm,
- std::vector<std::unique_ptr<autofill::PasswordForm>>* forms) {
- DCHECK(forms);
- forms->clear();
-
- GURL signon_realm_as_url(signon_realm);
- if (!signon_realm_as_url.SchemeIsHTTPOrHTTPS())
- return true;
-
- std::string organization_name =
- GetOrganizationIdentifyingName(signon_realm_as_url);
- if (organization_name.empty())
- return true;
-
- // SQLite does not provide a function to escape special characters, but
- // seemingly uses POSIX Extended Regular Expressions (ERE), and so does RE2.
- // In the worst case the bogus results will be filtered out below.
- static constexpr char kRESchemeAndSubdomains[] = "^https?://([\\w+%-]+\\.)*";
- static constexpr char kREDotAndEffectiveTLD[] = "(\\.[\\w+%-]+)+/$";
- const std::string signon_realms_with_same_organization_name_regexp =
- kRESchemeAndSubdomains + RE2::QuoteMeta(organization_name) +
- kREDotAndEffectiveTLD;
- sql::Statement s(db_.GetCachedStatement(
- SQL_FROM_HERE, get_same_organization_name_logins_statement_.c_str()));
- s.BindString(0, signon_realms_with_same_organization_name_regexp);
-
- PrimaryKeyToFormMap key_to_form_map;
- FormRetrievalResult result = StatementToForms(&s, nullptr, &key_to_form_map);
- for (auto& pair : key_to_form_map) {
- forms->push_back(std::move(pair.second));
- }
-
- using PasswordFormPtr = std::unique_ptr<autofill::PasswordForm>;
- base::EraseIf(*forms, [&organization_name](const PasswordFormPtr& form) {
- GURL candidate_signon_realm_as_url(form->signon_realm);
- DCHECK_EQ(form->scheme, PasswordForm::SCHEME_HTML);
- DCHECK(candidate_signon_realm_as_url.SchemeIsHTTPOrHTTPS());
- std::string candidate_form_organization_name =
- GetOrganizationIdentifyingName(candidate_signon_realm_as_url);
- return candidate_form_organization_name != organization_name;
- });
-
- return result == FormRetrievalResult::kSuccess;
-}
-
bool LoginDatabase::GetLoginsCreatedBetween(
const base::Time begin,
const base::Time end,
@@ -1902,11 +1856,6 @@
DCHECK(get_statement_psl_federated_.empty());
get_statement_psl_federated_ =
get_statement_ + psl_statement + psl_federated_statement;
- DCHECK(get_same_organization_name_logins_statement_.empty());
- get_same_organization_name_logins_statement_ =
- "SELECT " + all_column_names +
- " FROM LOGINS"
- " WHERE scheme == 0 AND signon_realm REGEXP ?";
DCHECK(created_statement_.empty());
created_statement_ =
"SELECT " + all_column_names +
diff --git a/components/password_manager/core/browser/login_database.h b/components/password_manager/core/browser/login_database.h
index f7d8fcc..1e949c22 100644
--- a/components/password_manager/core/browser/login_database.h
+++ b/components/password_manager/core/browser/login_database.h
@@ -123,18 +123,6 @@
std::vector<std::unique_ptr<autofill::PasswordForm>>* forms)
WARN_UNUSED_RESULT;
- // Retrieves all stored credentials with SCHEME_HTTP that have a realm whose
- // organization-identifying name -- that is, the first domain name label below
- // the effective TLD -- matches that of |signon_realm|. Return value indicates
- // a successful query (but potentially no results).
- //
- // For example, the organization-identifying name of "https://foo.example.org"
- // is `example`, and logins will be returned for "http://bar.example.co.uk",
- // but not for "http://notexample.com" or "https://example.foo.com".
- bool GetLoginsForSameOrganizationName(
- const std::string& signon_realm,
- std::vector<std::unique_ptr<autofill::PasswordForm>>* forms);
-
// Gets all logins created from |begin| onwards (inclusive) and before |end|.
// You may use a null Time value to do an unbounded search in either
// direction. |key_to_form_map| must not be null and will be used to return
@@ -332,7 +320,6 @@
std::string get_statement_psl_;
std::string get_statement_federated_;
std::string get_statement_psl_federated_;
- std::string get_same_organization_name_logins_statement_;
std::string created_statement_;
std::string synced_statement_;
std::string blacklisted_statement_;
diff --git a/components/password_manager/core/browser/login_database_unittest.cc b/components/password_manager/core/browser/login_database_unittest.cc
index 93f83e81..1a6e6bb 100644
--- a/components/password_manager/core/browser/login_database_unittest.cc
+++ b/components/password_manager/core/browser/login_database_unittest.cc
@@ -897,151 +897,6 @@
EXPECT_EQ(0U, result.size());
}
-TEST_F(LoginDatabaseTest,
- GetLoginsForSameOrganizationName_OnlyWebHTTPFormsAreConsidered) {
- static constexpr const struct {
- const PasswordFormData form_data;
- bool use_federated_login;
- const char* other_queried_signon_realm;
- bool expected_matches_itself;
- bool expected_matches_other_realm;
- } kTestCases[] = {
- {{PasswordForm::SCHEME_HTML, "https://example.com/",
- "https://example.com/origin", "", L"", L"", L"", L"u", L"p", false, 1},
- false,
- nullptr,
- true,
- true},
- {{PasswordForm::SCHEME_BASIC, "http://example.com/realm",
- "http://example.com/", "", L"", L"", L"", L"u", L"p", false, 1},
- false,
- nullptr,
- false,
- false},
- {{PasswordForm::SCHEME_OTHER, "ftp://example.com/realm",
- "ftp://example.com/", "", L"", L"", L"", L"u", L"p", false, 1},
- false,
- "http://example.com/realm",
- false,
- false},
- {{PasswordForm::SCHEME_HTML,
- "federation://example.com/accounts.google.com",
- "https://example.com/orgin", "", L"", L"", L"", L"u", L"", false, 1},
- true,
- "http://example.com/",
- false,
- false},
- {{PasswordForm::SCHEME_HTML, "android://hash@example.com/",
- "android://hash@example.com/", "", L"", L"", L"", L"u", L"p", false, 1},
- false,
- "http://example.com/",
- false,
- false},
- };
-
- for (const auto& test_case : kTestCases) {
- SCOPED_TRACE(test_case.form_data.signon_realm);
-
- std::unique_ptr<PasswordForm> form = FillPasswordFormWithData(
- test_case.form_data, test_case.use_federated_login);
- ASSERT_EQ(AddChangeForForm(*form), db().AddLogin(*form));
-
- std::vector<std::unique_ptr<PasswordForm>> same_organization_forms;
- EXPECT_TRUE(db().GetLoginsForSameOrganizationName(
- form->signon_realm, &same_organization_forms));
- EXPECT_EQ(test_case.expected_matches_itself ? 1u : 0u,
- same_organization_forms.size());
-
- if (test_case.other_queried_signon_realm) {
- same_organization_forms.clear();
- EXPECT_TRUE(db().GetLoginsForSameOrganizationName(
- test_case.other_queried_signon_realm, &same_organization_forms));
- EXPECT_EQ(test_case.expected_matches_other_realm ? 1u : 0u,
- same_organization_forms.size());
- }
-
- ASSERT_TRUE(db().RemoveLogin(*form, /*changes=*/nullptr));
- }
-}
-
-TEST_F(LoginDatabaseTest, GetLoginsForSameOrganizationName_DetailsOfMatching) {
- const struct {
- const char* saved_signon_realm;
- const char* queried_signon_realm;
- bool expected_matches;
- } kTestCases[] = {
- // PSL matches are also same-organization-name matches.
- {"http://psl.example.com/", "http://example.com/", true},
- {"http://example.com/", "http://sub.example.com/", true},
- {"https://a.b.example.co.uk/", "https://c.d.e.example.co.uk/", true},
-
- // Non-PSL but same-organization-name matches. Also an illustration why it
- // would be unsafe to offer these credentials for filling.
- {"https://example.com/", "https://example.co.uk/", true},
- {"https://example.co.uk/", "https://example.com/", true},
- {"https://a.example.appspot.com/", "https://b.example.co.uk/", true},
-
- // Same-organization-name matches are HTTP/HTTPS-agnostic.
- {"https://example.com/", "http://example.com/", true},
- {"http://example.com/", "https://example.com/", true},
-
- {"http://www.foo-bar.com/", "http://sub.foo-bar.com", true},
- {"http://www.foo_bar.com/", "http://sub.foo_bar.com", true},
- {"http://www.foo-bar.com/", "http://sub.foo%2Dbar.com", true},
- {"http://www.foo%21bar.com/", "http://sub.foo!bar.com", true},
- {"http://a.xn--sztr-7na0i.co.uk/", "http://xn--sztr-7na0i.com/", true},
- {"http://a.xn--sztr-7na0i.co.uk/", "http://www.sz\xc3\xb3t\xc3\xa1r.com/",
- true},
-
- {"http://www.foo+bar.com/", "http://sub.foo+bar.com", true},
- {"http://www.foooobar.com/", "http://sub.foo+bar.com", false},
- {"http://www.fobar.com/", "http://sub.foo?bar.com", false},
- {"http://www.foozbar.com/", "http://sub.foo.bar.com", false},
- {"http://www.foozbar.com/", "http://sub.foo[a-z]bar.com", false},
-
- {"https://notexample.com/", "https://example.com/", false},
- {"https://a.notexample.com/", "https://example.com/", false},
- {"https://example.com/", "https://notexample.com/", false},
- {"https://example.com/", "https://example.bar.com/", false},
- {"https://example.foo.com/", "https://example.com/", false},
- {"https://example.foo.com/", "https://example.bar.com/", false},
-
- // URLs without host portions, hosts without registry controlled domains
- // or hosts consisting of a registry.
- {"http://localhost/", "http://localhost/", false},
- {"https://example/", "https://example/", false},
- {"https://co.uk/", "https://co.uk/", false},
- {"https://example/", "https://example.com/", false},
- {"https://a.example/", "https://example.com/", false},
- {"https://example.com/", "https://example/", false},
- {"https://127.0.0.1/", "https://127.0.0.1/", false},
- {"https:/[3ffe:2a00:100:7031::1]/", "https:/[3ffe:2a00:100:7031::1]/",
- false},
-
- // Queried |signon-realms| are invalid URIs.
- {"https://example.com/", "", false},
- {"https://example.com/", "bad url", false},
- {"https://example.com/", "https://", false},
- {"https://example.com/", "http://www.foo;bar.com", false},
- {"https://example.com/", "example", false},
- };
-
- for (const auto& test_case : kTestCases) {
- SCOPED_TRACE(test_case.saved_signon_realm);
- SCOPED_TRACE(test_case.queried_signon_realm);
-
- std::unique_ptr<PasswordForm> form = FillPasswordFormWithData(
- {PasswordForm::SCHEME_HTML, test_case.saved_signon_realm,
- test_case.saved_signon_realm, "", L"", L"", L"", L"u", L"p", true, 1});
- std::vector<std::unique_ptr<PasswordForm>> result;
- ASSERT_EQ(AddChangeForForm(*form), db().AddLogin(*form));
- EXPECT_TRUE(db().GetLoginsForSameOrganizationName(
- test_case.queried_signon_realm, &result));
- EXPECT_EQ(test_case.expected_matches ? 1u : 0u, result.size());
- ASSERT_TRUE(db().RemoveLogin(*form, /*changes=*/nullptr));
- }
-}
-
static bool AddTimestampedLogin(LoginDatabase* db,
std::string url,
const std::string& unique_string,
diff --git a/components/password_manager/core/browser/mock_password_store.h b/components/password_manager/core/browser/mock_password_store.h
index 9c116d6..53aa478 100644
--- a/components/password_manager/core/browser/mock_password_store.h
+++ b/components/password_manager/core/browser/mock_password_store.h
@@ -22,8 +22,6 @@
MOCK_METHOD1(RemoveLogin, void(const autofill::PasswordForm&));
MOCK_METHOD2(GetLogins,
void(const PasswordStore::FormDigest&, PasswordStoreConsumer*));
- MOCK_METHOD2(GetLoginsForSameOrganizationName,
- void(const std::string&, PasswordStoreConsumer*));
MOCK_METHOD1(AddLogin, void(const autofill::PasswordForm&));
MOCK_METHOD1(UpdateLogin, void(const autofill::PasswordForm&));
MOCK_METHOD2(UpdateLoginWithPrimaryKey,
@@ -56,10 +54,6 @@
const PasswordStore::FormDigest& form) override {
return std::vector<std::unique_ptr<autofill::PasswordForm>>();
}
- std::vector<std::unique_ptr<autofill::PasswordForm>>
- FillLoginsForSameOrganizationName(const std::string& signon_realm) override {
- return std::vector<std::unique_ptr<autofill::PasswordForm>>();
- }
MOCK_METHOD1(FillAutofillableLogins,
bool(std::vector<std::unique_ptr<autofill::PasswordForm>>*));
MOCK_METHOD1(FillBlacklistLogins,
diff --git a/components/password_manager/core/browser/new_password_form_manager.cc b/components/password_manager/core/browser/new_password_form_manager.cc
index 01ab4856..1bb3857e 100644
--- a/components/password_manager/core/browser/new_password_form_manager.cc
+++ b/components/password_manager/core/browser/new_password_form_manager.cc
@@ -713,13 +713,12 @@
const PasswordStore::FormDigest& form_digest)
: client_(client),
metrics_recorder_(metrics_recorder),
- owned_form_fetcher_(
- form_fetcher ? nullptr
- : std::make_unique<FormFetcherImpl>(
- form_digest,
- client_,
- true /* should_migrate_http_passwords */,
- true /* should_query_suppressed_https_forms */)),
+ owned_form_fetcher_(form_fetcher
+ ? nullptr
+ : std::make_unique<FormFetcherImpl>(
+ form_digest,
+ client_,
+ true /* should_migrate_http_passwords */)),
form_fetcher_(form_fetcher ? form_fetcher : owned_form_fetcher_.get()),
form_saver_(std::move(form_saver)),
// TODO(https://crbug.com/831123): set correctly
diff --git a/components/password_manager/core/browser/password_form_manager.cc b/components/password_manager/core/browser/password_form_manager.cc
index c316deb..a8fb6c0 100644
--- a/components/password_manager/core/browser/password_form_manager.cc
+++ b/components/password_manager/core/browser/password_form_manager.cc
@@ -123,13 +123,12 @@
observed_form.IsPossibleChangePasswordFormWithoutUsername()),
client_(client),
form_saver_(std::move(form_saver)),
- owned_form_fetcher_(
- form_fetcher ? nullptr
- : std::make_unique<FormFetcherImpl>(
- PasswordStore::FormDigest(observed_form),
- client,
- true /* should_migrate_http_passwords */,
- true /* should_query_suppressed_https_forms */)),
+ owned_form_fetcher_(form_fetcher
+ ? nullptr
+ : std::make_unique<FormFetcherImpl>(
+ PasswordStore::FormDigest(observed_form),
+ client,
+ true /* should_migrate_http_passwords */)),
form_fetcher_(form_fetcher ? form_fetcher : owned_form_fetcher_.get()),
votes_uploader_(client, observed_form.IsPossibleChangePasswordForm()) {
// Non-HTML forms should not need any interaction with the renderer, and hence
@@ -162,10 +161,6 @@
PasswordFormManager::~PasswordFormManager() {
form_fetcher_->RemoveConsumer(this);
-
- metrics_recorder_->RecordHistogramsOnSuppressedAccounts(
- observed_form_.origin.SchemeIsCryptographic(), *form_fetcher_,
- pending_credentials_);
}
// static
diff --git a/components/password_manager/core/browser/password_form_manager_unittest.cc b/components/password_manager/core/browser/password_form_manager_unittest.cc
index f47e6211..9f3d248a 100644
--- a/components/password_manager/core/browser/password_form_manager_unittest.cc
+++ b/components/password_manager/core/browser/password_form_manager_unittest.cc
@@ -3794,254 +3794,6 @@
form_manager.GrabFetcher(std::move(new_fetcher));
}
-TEST_F(PasswordFormManagerTest,
- SuppressedHTTPSFormsHistogram_NotRecordedIfStoreWasTooSlow) {
- base::HistogramTester histogram_tester;
-
- fake_form_fetcher()->set_did_complete_querying_suppressed_forms(false);
- fake_form_fetcher()->Fetch();
- std::unique_ptr<PasswordFormManager> form_manager =
- std::make_unique<PasswordFormManager>(
- password_manager(), client(), client()->driver(), *observed_form(),
- std::make_unique<NiceMock<MockFormSaver>>(), fake_form_fetcher());
- form_manager->Init(nullptr);
- fake_form_fetcher()->NotifyFetchCompleted();
- form_manager.reset();
-
- histogram_tester.ExpectUniqueSample(
- "PasswordManager.QueryingSuppressedAccountsFinished", false, 1);
- const auto sample_counts = histogram_tester.GetTotalCountsForPrefix(
- "PasswordManager.SuppressedAccount.");
- EXPECT_THAT(sample_counts, IsEmpty());
-}
-
-TEST_F(PasswordFormManagerTest, SuppressedFormsHistograms) {
- static constexpr const struct {
- SuppressedFormType type;
- const char* expected_histogram_suffix;
- void (FakeFormFetcher::*suppressed_forms_setter_func)(
- const std::vector<const autofill::PasswordForm*>&);
- } kSuppressedFormTypeParams[] = {
- {SuppressedFormType::HTTPS, "HTTPSNotHTTP",
- &FakeFormFetcher::set_suppressed_https_forms},
- {SuppressedFormType::PSL_MATCH, "PSLMatching",
- &FakeFormFetcher::set_suppressed_psl_matching_forms},
- {SuppressedFormType::SAME_ORGANIZATION_NAME, "SameOrganizationName",
- &FakeFormFetcher::set_suppressed_same_organization_name_forms}};
-
- struct SuppressedFormData {
- const char* username_value;
- const char* password_value;
- PasswordForm::Type manual_or_generated;
- };
-
- static constexpr const char kUsernameAlpha[] = "user-alpha@gmail.com";
- static constexpr const char kPasswordAlpha[] = "password-alpha";
- static constexpr const char kUsernameBeta[] = "user-beta@gmail.com";
- static constexpr const char kPasswordBeta[] = "password-beta";
-
- static constexpr const SuppressedFormData kSuppressedGeneratedForm = {
- kUsernameAlpha, kPasswordAlpha, PasswordForm::TYPE_GENERATED};
- static constexpr const SuppressedFormData kOtherSuppressedGeneratedForm = {
- kUsernameBeta, kPasswordBeta, PasswordForm::TYPE_GENERATED};
- static constexpr const SuppressedFormData kSuppressedManualForm = {
- kUsernameAlpha, kPasswordBeta, PasswordForm::TYPE_MANUAL};
-
- const std::vector<const SuppressedFormData*> kSuppressedFormsNone;
- const std::vector<const SuppressedFormData*> kSuppressedFormsOneGenerated = {
- &kSuppressedGeneratedForm};
- const std::vector<const SuppressedFormData*> kSuppressedFormsTwoGenerated = {
- &kSuppressedGeneratedForm, &kOtherSuppressedGeneratedForm};
- const std::vector<const SuppressedFormData*> kSuppressedFormsOneManual = {
- &kSuppressedManualForm};
- const std::vector<const SuppressedFormData*> kSuppressedFormsTwoMixed = {
- &kSuppressedGeneratedForm, &kSuppressedManualForm};
-
- const struct {
- std::vector<const SuppressedFormData*> simulated_suppressed_forms;
- SimulatedManagerAction simulate_manager_action;
- SimulatedSubmitResult simulate_submit_result;
- const char* filled_username;
- const char* filled_password;
- int expected_histogram_sample_generated;
- int expected_histogram_sample_manual;
- const char* submitted_password; // nullptr if same as |filled_password|.
- } kTestCases[] = {
- // See PasswordManagerSuppressedAccountCrossActionsTaken in enums.xml.
- //
- // Legend: (SuppressAccountType, SubmitResult, SimulatedManagerAction,
- // UserAction)
- // 24 = (None, Passed, None, OverrideUsernameAndPassword)
- {kSuppressedFormsNone, SimulatedManagerAction::NONE,
- SimulatedSubmitResult::PASSED, kUsernameAlpha, kPasswordAlpha, 24, 24},
- // 5 = (None, NotSubmitted, Autofilled, None)
- {kSuppressedFormsNone, SimulatedManagerAction::AUTOFILLED,
- SimulatedSubmitResult::NONE, kUsernameAlpha, kPasswordAlpha, 5, 5},
- // 15 = (None, Failed, Autofilled, None)
- {kSuppressedFormsNone, SimulatedManagerAction::AUTOFILLED,
- SimulatedSubmitResult::FAILED, kUsernameAlpha, kPasswordAlpha, 15, 15},
-
- // 35 = (Exists, NotSubmitted, Autofilled, None)
- {kSuppressedFormsOneGenerated, SimulatedManagerAction::AUTOFILLED,
- SimulatedSubmitResult::NONE, kUsernameAlpha, kPasswordAlpha, 35, 5},
- // 145 = (ExistsSameUsernameAndPassword, Passed, Autofilled, None)
- // 25 = (None, Passed, Autofilled, None)
- {kSuppressedFormsOneGenerated, SimulatedManagerAction::AUTOFILLED,
- SimulatedSubmitResult::PASSED, kUsernameAlpha, kPasswordAlpha, 145, 25},
- // 104 = (ExistsSameUsername, Failed, None, OverrideUsernameAndPassword)
- // 14 = (None, Failed, None, OverrideUsernameAndPassword)
- {kSuppressedFormsOneGenerated, SimulatedManagerAction::NONE,
- SimulatedSubmitResult::FAILED, kUsernameAlpha, kPasswordBeta, 104, 14},
- // 84 = (ExistsDifferentUsername, Passed, None,
- // OverrideUsernameAndPassword)
- {kSuppressedFormsOneGenerated, SimulatedManagerAction::NONE,
- SimulatedSubmitResult::PASSED, kUsernameBeta, kPasswordAlpha, 84, 24},
-
- // 144 = (ExistsSameUsernameAndPassword, Passed, None,
- // OverrideUsernameAndPassword)
- {kSuppressedFormsOneManual, SimulatedManagerAction::NONE,
- SimulatedSubmitResult::PASSED, kUsernameAlpha, kPasswordBeta, 24, 144},
- {kSuppressedFormsTwoMixed, SimulatedManagerAction::NONE,
- SimulatedSubmitResult::PASSED, kUsernameBeta, kPasswordAlpha, 84, 84},
-
- // 115 = (ExistsSameUsername, Passed, Autofilled, None)
- {kSuppressedFormsTwoGenerated, SimulatedManagerAction::AUTOFILLED,
- SimulatedSubmitResult::PASSED, kUsernameAlpha, kPasswordAlpha, 145, 25},
- {kSuppressedFormsTwoGenerated, SimulatedManagerAction::AUTOFILLED,
- SimulatedSubmitResult::PASSED, kUsernameAlpha, kPasswordBeta, 115, 25},
- {kSuppressedFormsTwoGenerated, SimulatedManagerAction::AUTOFILLED,
- SimulatedSubmitResult::PASSED, kUsernameBeta, kPasswordAlpha, 115, 25},
- {kSuppressedFormsTwoGenerated, SimulatedManagerAction::AUTOFILLED,
- SimulatedSubmitResult::PASSED, kUsernameBeta, kPasswordBeta, 145, 25},
-
- // 86 = (ExistsDifferentUsername, Passed, Autofilled, Choose)
- // 26 = (None, Passed, Autofilled, Choose)
- {kSuppressedFormsOneGenerated, SimulatedManagerAction::OFFERED,
- SimulatedSubmitResult::PASSED, kUsernameBeta, kPasswordAlpha, 86, 26},
- // 72 = (ExistsDifferentUsername, Failed, None, ChoosePSL)
- // 12 = (None, Failed, None, ChoosePSL)
- {kSuppressedFormsOneGenerated, SimulatedManagerAction::OFFERED_PSL,
- SimulatedSubmitResult::FAILED, kUsernameBeta, kPasswordAlpha, 72, 12},
- // 148 = (ExistsSameUsernameAndPassword, Passed, Autofilled,
- // OverridePassword)
- // 28 = (None, Passed, Autofilled, OverridePassword)
- {kSuppressedFormsTwoGenerated, SimulatedManagerAction::AUTOFILLED,
- SimulatedSubmitResult::PASSED, kUsernameBeta, kPasswordAlpha, 148, 28,
- kPasswordBeta},
- };
-
- for (const auto& suppression_params : kSuppressedFormTypeParams) {
- for (const auto& test_case : kTestCases) {
- SCOPED_TRACE(suppression_params.expected_histogram_suffix);
- SCOPED_TRACE(test_case.expected_histogram_sample_manual);
- SCOPED_TRACE(test_case.expected_histogram_sample_generated);
-
- base::HistogramTester histogram_tester;
- ukm::TestAutoSetUkmRecorder test_ukm_recorder;
-
- std::vector<PasswordForm> suppressed_forms;
- for (const auto* form_data : test_case.simulated_suppressed_forms) {
- suppressed_forms.push_back(CreateSuppressedForm(
- suppression_params.type, form_data->username_value,
- form_data->password_value, form_data->manual_or_generated));
- }
-
- std::vector<const PasswordForm*> suppressed_forms_ptrs;
- for (const auto& form : suppressed_forms)
- suppressed_forms_ptrs.push_back(&form);
-
- FakeFormFetcher fetcher;
- fetcher.set_did_complete_querying_suppressed_forms(true);
-
- (&fetcher->*suppression_params.suppressed_forms_setter_func)(
- suppressed_forms_ptrs);
-
- SimulateActionsOnHTTPObservedForm(
- &fetcher, test_case.simulate_manager_action,
- test_case.simulate_submit_result, test_case.filled_username,
- test_case.filled_password, test_case.submitted_password);
-
- histogram_tester.ExpectUniqueSample(
- "PasswordManager.QueryingSuppressedAccountsFinished", true, 1);
-
- EXPECT_THAT(
- histogram_tester.GetAllSamples(
- "PasswordManager.SuppressedAccount.Generated." +
- std::string(suppression_params.expected_histogram_suffix)),
- ::testing::ElementsAre(
- base::Bucket(test_case.expected_histogram_sample_generated, 1)));
- EXPECT_THAT(
- histogram_tester.GetAllSamples(
- "PasswordManager.SuppressedAccount.Manual." +
- std::string(suppression_params.expected_histogram_suffix)),
- ::testing::ElementsAre(
- base::Bucket(test_case.expected_histogram_sample_manual, 1)));
-
- auto entries = test_ukm_recorder.GetEntriesByName(
- ukm::builders::PasswordForm::kEntryName);
- EXPECT_EQ(1u, entries.size());
- for (const auto* const entry : entries) {
- test_ukm_recorder.ExpectEntryMetric(
- entry,
- "SuppressedAccount.Generated." +
- std::string(suppression_params.expected_histogram_suffix),
- test_case.expected_histogram_sample_generated /
- PasswordFormMetricsRecorder::kMaxNumActionsTakenNew);
- test_ukm_recorder.ExpectEntryMetric(
- entry,
- "SuppressedAccount.Manual." +
- std::string(suppression_params.expected_histogram_suffix),
- test_case.expected_histogram_sample_manual /
- PasswordFormMetricsRecorder::kMaxNumActionsTakenNew);
- }
- }
- }
-}
-
-// If the frame containing the login form is served over HTTPS, no histograms on
-// supressed HTTPS forms should be recorded. Everything else should still be.
-TEST_F(PasswordFormManagerTest, SuppressedHTTPSFormsHistogram_NotRecordedFor) {
- base::HistogramTester histogram_tester;
-
- PasswordForm https_observed_form = *observed_form();
- https_observed_form.origin = GURL("https://accounts.google.com/a/LoginAuth");
- https_observed_form.signon_realm = "https://accounts.google.com/";
-
- // Only the scheme of the frame containing the login form maters, not the
- // scheme of the main frame.
- ASSERT_FALSE(client()->IsMainFrameSecure());
-
- // Even if there were any suppressed HTTPS forms, they should be are ignored
- // (but there should be none in production in this case).
- FakeFormFetcher fetcher;
- fetcher.set_suppressed_https_forms({saved_match()});
- fetcher.set_did_complete_querying_suppressed_forms(true);
- fetcher.Fetch();
-
- std::unique_ptr<PasswordFormManager> form_manager =
- std::make_unique<PasswordFormManager>(
- password_manager(), client(), client()->driver(), https_observed_form,
- std::make_unique<NiceMock<MockFormSaver>>(), &fetcher);
- form_manager->Init(nullptr);
- fetcher.NotifyFetchCompleted();
- form_manager.reset();
-
- histogram_tester.ExpectUniqueSample(
- "PasswordManager.QueryingSuppressedAccountsFinished", true, 1);
- histogram_tester.ExpectTotalCount(
- "PasswordManager.SuppressedAccount.Generated.HTTPSNotHTTP", 0);
- histogram_tester.ExpectTotalCount(
- "PasswordManager.SuppressedAccount.Manual.HTTPSNotHTTP", 0);
- histogram_tester.ExpectTotalCount(
- "PasswordManager.SuppressedAccount.Generated.PSLMatching", 1);
- histogram_tester.ExpectTotalCount(
- "PasswordManager.SuppressedAccount.Manual.PSLMatching", 1);
- histogram_tester.ExpectTotalCount(
- "PasswordManager.SuppressedAccount.Generated.SameOrganizationName", 1);
- histogram_tester.ExpectTotalCount(
- "PasswordManager.SuppressedAccount.Manual.SameOrganizationName", 1);
-}
-
// Check that a cloned PasswordFormManager reacts correctly to Save.
TEST_F(PasswordFormManagerTest, Clone_OnSave) {
FakeFormFetcher fetcher;
diff --git a/components/password_manager/core/browser/password_form_metrics_recorder.cc b/components/password_manager/core/browser/password_form_metrics_recorder.cc
index 858b8aa..a0334a00 100644
--- a/components/password_manager/core/browser/password_form_metrics_recorder.cc
+++ b/components/password_manager/core/browser/password_form_metrics_recorder.cc
@@ -557,78 +557,6 @@
(manager_action_ + kManagerActionMax * submit_result_);
}
-void PasswordFormMetricsRecorder::RecordHistogramsOnSuppressedAccounts(
- bool observed_form_origin_has_cryptographic_scheme,
- const FormFetcher& form_fetcher,
- const PasswordForm& pending_credentials) {
- UMA_HISTOGRAM_BOOLEAN("PasswordManager.QueryingSuppressedAccountsFinished",
- form_fetcher.DidCompleteQueryingSuppressedForms());
-
- if (!form_fetcher.DidCompleteQueryingSuppressedForms())
- return;
-
- SuppressedAccountExistence best_match = kSuppressedAccountNone;
-
- if (!observed_form_origin_has_cryptographic_scheme) {
- best_match = GetBestMatchingSuppressedAccount(
- form_fetcher.GetSuppressedHTTPSForms(), PasswordForm::TYPE_GENERATED,
- pending_credentials);
- UMA_HISTOGRAM_ENUMERATION(
- "PasswordManager.SuppressedAccount.Generated.HTTPSNotHTTP",
- GetHistogramSampleForSuppressedAccounts(best_match),
- kMaxSuppressedAccountStats);
- ukm_entry_builder_.SetSuppressedAccount_Generated_HTTPSNotHTTP(best_match);
-
- best_match = GetBestMatchingSuppressedAccount(
- form_fetcher.GetSuppressedHTTPSForms(), PasswordForm::TYPE_MANUAL,
- pending_credentials);
- UMA_HISTOGRAM_ENUMERATION(
- "PasswordManager.SuppressedAccount.Manual.HTTPSNotHTTP",
- GetHistogramSampleForSuppressedAccounts(best_match),
- kMaxSuppressedAccountStats);
- ukm_entry_builder_.SetSuppressedAccount_Manual_HTTPSNotHTTP(best_match);
- }
-
- best_match = GetBestMatchingSuppressedAccount(
- form_fetcher.GetSuppressedPSLMatchingForms(),
- PasswordForm::TYPE_GENERATED, pending_credentials);
- UMA_HISTOGRAM_ENUMERATION(
- "PasswordManager.SuppressedAccount.Generated.PSLMatching",
- GetHistogramSampleForSuppressedAccounts(best_match),
- kMaxSuppressedAccountStats);
- ukm_entry_builder_.SetSuppressedAccount_Generated_PSLMatching(best_match);
- best_match = GetBestMatchingSuppressedAccount(
- form_fetcher.GetSuppressedPSLMatchingForms(), PasswordForm::TYPE_MANUAL,
- pending_credentials);
- UMA_HISTOGRAM_ENUMERATION(
- "PasswordManager.SuppressedAccount.Manual.PSLMatching",
- GetHistogramSampleForSuppressedAccounts(best_match),
- kMaxSuppressedAccountStats);
- ukm_entry_builder_.SetSuppressedAccount_Manual_PSLMatching(best_match);
-
- best_match = GetBestMatchingSuppressedAccount(
- form_fetcher.GetSuppressedSameOrganizationNameForms(),
- PasswordForm::TYPE_GENERATED, pending_credentials);
- UMA_HISTOGRAM_ENUMERATION(
- "PasswordManager.SuppressedAccount.Generated.SameOrganizationName",
- GetHistogramSampleForSuppressedAccounts(best_match),
- kMaxSuppressedAccountStats);
- ukm_entry_builder_.SetSuppressedAccount_Generated_SameOrganizationName(
- best_match);
- best_match = GetBestMatchingSuppressedAccount(
- form_fetcher.GetSuppressedSameOrganizationNameForms(),
- PasswordForm::TYPE_MANUAL, pending_credentials);
- UMA_HISTOGRAM_ENUMERATION(
- "PasswordManager.SuppressedAccount.Manual.SameOrganizationName",
- GetHistogramSampleForSuppressedAccounts(best_match),
- kMaxSuppressedAccountStats);
- ukm_entry_builder_.SetSuppressedAccount_Manual_SameOrganizationName(
- best_match);
-
- if (current_bubble_ != CurrentBubbleOfInterest::kNone)
- RecordUIDismissalReason(metrics_util::NO_DIRECT_INTERACTION);
-}
-
void PasswordFormMetricsRecorder::RecordPasswordBubbleShown(
metrics_util::CredentialSourceType credential_source_type,
metrics_util::UIDisplayDisposition display_disposition) {
@@ -715,40 +643,4 @@
ukm_entry_builder_.SetManagerFill_Action(event);
}
-PasswordFormMetricsRecorder::SuppressedAccountExistence
-PasswordFormMetricsRecorder::GetBestMatchingSuppressedAccount(
- const std::vector<const PasswordForm*>& suppressed_forms,
- PasswordForm::Type manual_or_generated,
- const PasswordForm& pending_credentials) const {
- SuppressedAccountExistence best_matching_account = kSuppressedAccountNone;
- for (const PasswordForm* form : suppressed_forms) {
- if (form->type != manual_or_generated)
- continue;
-
- SuppressedAccountExistence current_account;
- if (pending_credentials.password_value.empty())
- current_account = kSuppressedAccountExists;
- else if (form->username_value != pending_credentials.username_value)
- current_account = kSuppressedAccountExistsDifferentUsername;
- else if (form->password_value != pending_credentials.password_value)
- current_account = kSuppressedAccountExistsSameUsername;
- else
- current_account = kSuppressedAccountExistsSameUsernameAndPassword;
-
- best_matching_account = std::max(best_matching_account, current_account);
- }
- return best_matching_account;
-}
-
-int PasswordFormMetricsRecorder::GetHistogramSampleForSuppressedAccounts(
- SuppressedAccountExistence best_matching_account) const {
- // Encoding: most significant digit is the |best_matching_account|.
- int mixed_base_encoding = 0;
- mixed_base_encoding += best_matching_account;
- mixed_base_encoding *= PasswordFormMetricsRecorder::kMaxNumActionsTakenNew;
- mixed_base_encoding += GetActionsTakenNew();
- DCHECK_LT(mixed_base_encoding, kMaxSuppressedAccountStats);
- return mixed_base_encoding;
-}
-
} // namespace password_manager
diff --git a/components/password_manager/core/browser/password_form_metrics_recorder.h b/components/password_manager/core/browser/password_form_metrics_recorder.h
index 3627d22..cdce846 100644
--- a/components/password_manager/core/browser/password_form_metrics_recorder.h
+++ b/components/password_manager/core/browser/password_form_metrics_recorder.h
@@ -29,7 +29,6 @@
namespace password_manager {
-class FormFetcher;
struct InteractionsStats;
// The pupose of this class is to record various types of metrics about the
@@ -93,24 +92,6 @@
kManagerFillEventAutofilled
};
- // Enumerates whether there were `suppressed` credentials. These are stored
- // credentials that were not filled, even though they might be related to the
- // observed form. See FormFetcher::GetSuppressed* for details.
- //
- // If suppressed credentials exist, it is also recorded whether their username
- // and/or password matched those submitted.
- enum SuppressedAccountExistence {
- kSuppressedAccountNone,
- // Recorded when there exists a suppressed account, but there was no
- // submitted form to compare its username and password to.
- kSuppressedAccountExists,
- // Recorded when there was a submitted form.
- kSuppressedAccountExistsDifferentUsername,
- kSuppressedAccountExistsSameUsername,
- kSuppressedAccountExistsSameUsernameAndPassword,
- kSuppressedAccountExistenceMax,
- };
-
// What the form is used for. kSubmittedFormTypeUnspecified is only set before
// the SetSubmittedFormType() is called, and should never be actually
// uploaded.
@@ -287,14 +268,6 @@
kManagerActionNewMax * static_cast<int>(UserAction::kMax) *
kSubmitResultMax;
- // The maximum number of combinations recorded into histograms in the
- // PasswordManager.SuppressedAccount.* family.
- static constexpr int kMaxSuppressedAccountStats =
- kSuppressedAccountExistenceMax *
- PasswordFormMetricsRecorder::kManagerActionNewMax *
- static_cast<int>(UserAction::kMax) *
- PasswordFormMetricsRecorder::kSubmitResultMax;
-
// Called if the user could generate a password for this form.
void MarkGenerationAvailable();
@@ -342,16 +315,6 @@
// submission.
void SetSubmissionIndicatorEvent(autofill::SubmissionIndicatorEvent event);
- // Records all histograms in the PasswordManager.SuppressedAccount.* family.
- // Takes the FormFetcher intance which owns the login data from PasswordStore.
- // |pending_credentials| stores credentials when the form was submitted but
- // success was still unknown. It contains credentials that are ready to be
- // written (saved or updated) to a password store.
- void RecordHistogramsOnSuppressedAccounts(
- bool observed_form_origin_has_cryptographic_scheme,
- const FormFetcher& form_fetcher,
- const autofill::PasswordForm& pending_credentials);
-
// Records the event that a password bubble was shown.
void RecordPasswordBubbleShown(
metrics_util::CredentialSourceType credential_source_type,
@@ -441,29 +404,6 @@
// UMA.
int GetActionsTaken() const;
- // When supplied with the list of all |suppressed_forms| that belong to
- // certain suppressed credential type (see FormFetcher::GetSuppressed*),
- // filters that list down to forms whose type matches |manual_or_generated|,
- // and selects the suppressed account that matches |pending_credentials| most
- // closely. |pending_credentials| stores credentials when the form was
- // submitted but success was still unknown. It contains credentials that are
- // ready to be written (saved or updated) to a password store.
- SuppressedAccountExistence GetBestMatchingSuppressedAccount(
- const std::vector<const autofill::PasswordForm*>& suppressed_forms,
- autofill::PasswordForm::Type manual_or_generated,
- const autofill::PasswordForm& pending_credentials) const;
-
- // Encodes a UMA histogram sample for |best_matching_account| and
- // GetActionsTakenNew(). This is a mixed-based representation of a combination
- // of four attributes:
- // -- whether there were suppressed credentials (and if so, their relation to
- // the submitted username/password).
- // -- whether the |observed_form_| got ultimately submitted
- // -- what action the password manager performed (|manager_action_|),
- // -- and what action the user performed (|user_action_|_).
- int GetHistogramSampleForSuppressedAccounts(
- SuppressedAccountExistence best_matching_account) const;
-
// True if the main frame's visible URL, at the time this PasswordFormManager
// was created, is secure.
const bool is_main_frame_secure_;
diff --git a/components/password_manager/core/browser/password_manager_unittest.cc b/components/password_manager/core/browser/password_manager_unittest.cc
index 4bd2f56..27304f9 100644
--- a/components/password_manager/core/browser/password_manager_unittest.cc
+++ b/components/password_manager/core/browser/password_manager_unittest.cc
@@ -241,8 +241,6 @@
void SetUp() override {
store_ = new testing::StrictMock<MockPasswordStore>;
EXPECT_CALL(*store_, ReportMetrics(_, _, _)).Times(AnyNumber());
- EXPECT_CALL(*store_, GetLoginsForSameOrganizationName(_, _))
- .Times(AnyNumber());
CHECK(store_->Init(syncer::SyncableService::StartSyncFlare(), nullptr));
ON_CALL(client_, GetPasswordStore()).WillByDefault(Return(store_.get()));
@@ -2206,12 +2204,10 @@
if (found_matched_logins_in_store) {
EXPECT_CALL(*store_, GetLogins(_, _))
.WillRepeatedly(WithArg<1>(InvokeConsumer(form)));
- EXPECT_CALL(*store_, GetLoginsForSameOrganizationName(_, _));
EXPECT_CALL(driver_, FillPasswordForm(_)).Times(2);
} else {
EXPECT_CALL(*store_, GetLogins(_, _))
.WillRepeatedly(WithArg<1>(InvokeEmptyConsumerWithForms()));
- EXPECT_CALL(*store_, GetLoginsForSameOrganizationName(_, _));
}
std::unique_ptr<PasswordFormManagerForUI> form_manager;
if (found_matched_logins_in_store) {
diff --git a/components/password_manager/core/browser/password_store.cc b/components/password_manager/core/browser/password_store.cc
index 6a9c8a6e..1494ae963 100644
--- a/components/password_manager/core/browser/password_store.cc
+++ b/components/password_manager/core/browser/password_store.cc
@@ -247,15 +247,6 @@
}
}
-void PasswordStore::GetLoginsForSameOrganizationName(
- const std::string& signon_realm,
- PasswordStoreConsumer* consumer) {
- PostLoginsTaskAndReplyToConsumerWithResult(
- consumer,
- base::BindOnce(&PasswordStore::GetLoginsForSameOrganizationNameImpl, this,
- signon_realm));
-}
-
void PasswordStore::GetAutofillableLogins(PasswordStoreConsumer* consumer) {
PostLoginsTaskAndReplyToConsumerWithResult(
consumer,
@@ -795,13 +786,6 @@
return FillMatchingLogins(form);
}
-std::vector<std::unique_ptr<autofill::PasswordForm>>
-PasswordStore::GetLoginsForSameOrganizationNameImpl(
- const std::string& signon_realm) {
- DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
- return FillLoginsForSameOrganizationName(signon_realm);
-}
-
std::vector<std::unique_ptr<PasswordForm>>
PasswordStore::GetAutofillableLoginsImpl() {
DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
diff --git a/components/password_manager/core/browser/password_store.h b/components/password_manager/core/browser/password_store.h
index 15c1a0cb..4a937b16 100644
--- a/components/password_manager/core/browser/password_store.h
+++ b/components/password_manager/core/browser/password_store.h
@@ -190,21 +190,6 @@
virtual void GetLogins(const FormDigest& form,
PasswordStoreConsumer* consumer);
- // Returns all stored credentials with SCHEME_HTTP that have a realm whose
- // organization-identifying name -- that is, the first domain name label below
- // the effective TLD -- matches that of |signon_realm|. Notifies |consumer| on
- // completion. The request will be cancelled if the consumer is destroyed.
- //
- // WARNING: This is *NOT* PSL (Public Suffix List) matching. The logins
- // returned by this method are not safe to be filled into the observed form.
- //
- // For example, the organization-identifying name of "https://foo.example.org"
- // is `example`, and logins will be returned for "http://bar.example.co.uk",
- // but not for "http://notexample.com" or "https://example.foo.com".
- virtual void GetLoginsForSameOrganizationName(
- const std::string& signon_realm,
- PasswordStoreConsumer* consumer);
-
// Gets the complete list of PasswordForms that are not blacklist entries--and
// are thus auto-fillable. |consumer| will be notified on completion.
// The request will be cancelled if the consumer is destroyed.
@@ -423,11 +408,6 @@
virtual std::vector<std::unique_ptr<autofill::PasswordForm>>
FillMatchingLogins(const FormDigest& form) = 0;
- // Finds and returns all organization-name-matching logins, or returns an
- // empty list on error.
- virtual std::vector<std::unique_ptr<autofill::PasswordForm>>
- FillLoginsForSameOrganizationName(const std::string& signon_realm) = 0;
-
// Synchronous implementation for manipulating with statistics.
virtual void AddSiteStatsImpl(const InteractionsStats& stats) = 0;
virtual void RemoveSiteStatsImpl(const GURL& origin_domain) = 0;
@@ -576,11 +556,6 @@
std::vector<std::unique_ptr<autofill::PasswordForm>> GetLoginsImpl(
const FormDigest& form);
- // Finds all logins organization-name-matching |signon_realm| and returns the
- // result.
- std::vector<std::unique_ptr<autofill::PasswordForm>>
- GetLoginsForSameOrganizationNameImpl(const std::string& signon_realm);
-
// Finds all non-blacklist PasswordForms and returns the result.
std::vector<std::unique_ptr<autofill::PasswordForm>>
GetAutofillableLoginsImpl();
diff --git a/components/password_manager/core/browser/password_store_default.cc b/components/password_manager/core/browser/password_store_default.cc
index 961f67b..ce3a705e 100644
--- a/components/password_manager/core/browser/password_store_default.cc
+++ b/components/password_manager/core/browser/password_store_default.cc
@@ -173,16 +173,6 @@
return matched_forms;
}
-std::vector<std::unique_ptr<PasswordForm>>
-PasswordStoreDefault::FillLoginsForSameOrganizationName(
- const std::string& signon_realm) {
- std::vector<std::unique_ptr<PasswordForm>> forms;
- if (login_db_ &&
- !login_db_->GetLoginsForSameOrganizationName(signon_realm, &forms))
- return std::vector<std::unique_ptr<PasswordForm>>();
- return forms;
-}
-
bool PasswordStoreDefault::FillAutofillableLogins(
std::vector<std::unique_ptr<PasswordForm>>* forms) {
DCHECK(background_task_runner()->RunsTasksInCurrentSequence());
diff --git a/components/password_manager/core/browser/password_store_default.h b/components/password_manager/core/browser/password_store_default.h
index e3d7a92..6bfeff9 100644
--- a/components/password_manager/core/browser/password_store_default.h
+++ b/components/password_manager/core/browser/password_store_default.h
@@ -71,8 +71,6 @@
base::Time delete_end) override;
std::vector<std::unique_ptr<autofill::PasswordForm>> FillMatchingLogins(
const FormDigest& form) override;
- std::vector<std::unique_ptr<autofill::PasswordForm>>
- FillLoginsForSameOrganizationName(const std::string& signon_realm) override;
bool FillAutofillableLogins(
std::vector<std::unique_ptr<autofill::PasswordForm>>* forms) override;
bool FillBlacklistLogins(
diff --git a/components/password_manager/core/browser/password_store_unittest.cc b/components/password_manager/core/browser/password_store_unittest.cc
index 67e9d7ba..e344680 100644
--- a/components/password_manager/core/browser/password_store_unittest.cc
+++ b/components/password_manager/core/browser/password_store_unittest.cc
@@ -55,10 +55,6 @@
"https://psl.example.com/origin";
constexpr const char kTestUnrelatedWebRealm[] = "https://notexample.com/";
constexpr const char kTestUnrelatedWebOrigin[] = "https:/notexample.com/origin";
-constexpr const char kTestSameOrganizationNameWebRealm[] =
- "https://example.appspot.com/";
-constexpr const char kTestSameOrganizationNameWebOrigin[] =
- "https://example.appspot.com/origin";
constexpr const char kTestInsecureWebRealm[] = "http://one.example.com/";
constexpr const char kTestInsecureWebOrigin[] = "http://one.example.com/origin";
constexpr const char kTestAndroidRealm1[] =
@@ -851,59 +847,6 @@
store->ShutdownOnUIThread();
}
-TEST_F(PasswordStoreTest, GetLoginsForSameOrganizationName) {
- static constexpr const PasswordFormData kSameOrganizationCredentials[] = {
- // Credential that is an exact match of the observed form.
- {PasswordForm::SCHEME_HTML, kTestWebRealm1, kTestWebOrigin1, "", L"", L"",
- L"", L"username_value_1", L"", true, 1},
- // Credential that is a PSL match of the observed form.
- {PasswordForm::SCHEME_HTML, kTestPSLMatchingWebRealm,
- kTestPSLMatchingWebOrigin, "", L"", L"", L"", L"username_value_2", L"",
- true, 1},
- // Credential for the HTTP version of the observed form. (Should not be
- // filled, but returned as part of same-organization-name matches).
- {PasswordForm::SCHEME_HTML, kTestInsecureWebRealm, kTestInsecureWebOrigin,
- "", L"", L"", L"", L"username_value_3", L"", true, 1},
- // Credential for a signon realm with a different TLD, but same
- // organization identifying name.
- {PasswordForm::SCHEME_HTML, kTestSameOrganizationNameWebRealm,
- kTestSameOrganizationNameWebOrigin, "", L"", L"", L"",
- L"username_value_4", L"", true, 1},
- };
-
- static constexpr const PasswordFormData kNotSameOrganizationCredentials[] = {
- // Unrelated Web credential.
- {PasswordForm::SCHEME_HTML, kTestUnrelatedWebRealm,
- kTestUnrelatedWebOrigin, "", L"", L"", L"", L"username_value_5", L"",
- true, 1},
- // Credential for an affiliated Android application.
- {PasswordForm::SCHEME_HTML, kTestAndroidRealm1, "", "", L"", L"", L"",
- L"username_value_6", L"", true, 1}};
-
- scoped_refptr<PasswordStoreDefault> store(new PasswordStoreDefault(
- std::make_unique<LoginDatabase>(test_login_db_file_path())));
- store->Init(syncer::SyncableService::StartSyncFlare(), nullptr);
-
- std::vector<std::unique_ptr<PasswordForm>> expected_results;
- for (const auto& form_data : kSameOrganizationCredentials) {
- expected_results.push_back(FillPasswordFormWithData(form_data));
- store->AddLogin(*expected_results.back());
- }
-
- for (const auto& form_data : kNotSameOrganizationCredentials) {
- store->AddLogin(*FillPasswordFormWithData(form_data));
- }
-
- const std::string observed_form_realm = kTestWebRealm1;
- MockPasswordStoreConsumer mock_consumer;
- EXPECT_CALL(mock_consumer,
- OnGetPasswordStoreResultsConstRef(
- UnorderedPasswordFormElementsAre(&expected_results)));
- store->GetLoginsForSameOrganizationName(observed_form_realm, &mock_consumer);
- WaitForPasswordStore();
- store->ShutdownOnUIThread();
-}
-
#if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)
TEST_F(PasswordStoreTest, CheckPasswordReuse) {
static constexpr PasswordFormData kTestCredentials[] = {
diff --git a/components/password_manager/core/browser/suppressed_form_fetcher.cc b/components/password_manager/core/browser/suppressed_form_fetcher.cc
deleted file mode 100644
index 1a52b089..0000000
--- a/components/password_manager/core/browser/suppressed_form_fetcher.cc
+++ /dev/null
@@ -1,41 +0,0 @@
-// Copyright 2017 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 "components/password_manager/core/browser/suppressed_form_fetcher.h"
-
-#include "base/logging.h"
-#include "base/stl_util.h"
-#include "components/password_manager/core/browser/password_manager_client.h"
-#include "components/password_manager/core/browser/password_store.h"
-#include "url/gurl.h"
-
-namespace password_manager {
-
-SuppressedFormFetcher::SuppressedFormFetcher(
- const std::string& observed_signon_realm,
- const PasswordManagerClient* client,
- Consumer* consumer)
- : client_(client),
- consumer_(consumer),
- observed_signon_realm_(observed_signon_realm) {
- DCHECK(client_);
- DCHECK(consumer_);
- DCHECK(GURL(observed_signon_realm_).SchemeIsHTTPOrHTTPS());
- client_->GetPasswordStore()->GetLoginsForSameOrganizationName(
- observed_signon_realm_, this);
-}
-
-SuppressedFormFetcher::~SuppressedFormFetcher() = default;
-
-void SuppressedFormFetcher::OnGetPasswordStoreResults(
- std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
- base::EraseIf(results,
- [this](const std::unique_ptr<autofill::PasswordForm>& form) {
- return form->signon_realm == observed_signon_realm_;
- });
-
- consumer_->ProcessSuppressedForms(std::move(results));
-}
-
-} // namespace password_manager
diff --git a/components/password_manager/core/browser/suppressed_form_fetcher.h b/components/password_manager/core/browser/suppressed_form_fetcher.h
deleted file mode 100644
index 7f663494..0000000
--- a/components/password_manager/core/browser/suppressed_form_fetcher.h
+++ /dev/null
@@ -1,62 +0,0 @@
-// Copyright 2017 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 COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_SUPPRESSED_FORM_FETCHER_H_
-#define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_SUPPRESSED_FORM_FETCHER_H_
-
-#include <memory>
-#include <vector>
-
-#include "base/gtest_prod_util.h"
-#include "base/macros.h"
-#include "components/autofill/core/common/password_form.h"
-#include "components/password_manager/core/browser/password_store_consumer.h"
-
-namespace password_manager {
-
-class PasswordManagerClient;
-
-// Fetches credentials saved for the HTTPS counterpart of the given HTTP realm.
-//
-// Filling these HTTPS credentials into forms served over HTTP is obviously
-// suppressed, the purpose of doing such a query is to collect metrics on how
-// often this happens and inconveniences the user.
-//
-// This logic is implemented by this class, a separate PasswordStore consumer,
-// to make it very sure that these credentials will not get mistakenly filled.
-class SuppressedFormFetcher : public PasswordStoreConsumer {
- public:
- // Interface to be implemented by the consumer of this class.
- class Consumer {
- public:
- virtual void ProcessSuppressedForms(
- std::vector<std::unique_ptr<autofill::PasswordForm>> forms) = 0;
- };
-
- SuppressedFormFetcher(const std::string& observed_signon_realm,
- const PasswordManagerClient* client,
- Consumer* consumer);
- ~SuppressedFormFetcher() override;
-
- protected:
- // PasswordStoreConsumer:
- void OnGetPasswordStoreResults(
- std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
-
- private:
- FRIEND_TEST_ALL_PREFIXES(SuppressedFormFetcherTest, EmptyStore);
- FRIEND_TEST_ALL_PREFIXES(SuppressedFormFetcherTest, FullStore);
-
- // The client and the consumer should outlive |this|.
- const PasswordManagerClient* client_;
- Consumer* consumer_;
-
- const std::string observed_signon_realm_;
-
- DISALLOW_COPY_AND_ASSIGN(SuppressedFormFetcher);
-};
-
-} // namespace password_manager
-
-#endif // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_SUPPRESSED_FORM_FETCHER_H_
diff --git a/components/password_manager/core/browser/suppressed_form_fetcher_unittest.cc b/components/password_manager/core/browser/suppressed_form_fetcher_unittest.cc
deleted file mode 100644
index 97d9f7e..0000000
--- a/components/password_manager/core/browser/suppressed_form_fetcher_unittest.cc
+++ /dev/null
@@ -1,155 +0,0 @@
-// Copyright 2017 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 "components/password_manager/core/browser/suppressed_form_fetcher.h"
-
-#include "base/macros.h"
-#include "base/test/scoped_task_environment.h"
-#include "components/password_manager/core/browser/mock_password_store.h"
-#include "components/password_manager/core/browser/password_manager_test_utils.h"
-#include "components/password_manager/core/browser/stub_password_manager_client.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace password_manager {
-namespace {
-
-using autofill::PasswordForm;
-using testing::_;
-
-constexpr const char kTestHttpURL[] = "http://one.example.com/";
-constexpr const char kTestHttpsURL[] = "https://one.example.com/";
-constexpr const char kTestPSLMatchingHttpURL[] = "http://psl.example.com/";
-constexpr const char kTestPSLMatchingHttpsURL[] = "https://psl.example.com/";
-constexpr const char kTestHttpSameOrgNameURL[] = "http://login.example.co.uk/";
-constexpr const char kTestHttpsSameOrgNameURL[] =
- "https://login.example.co.uk/";
-
-class MockConsumer : public SuppressedFormFetcher::Consumer {
- public:
- MockConsumer() = default;
- ~MockConsumer() = default;
-
- // GMock still cannot mock methods with move-only args.
- MOCK_METHOD1(ProcessSuppressedHTTPSFormsConstRef,
- void(const std::vector<std::unique_ptr<PasswordForm>>&));
-
- protected:
- // SuppressedFormFetcher::Consumer:
- void ProcessSuppressedForms(
- std::vector<std::unique_ptr<PasswordForm>> forms) override {
- ProcessSuppressedHTTPSFormsConstRef(forms);
- }
-
- private:
- DISALLOW_COPY_AND_ASSIGN(MockConsumer);
-};
-
-class PasswordManagerClientWithMockStore : public StubPasswordManagerClient {
- public:
- PasswordManagerClientWithMockStore()
- : mock_store_(new ::testing::StrictMock<MockPasswordStore>()) {}
- ~PasswordManagerClientWithMockStore() override {
- mock_store_->ShutdownOnUIThread();
- }
-
- MockPasswordStore& mock_password_store() const { return *mock_store_; }
-
- protected:
- // StubPasswordManagerClient:
- PasswordStore* GetPasswordStore() const override { return mock_store_.get(); }
-
- private:
- scoped_refptr<MockPasswordStore> mock_store_;
-
- DISALLOW_COPY_AND_ASSIGN(PasswordManagerClientWithMockStore);
-};
-
-} // namespace
-
-class SuppressedFormFetcherTest : public testing::Test {
- public:
- SuppressedFormFetcherTest() = default;
- ~SuppressedFormFetcherTest() override = default;
-
- MockConsumer* mock_consumer() { return &consumer_; }
- MockPasswordStore* mock_store() { return &client_.mock_password_store(); }
- PasswordManagerClientWithMockStore* mock_client() { return &client_; }
-
- private:
- base::test::ScopedTaskEnvironment
- task_environment_; // Needed by the MockPasswordStore.
-
- MockConsumer consumer_;
- PasswordManagerClientWithMockStore client_;
-
- DISALLOW_COPY_AND_ASSIGN(SuppressedFormFetcherTest);
-};
-
-TEST_F(SuppressedFormFetcherTest, EmptyStore) {
- EXPECT_CALL(*mock_store(), GetLoginsForSameOrganizationName(kTestHttpURL, _));
- SuppressedFormFetcher suppressed_form_fetcher(kTestHttpURL, mock_client(),
- mock_consumer());
- EXPECT_CALL(*mock_consumer(),
- ProcessSuppressedHTTPSFormsConstRef(::testing::IsEmpty()));
- suppressed_form_fetcher.OnGetPasswordStoreResults(
- std::vector<std::unique_ptr<PasswordForm>>());
-}
-
-TEST_F(SuppressedFormFetcherTest, FullStore) {
- static constexpr const PasswordFormData kSuppressedCredentials[] = {
- // Credential that is for the HTTPS counterpart of the observed form.
- {PasswordForm::SCHEME_HTML, kTestHttpsURL, kTestHttpsURL, "", L"", L"",
- L"", L"username_value_1", L"password_value_1", true, 1},
- // Once again, but with a different username/password.
- {PasswordForm::SCHEME_HTML, kTestHttpsURL, kTestHttpsURL, "", L"", L"",
- L"", L"username_value_2", L"password_value_2", true, 1},
- // A PSL match to the observed form.
- {PasswordForm::SCHEME_HTML, kTestPSLMatchingHttpURL,
- kTestPSLMatchingHttpURL, "", L"", L"", L"", L"username_value_2",
- L"password_value_2", true, 1},
- // A PSL match to the HTTPS counterpart of the observed form. Note that
- // this is *not* a PSL match to the observed form, but a same organization
- // name match.
- {PasswordForm::SCHEME_HTML, kTestPSLMatchingHttpsURL,
- kTestPSLMatchingHttpsURL, "", L"", L"", L"", L"username_value_3",
- L"password_value_3", true, 1},
- // Credentials for a HTTP origin with the same organization
- // identifying name.
- {PasswordForm::SCHEME_HTML, kTestHttpSameOrgNameURL,
- kTestHttpSameOrgNameURL, "", L"", L"", L"", L"username_value_4",
- L"password_value_4", true, 1},
- // Credentials for a HTTPS origin with the same organization
- // identifying name.
- {PasswordForm::SCHEME_HTML, kTestHttpsSameOrgNameURL,
- kTestHttpsSameOrgNameURL, "", L"", L"", L"", L"username_value_5",
- L"password_value_5", true, 1}};
-
- static const PasswordFormData kNotSuppressedCredentials[] = {
- // Credential exactly matching the observed form.
- {PasswordForm::SCHEME_HTML, kTestHttpURL, kTestHttpURL, "", L"", L"", L"",
- L"username_value_1", L"password_value_1", true, 1},
- };
-
- std::vector<std::unique_ptr<PasswordForm>> simulated_store_results;
- std::vector<std::unique_ptr<PasswordForm>> expected_results;
- for (const auto& form_data : kSuppressedCredentials) {
- expected_results.push_back(FillPasswordFormWithData(form_data));
- simulated_store_results.push_back(FillPasswordFormWithData(form_data));
- }
- for (const auto& form_data : kNotSuppressedCredentials) {
- simulated_store_results.push_back(FillPasswordFormWithData(form_data));
- }
-
- EXPECT_CALL(*mock_store(), GetLoginsForSameOrganizationName(kTestHttpURL, _));
- SuppressedFormFetcher suppressed_form_fetcher(kTestHttpURL, mock_client(),
- mock_consumer());
- EXPECT_CALL(*mock_consumer(),
- ProcessSuppressedHTTPSFormsConstRef(
- UnorderedPasswordFormElementsAre(&expected_results)));
- suppressed_form_fetcher.OnGetPasswordStoreResults(
- std::move(simulated_store_results));
-}
-
-} // namespace password_manager
diff --git a/components/password_manager/core/browser/test_password_store.cc b/components/password_manager/core/browser/test_password_store.cc
index 80ba9bd0..654d5fee 100644
--- a/components/password_manager/core/browser/test_password_store.cc
+++ b/components/password_manager/core/browser/test_password_store.cc
@@ -143,16 +143,6 @@
return DatabaseCleanupResult::kSuccess;
}
-std::vector<std::unique_ptr<autofill::PasswordForm>>
-TestPasswordStore::FillLoginsForSameOrganizationName(
- const std::string& signon_realm) {
- // Note: To keep TestPasswordStore simple, and because no tests currently
- // require anything more complex, this is a simplistic implementation which
- // assumes that that the signon_realm is a serialised URL.
- return FillMatchingLogins(FormDigest(autofill::PasswordForm::SCHEME_HTML,
- signon_realm, GURL(signon_realm)));
-}
-
std::vector<InteractionsStats> TestPasswordStore::GetSiteStatsImpl(
const GURL& origin_domain) {
return std::vector<InteractionsStats>();
diff --git a/components/password_manager/core/browser/test_password_store.h b/components/password_manager/core/browser/test_password_store.h
index ba764c4..62107b98 100644
--- a/components/password_manager/core/browser/test_password_store.h
+++ b/components/password_manager/core/browser/test_password_store.h
@@ -59,8 +59,6 @@
bool FillBlacklistLogins(
std::vector<std::unique_ptr<autofill::PasswordForm>>* forms) override;
DatabaseCleanupResult DeleteUndecryptableLogins() override;
- std::vector<std::unique_ptr<autofill::PasswordForm>>
- FillLoginsForSameOrganizationName(const std::string& signon_realm) override;
std::vector<InteractionsStats> GetSiteStatsImpl(
const GURL& origin_domain) override;
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index 55c918dc..3f894723 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -43190,6 +43190,9 @@
</enum>
<enum name="PasswordManagerSuppressedAccountCrossActionsTaken">
+ <obsolete>
+ Deprecated 03/2019.
+ </obsolete>
<summary>
The value is a mixed-base encoding of the combination of four attributes:
whether there were suppressed stored credentials (and the kind if there
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index cdebabee..1021448 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -85068,6 +85068,9 @@
<histogram name="PasswordManager.QueryingSuppressedAccountsFinished"
enum="Boolean">
+ <obsolete>
+ Deprecated 03/2019.
+ </obsolete>
<owner>engedy@chromium.org</owner>
<summary>
Records, for each password form seen by the password manager, whether the
@@ -85509,6 +85512,9 @@
<histogram name="PasswordManager.SuppressedAccount"
enum="PasswordManagerSuppressedAccountCrossActionsTaken">
+ <obsolete>
+ Deprecated 03/2019.
+ </obsolete>
<owner>engedy@chromium.org</owner>
<summary>
Records, for each password form seen by the password manager, whether there
@@ -146745,6 +146751,9 @@
</histogram_suffixes>
<histogram_suffixes name="PasswordManagerSuppressedAccountReason" separator=".">
+ <obsolete>
+ Deprecated 03/2019.
+ </obsolete>
<suffix name="HTTPSNotHTTP"
label="The credential was suppressed because it was for an HTTPS origin
whereas the observed form was for an HTTP origin."/>
@@ -146761,6 +146770,9 @@
</histogram_suffixes>
<histogram_suffixes name="PasswordManagerSuppressedAccountType" separator=".">
+ <obsolete>
+ Deprecated 03/2019.
+ </obsolete>
<suffix name="Generated" label="The password was originally auto-generated."/>
<suffix name="Manual"
label="The password was originally typed in by the user."/>
diff --git a/tools/metrics/ukm/ukm.xml b/tools/metrics/ukm/ukm.xml
index 956d705..6f6ae823 100644
--- a/tools/metrics/ukm/ukm.xml
+++ b/tools/metrics/ukm/ukm.xml
@@ -4947,6 +4947,9 @@
</summary>
</metric>
<metric name="SuppressedAccount.Generated.HTTPSNotHTTP">
+ <obsolete>
+ Deprecated 03/2019.
+ </obsolete>
<summary>
Records, for each password form seen by the password manager, whether
there were `suppressed` credentials, meaning stored credentials that were
@@ -4960,6 +4963,9 @@
</summary>
</metric>
<metric name="SuppressedAccount.Generated.PSLMatching">
+ <obsolete>
+ Deprecated 03/2019.
+ </obsolete>
<summary>
Records, for each password form seen by the password manager, whether
there were `suppressed` credentials, meaning stored credentials that were
@@ -4974,6 +4980,9 @@
</summary>
</metric>
<metric name="SuppressedAccount.Generated.SameOrganizationName">
+ <obsolete>
+ Deprecated 03/2019.
+ </obsolete>
<summary>
Records, for each password form seen by the password manager, whether
there were `suppressed` credentials, meaning stored credentials that were
@@ -4987,6 +4996,9 @@
</summary>
</metric>
<metric name="SuppressedAccount.Manual.HTTPSNotHTTP">
+ <obsolete>
+ Deprecated 03/2019.
+ </obsolete>
<summary>
Records, for each password form seen by the password manager, whether
there were `suppressed` credentials, meaning stored credentials that were
@@ -5000,6 +5012,9 @@
</summary>
</metric>
<metric name="SuppressedAccount.Manual.PSLMatching">
+ <obsolete>
+ Deprecated 03/2019.
+ </obsolete>
<summary>
Records, for each password form seen by the password manager, whether
there were `suppressed` credentials, meaning stored credentials that were
@@ -5014,6 +5029,9 @@
</summary>
</metric>
<metric name="SuppressedAccount.Manual.SameOrganizationName">
+ <obsolete>
+ Deprecated 03/2019.
+ </obsolete>
<summary>
Records, for each password form seen by the password manager, whether
there were `suppressed` credentials, meaning stored credentials that were
