Commit 098c841d authored by Mohamed Amir Yosef's avatar Mohamed Amir Yosef Committed by Commit Bot

[Passwords] Suppress account storage promo for blocked websites...

... in the password autofill dropdown.

If the user opted to never save passwords for a specific website,
it might be confusing to see a dropdown menu item with a promo for
account storage.

This CL add a feature to suppress that.

Bug: 1144755
Change-Id: I149b2630587f1afefebbb7a673b3c07d9593c97f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2531774Reviewed-by: default avatarMarc Treib <treib@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826250}
parent fd2281f3
...@@ -29,6 +29,13 @@ using Logger = autofill::SavePasswordProgressLogger; ...@@ -29,6 +29,13 @@ using Logger = autofill::SavePasswordProgressLogger;
namespace password_manager { namespace password_manager {
namespace { namespace {
// Controls whether we should suppress the account storage promos for websites
// that are blocked by the user.
const base::Feature kSuppressAccountStoragePromosForBlockedWebsite{
"SuppressAccountStoragePromosForBlockedWebsite",
base::FEATURE_DISABLED_BY_DEFAULT};
bool PreferredRealmIsFromAndroid(const PasswordFormFillData& fill_data) { bool PreferredRealmIsFromAndroid(const PasswordFormFillData& fill_data) {
return FacetURI::FromPotentiallyInvalidSpec(fill_data.preferred_realm) return FacetURI::FromPotentiallyInvalidSpec(fill_data.preferred_realm)
.IsValidAndroidFacetURI(); .IsValidAndroidFacetURI();
...@@ -126,6 +133,7 @@ LikelyFormFilling SendFillInformationToRenderer( ...@@ -126,6 +133,7 @@ LikelyFormFilling SendFillInformationToRenderer(
const std::vector<const PasswordForm*>& best_matches, const std::vector<const PasswordForm*>& best_matches,
const std::vector<const PasswordForm*>& federated_matches, const std::vector<const PasswordForm*>& federated_matches,
const PasswordForm* preferred_match, const PasswordForm* preferred_match,
bool blocked_by_user,
PasswordFormMetricsRecorder* metrics_recorder) { PasswordFormMetricsRecorder* metrics_recorder) {
DCHECK(driver); DCHECK(driver);
DCHECK_EQ(PasswordForm::Scheme::kHtml, observed_form.scheme); DCHECK_EQ(PasswordForm::Scheme::kHtml, observed_form.scheme);
...@@ -141,10 +149,15 @@ LikelyFormFilling SendFillInformationToRenderer( ...@@ -141,10 +149,15 @@ LikelyFormFilling SendFillInformationToRenderer(
} }
if (best_matches.empty()) { if (best_matches.empty()) {
bool should_suppres_popup =
blocked_by_user && base::FeatureList::IsEnabled(
kSuppressAccountStoragePromosForBlockedWebsite);
bool should_show_popup_without_passwords = bool should_show_popup_without_passwords =
client->GetPasswordFeatureManager()->ShouldShowAccountStorageOptIn() || !should_suppres_popup &&
(client->GetPasswordFeatureManager()->ShouldShowAccountStorageOptIn() ||
client->GetPasswordFeatureManager()->ShouldShowAccountStorageReSignin( client->GetPasswordFeatureManager()->ShouldShowAccountStorageReSignin(
client->GetLastCommittedURL()); client->GetLastCommittedURL()));
driver->InformNoSavedCredentials(should_show_popup_without_passwords); driver->InformNoSavedCredentials(should_show_popup_without_passwords);
metrics_recorder->RecordFillEvent( metrics_recorder->RecordFillEvent(
PasswordFormMetricsRecorder::kManagerFillEventNoCredential); PasswordFormMetricsRecorder::kManagerFillEventNoCredential);
......
...@@ -46,6 +46,7 @@ LikelyFormFilling SendFillInformationToRenderer( ...@@ -46,6 +46,7 @@ LikelyFormFilling SendFillInformationToRenderer(
const std::vector<const PasswordForm*>& best_matches, const std::vector<const PasswordForm*>& best_matches,
const std::vector<const PasswordForm*>& federated_matches, const std::vector<const PasswordForm*>& federated_matches,
const PasswordForm* preferred_match, const PasswordForm* preferred_match,
bool blocked_by_user,
PasswordFormMetricsRecorder* metrics_recorder); PasswordFormMetricsRecorder* metrics_recorder);
// Create a PasswordFormFillData structure in preparation for filling a form // Create a PasswordFormFillData structure in preparation for filling a form
......
...@@ -150,7 +150,7 @@ TEST_F(PasswordFormFillingTest, NoSavedCredentials) { ...@@ -150,7 +150,7 @@ TEST_F(PasswordFormFillingTest, NoSavedCredentials) {
LikelyFormFilling likely_form_filling = SendFillInformationToRenderer( LikelyFormFilling likely_form_filling = SendFillInformationToRenderer(
&client_, &driver_, observed_form_, best_matches, federated_matches_, &client_, &driver_, observed_form_, best_matches, federated_matches_,
nullptr, metrics_recorder_.get()); nullptr, /*blocked_by_user=*/false, metrics_recorder_.get());
EXPECT_EQ(LikelyFormFilling::kNoFilling, likely_form_filling); EXPECT_EQ(LikelyFormFilling::kNoFilling, likely_form_filling);
} }
...@@ -169,7 +169,7 @@ TEST_F(PasswordFormFillingTest, Autofill) { ...@@ -169,7 +169,7 @@ TEST_F(PasswordFormFillingTest, Autofill) {
LikelyFormFilling likely_form_filling = SendFillInformationToRenderer( LikelyFormFilling likely_form_filling = SendFillInformationToRenderer(
&client_, &driver_, observed_form_, best_matches, federated_matches_, &client_, &driver_, observed_form_, best_matches, federated_matches_,
&saved_match_, metrics_recorder_.get()); &saved_match_, /*blocked_by_user=*/false, metrics_recorder_.get());
// On Android Touch To Fill will prevent autofilling credentials on page load. // On Android Touch To Fill will prevent autofilling credentials on page load.
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
...@@ -241,7 +241,7 @@ TEST_F(PasswordFormFillingTest, TestFillOnLoadSuggestion) { ...@@ -241,7 +241,7 @@ TEST_F(PasswordFormFillingTest, TestFillOnLoadSuggestion) {
LikelyFormFilling likely_form_filling = SendFillInformationToRenderer( LikelyFormFilling likely_form_filling = SendFillInformationToRenderer(
&client_, &driver_, observed_form, best_matches, federated_matches_, &client_, &driver_, observed_form, best_matches, federated_matches_,
&saved_match_, metrics_recorder_.get()); &saved_match_, /*blocked_by_user=*/false, metrics_recorder_.get());
// In all cases where a current password exists, fill on load should be // In all cases where a current password exists, fill on load should be
// permitted. Otherwise, the renderer will not fill anyway and return // permitted. Otherwise, the renderer will not fill anyway and return
...@@ -314,7 +314,7 @@ TEST_F(PasswordFormFillingTest, TestFillOnLoadSuggestionWithPrefill) { ...@@ -314,7 +314,7 @@ TEST_F(PasswordFormFillingTest, TestFillOnLoadSuggestionWithPrefill) {
LikelyFormFilling likely_form_filling = SendFillInformationToRenderer( LikelyFormFilling likely_form_filling = SendFillInformationToRenderer(
&client_, &driver_, observed_form, best_matches, federated_matches_, &client_, &driver_, observed_form, best_matches, federated_matches_,
&preferred_match, metrics_recorder_.get()); &preferred_match, /*blocked_by_user=*/false, metrics_recorder_.get());
EXPECT_EQ(test_case.likely_form_filling, likely_form_filling); EXPECT_EQ(test_case.likely_form_filling, likely_form_filling);
} }
...@@ -331,7 +331,7 @@ TEST_F(PasswordFormFillingTest, AutofillPSLMatch) { ...@@ -331,7 +331,7 @@ TEST_F(PasswordFormFillingTest, AutofillPSLMatch) {
LikelyFormFilling likely_form_filling = SendFillInformationToRenderer( LikelyFormFilling likely_form_filling = SendFillInformationToRenderer(
&client_, &driver_, observed_form_, best_matches, federated_matches_, &client_, &driver_, observed_form_, best_matches, federated_matches_,
&psl_saved_match_, metrics_recorder_.get()); &psl_saved_match_, /*blocked_by_user=*/false, metrics_recorder_.get());
EXPECT_EQ(LikelyFormFilling::kFillOnAccountSelect, likely_form_filling); EXPECT_EQ(LikelyFormFilling::kFillOnAccountSelect, likely_form_filling);
// Check that the message to the renderer (i.e. |fill_data|) is filled // Check that the message to the renderer (i.e. |fill_data|) is filled
...@@ -362,7 +362,7 @@ TEST_F(PasswordFormFillingTest, NoAutofillOnHttp) { ...@@ -362,7 +362,7 @@ TEST_F(PasswordFormFillingTest, NoAutofillOnHttp) {
EXPECT_CALL(client_, IsCommittedMainFrameSecure).WillOnce(Return(false)); EXPECT_CALL(client_, IsCommittedMainFrameSecure).WillOnce(Return(false));
LikelyFormFilling likely_form_filling = SendFillInformationToRenderer( LikelyFormFilling likely_form_filling = SendFillInformationToRenderer(
&client_, &driver_, observed_http_form, best_matches, federated_matches_, &client_, &driver_, observed_http_form, best_matches, federated_matches_,
&saved_http_match, metrics_recorder_.get()); &saved_http_match, /*blocked_by_user=*/false, metrics_recorder_.get());
EXPECT_EQ(LikelyFormFilling::kFillOnAccountSelect, likely_form_filling); EXPECT_EQ(LikelyFormFilling::kFillOnAccountSelect, likely_form_filling);
} }
...@@ -372,7 +372,7 @@ TEST_F(PasswordFormFillingTest, TouchToFill) { ...@@ -372,7 +372,7 @@ TEST_F(PasswordFormFillingTest, TouchToFill) {
LikelyFormFilling likely_form_filling = SendFillInformationToRenderer( LikelyFormFilling likely_form_filling = SendFillInformationToRenderer(
&client_, &driver_, observed_form_, best_matches, federated_matches_, &client_, &driver_, observed_form_, best_matches, federated_matches_,
&saved_match_, metrics_recorder_.get()); &saved_match_, /*blocked_by_user=*/false, metrics_recorder_.get());
EXPECT_EQ(LikelyFormFilling::kFillOnAccountSelect, likely_form_filling); EXPECT_EQ(LikelyFormFilling::kFillOnAccountSelect, likely_form_filling);
} }
#endif #endif
......
...@@ -817,7 +817,8 @@ void PasswordFormManager::Fill() { ...@@ -817,7 +817,8 @@ void PasswordFormManager::Fill() {
SendFillInformationToRenderer( SendFillInformationToRenderer(
client_, driver_.get(), *observed_password_form.get(), client_, driver_.get(), *observed_password_form.get(),
form_fetcher_->GetBestMatches(), form_fetcher_->GetFederatedMatches(), form_fetcher_->GetBestMatches(), form_fetcher_->GetFederatedMatches(),
form_fetcher_->GetPreferredMatch(), metrics_recorder_.get()); form_fetcher_->GetPreferredMatch(), form_fetcher_->IsBlacklisted(),
metrics_recorder_.get());
} }
void PasswordFormManager::FillForm( void PasswordFormManager::FillForm(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment