Revert "[mullet m3] Remove address profile filtering for manual fallback."
This reverts commit b7b5fb6000d8530d49dced09347c430d6dfd5fd1.
Reason for revert: This is leading to suggestions being displayed without a main text. We should check that in the M1 manualfallback case (address field), a main text exists, see https://screenshot.googleplex.com/3zfqc8Nvyuxdd5V
Original change's description:
> [mullet m3] Remove address profile filtering for manual fallback.
>
> Address profiles which were not used for > 180 days are not shown
> to the user. As agreed with with the UX team, all address profiles
> should be shown to the user if the Autofill was triggered manually.
>
> Bug: 1521250
> Change-Id: I900e6f14da0f54c2c61ebf4e51f86e1581758310
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5266938
> Reviewed-by: Jihad Hanna <jihadghanna@google.com>
> Commit-Queue: Timofey Chudakov <tchudakov@google.com>
> Cr-Commit-Position: refs/heads/main@{#1259250}
(cherry picked from commit df405cd9af45df0b69b4ce0147f89e6ceae27701)
Bug: 1521250
Change-Id: Ic4477e013acb6cea20af9730b46cc429b49e7bbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5339856
Reviewed-by: Timofey Chudakov <tchudakov@google.com>
Commit-Queue: Bruno Braga <brunobraga@google.com>
Reviewed-by: Jihad Hanna <jihadghanna@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1268103}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5354908
Reviewed-by: Florian Leimgruber <fleimgruber@google.com>
Cr-Commit-Position: refs/branch-heads/6312@{#505}
Cr-Branched-From: 6711dcdae48edaf98cbc6964f90fac85b7d9986e-refs/heads/main@{#1262506}
diff --git a/components/autofill/core/browser/autofill_suggestion_generator.cc b/components/autofill/core/browser/autofill_suggestion_generator.cc
index 81771f2..e7124f77 100644
--- a/components/autofill/core/browser/autofill_suggestion_generator.cc
+++ b/components/autofill/core/browser/autofill_suggestion_generator.cc
@@ -1137,21 +1137,18 @@
FieldType trigger_field_type,
std::optional<FieldTypeSet> last_targeted_fields,
AutofillSuggestionTriggerSource trigger_source) {
+ // If the user manually triggered suggestions from the context menu, all
+ // available profiles should be shown. Selecting a suggestion overwrites the
+ // triggering field's value.
+ const std::u16string field_value_for_filtering =
+ trigger_source != AutofillSuggestionTriggerSource::kManualFallbackAddress
+ ? trigger_field.value
+ : u"";
+
std::vector<raw_ptr<const AutofillProfile, VectorExperimental>>
- profiles_to_suggest;
- if (IsAddressAutofillManuallyTriggered(trigger_source)) {
- // If the user manually triggered suggestions from the context menu, all
- // available profiles should be shown. Selecting a suggestion overwrites the
- // triggering field's value.
- for (const AutofillProfile* profile :
- personal_data_->GetProfilesToSuggest()) {
- profiles_to_suggest.push_back(profile);
- }
- } else {
- profiles_to_suggest =
- GetProfilesToSuggest(trigger_field_type, trigger_field.value,
- trigger_field.is_autofilled, field_types);
- }
+ profiles_to_suggest =
+ GetProfilesToSuggest(trigger_field_type, field_value_for_filtering,
+ trigger_field.is_autofilled, field_types);
// Find the profiles that were hidden prior to the effects of the feature
// kAutofillUseAddressRewriterInProfileSubsetComparison.
@@ -1169,15 +1166,13 @@
// main text, to be filled in the triggering field, differs regardless of
// the other fields.
std::vector<raw_ptr<const AutofillProfile, VectorExperimental>>
- previously_suggested_profiles;
- if (IsAddressAutofillManuallyTriggered(trigger_source) ||
- street_address_field_types.contains(trigger_field_type)) {
- previously_suggested_profiles = profiles_to_suggest;
- } else {
- previously_suggested_profiles = GetProfilesToSuggest(
- trigger_field_type, trigger_field.value, trigger_field.is_autofilled,
- field_types_without_address_types);
- }
+ previously_suggested_profiles =
+ street_address_field_types.contains(trigger_field_type)
+ ? profiles_to_suggest
+ : GetProfilesToSuggest(trigger_field_type,
+ field_value_for_filtering,
+ trigger_field.is_autofilled,
+ field_types_without_address_types);
for (const AutofillProfile* profile : previously_suggested_profiles) {
previously_hidden_profiles_guid.erase(profile->guid());
}
diff --git a/components/autofill/core/browser/autofill_suggestion_generator_unittest.cc b/components/autofill/core/browser/autofill_suggestion_generator_unittest.cc
index 952deb1..aa70e12 100644
--- a/components/autofill/core/browser/autofill_suggestion_generator_unittest.cc
+++ b/components/autofill/core/browser/autofill_suggestion_generator_unittest.cc
@@ -2025,36 +2025,6 @@
EXPECT_THAT(manual_fallback_suggestions, ContainsAddressFooterSuggestions());
}
-// Tests that regular suggestions are filtered by the last usage timestamp, but
-// manual fallback suggestions are not.
-TEST_F(AutofillSuggestionGeneratorTest,
- GetSuggestionsForProfiles_TimestampFiltering) {
- AutofillProfile profile1 = test::GetFullProfile();
- AutofillProfile profile2 = test::GetFullProfile2();
- profile2.set_use_date(AutofillClock::Now() - kDisusedDataModelTimeDelta -
- base::Days(1));
- personal_data().AddProfile(profile1);
- personal_data().AddProfile(profile2);
-
- // Expect that regular suggestions filter.
- std::vector<Suggestion> address_suggestions =
- suggestion_generator()->GetSuggestionsForProfiles(
- {NAME_FIRST}, FormFieldData(), NAME_FIRST,
- /*last_targeted_fields=*/std::nullopt,
- AutofillSuggestionTriggerSource::kFormControlElementClicked);
- EXPECT_EQ(address_suggestions.size(), 3ul);
- EXPECT_THAT(address_suggestions, ContainsAddressFooterSuggestions());
-
- // But manual fallback suggestions do not.
- std::vector<Suggestion> manual_fallback_suggestions =
- suggestion_generator()->GetSuggestionsForProfiles(
- {NAME_FIRST}, FormFieldData(), NAME_FIRST,
- /*last_targeted_fields=*/std::nullopt,
- AutofillSuggestionTriggerSource::kManualFallbackAddress);
- EXPECT_EQ(manual_fallback_suggestions.size(), 4ul);
- EXPECT_THAT(manual_fallback_suggestions, ContainsAddressFooterSuggestions());
-}
-
// TODO(crbug.com/1441410): Clean up when the feature is launched.
TEST_F(AutofillSuggestionGeneratorTest, ClearAddressFormSuggestion) {
base::test::ScopedFeatureList features;
diff --git a/components/autofill/core/browser/browser_autofill_manager_unittest.cc b/components/autofill/core/browser/browser_autofill_manager_unittest.cc
index 086239c..35c2c4e 100644
--- a/components/autofill/core/browser/browser_autofill_manager_unittest.cc
+++ b/components/autofill/core/browser/browser_autofill_manager_unittest.cc
@@ -1441,12 +1441,12 @@
GetAutofillSuggestions(form, first_field);
external_delegate()->CheckSuggestionsNotReturned(first_field.global_id());
- // Expect 3 address suggestions + footer because the fixture created three
+ // Expect 2 address suggestions + footer because the fixture created three
// profiles during set up (see `CreateTestAutofillProfiles()`).
GetAutofillSuggestions(
form, first_field,
AutofillSuggestionTriggerSource::kManualFallbackAddress);
- external_delegate()->CheckSuggestionCount(first_field.global_id(), 5);
+ external_delegate()->CheckSuggestionCount(first_field.global_id(), 4);
// Expect 3 credit card suggestions + footer because the fixture created 3
// credit cards during setup (see `CreateTestCreditCards()`).
GetAutofillSuggestions(
@@ -1468,12 +1468,13 @@
GetAutofillSuggestions(form, first_field);
external_delegate()->CheckNoSuggestions(first_field.global_id());
- // Expect 3 address suggestions + footer because the fixture created three
- // profiles during set up (see `CreateTestAutofillProfiles()`).
+ // Expect 2 address suggestions + footer because the fixture created three
+ // profiles during set up, one of which is empty and cannot be suggested
+ // (see `CreateTestAutofillProfiles()`).
GetAutofillSuggestions(
form, first_field,
AutofillSuggestionTriggerSource::kManualFallbackAddress);
- external_delegate()->CheckSuggestionCount(first_field.global_id(), 5);
+ external_delegate()->CheckSuggestionCount(first_field.global_id(), 4);
// Expect 4 credit card suggestions + footer because the fixture created 3
// credit cards during setup (see `CreateTestCreditCards()`).
GetAutofillSuggestions(
@@ -1496,11 +1497,12 @@
FormsSeen({form});
for (const auto& field : form.fields) {
- // Expect 3 address suggestions + footer because the fixture created three
- // profiles during set up (see `CreateTestAutofillProfiles()`).
+ // Expect 2 address suggestions + footer because the fixture created three
+ // profiles during set up, one of which is empty and cannot be suggested
+ // (see `CreateTestAutofillProfiles()`).
GetAutofillSuggestions(
form, field, AutofillSuggestionTriggerSource::kManualFallbackAddress);
- external_delegate()->CheckSuggestionCount(field.global_id(), 5);
+ external_delegate()->CheckSuggestionCount(field.global_id(), 4);
base::ranges::all_of(
external_delegate()->suggestions(), [](const Suggestion& suggestion) {
return suggestion.popup_item_id == PopupItemId::kAddressEntry;
@@ -1528,12 +1530,12 @@
FormsSeen({form});
const FormFieldData& cc_name_field = form.fields[0];
- // Expect 3 address suggestions + footer because the fixture created three
+ // Expect 2 address suggestions + footer because the fixture created three
// profiles during set up (see `CreateTestAutofillProfiles()`).
GetAutofillSuggestions(
form, cc_name_field,
AutofillSuggestionTriggerSource::kManualFallbackAddress);
- external_delegate()->CheckSuggestionCount(cc_name_field.global_id(), 5);
+ external_delegate()->CheckSuggestionCount(cc_name_field.global_id(), 4);
// Expect 2 credit card suggestions + footer because manual fallback flow
// triggered on a classified credit card field should generate regular
// suggestions.