Skip to content

Commit

Permalink
Cleaned up feature AutofillUseConsistentPopupSettingsIcon
Browse files Browse the repository at this point in the history
Change-Id: I85a553dc07e4512b6d5a82ef9f557ecb734cf354
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4176926
Reviewed-by: Matthias Körber <koerber@google.com>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Commit-Queue: Jihad Hanna <jihadghanna@google.com>
Cr-Commit-Position: refs/heads/main@{#1097847}
  • Loading branch information
jihadghanna authored and Chromium LUCI CQ committed Jan 27, 2023
1 parent 5ec8ef5 commit 7197927
Show file tree
Hide file tree
Showing 10 changed files with 15 additions and 63 deletions.
6 changes: 0 additions & 6 deletions chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3356,12 +3356,6 @@ const FeatureEntry kFeatureEntries[] = {
flag_descriptions::kAutofillMoreProminentPopupName,
flag_descriptions::kAutofillMoreProminentPopupDescription, kOsDesktop,
FEATURE_VALUE_TYPE(autofill::features::kAutofillMoreProminentPopup)},
{"autofill-use-consistent-popup-settings-icons",
flag_descriptions::kAutofillUseConsistentPopupSettingsIconsName,
flag_descriptions::kAutofillUseConsistentPopupSettingsIconsDescription,
kOsDesktop,
FEATURE_VALUE_TYPE(
autofill::features::kAutofillUseConsistentPopupSettingsIcons)},
{"smooth-scrolling", flag_descriptions::kSmoothScrollingName,
flag_descriptions::kSmoothScrollingDescription,
// Mac has a separate implementation with its own setting to disable.
Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/flag-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -645,11 +645,6 @@
"owners": [ "jsaul@google.com", "payments-autofill-team@google.com" ],
"expiry_milestone": 112
},
{
"name": "autofill-use-consistent-popup-settings-icons",
"owners": [ "koerber", "mamir" ],
"expiry_milestone": 110
},
{
"name": "autofill-use-improved-label-disambiguation",
"owners": [ "autofill-squad-muc@google.com" ],
Expand Down
6 changes: 0 additions & 6 deletions chrome/browser/flag_descriptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -515,12 +515,6 @@ const char kAutofillRemoveCardExpirationAndTypeTitlesDescription[] =
"When enabled, expiration and type titles will be removed from the Chrome "
"Payment Settings page on desktop platforms.";

const char kAutofillUseConsistentPopupSettingsIconsName[] =
"Consistent Autofill settings icon";
const char kAutofillUseConsistentPopupSettingsIconsDescription[] =
"If enabled, all Autofill data types including addresses, credit cards and "
"passwords will use a consistent icon in the popup settings footer.";

const char kAutofillSaveAndFillVPAName[] =
"Offer save and autofill of UPI/VPA values";
const char kAutofillSaveAndFillVPADescription[] =
Expand Down
3 changes: 0 additions & 3 deletions chrome/browser/flag_descriptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -310,9 +310,6 @@ extern const char kAutofillUpstreamAllowAllEmailDomainsDescription[];
extern const char kAutofillMoreProminentPopupName[];
extern const char kAutofillMoreProminentPopupDescription[];

extern const char kAutofillUseConsistentPopupSettingsIconsName[];
extern const char kAutofillUseConsistentPopupSettingsIconsDescription[];

extern const char kAutofillUseImprovedLabelDisambiguationName[];
extern const char kAutofillUseImprovedLabelDisambiguationDescription[];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,7 @@ std::unique_ptr<views::ImageView> GetIconImageViewByName(
return ImageViewFromVectorIcon(kAccountCircleIcon);

if (icon_str == "settingsIcon") {
return ImageViewFromVectorIcon(
base::FeatureList::IsEnabled(
autofill::features::kAutofillUseConsistentPopupSettingsIcons)
? kMonoColorProductIcon
: vector_icons::kSettingsIcon);
return ImageViewFromVectorIcon(kMonoColorProductIcon);
}

if (icon_str == "empty")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -440,11 +440,7 @@ void AutofillExternalDelegate::ApplyAutofillOptions(
// popup layout experiment.
suggestions->push_back(Suggestion(GetSettingsSuggestionValue()));
suggestions->back().frontend_id = POPUP_ITEM_ID_AUTOFILL_OPTIONS;

if (base::FeatureList::IsEnabled(
features::kAutofillUseConsistentPopupSettingsIcons)) {
suggestions->back().icon = "settingsIcon";
}
suggestions->back().icon = "settingsIcon";

// On Android and Desktop, Google Pay branding is shown along with Settings.
// So Google Pay Icon is just attached to an existing menu item.
Expand Down
32 changes: 13 additions & 19 deletions components/autofill/core/browser/personal_data_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1425,27 +1425,21 @@ std::vector<Suggestion> PersonalDataManager::GetProfileSuggestions(
suggestion_selection::PrepareSuggestions(labels, &unique_suggestions,
comparator);

// If this feature is enabled, we add an icon to the address (profile)
// suggestion if there is more than on profile related field in the form.
if (base::FeatureList::IsEnabled(
features::kAutofillUseConsistentPopupSettingsIcons)) {
// Returns true if |type| is related to address profiles.
auto is_field_type_profile_related = [](ServerFieldType type) {
FieldTypeGroup group = AutofillType(type).group();
return group == FieldTypeGroup::kName ||
group == FieldTypeGroup::kAddressHome ||
group == FieldTypeGroup::kPhoneHome ||
group == FieldTypeGroup::kEmail;
};

if (base::ranges::count_if(field_types, is_field_type_profile_related) >
1) {
for (auto& suggestion : unique_suggestions) {
suggestion.icon = "accountIcon";
}
// We add an icon to the address (profile) suggestion if there is more than
// one profile related field in the form. Returns true if |type| is related to
// address profiles.
auto is_field_type_profile_related = [](ServerFieldType type) {
FieldTypeGroup group = AutofillType(type).group();
return group == FieldTypeGroup::kName ||
group == FieldTypeGroup::kAddressHome ||
group == FieldTypeGroup::kPhoneHome ||
group == FieldTypeGroup::kEmail;
};
if (base::ranges::count_if(field_types, is_field_type_profile_related) > 1) {
for (auto& suggestion : unique_suggestions) {
suggestion.icon = "accountIcon";
}
}

return unique_suggestions;
}

Expand Down
6 changes: 0 additions & 6 deletions components/autofill/core/common/autofill_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -556,12 +556,6 @@ BASE_FEATURE(kAutofillUseImprovedLabelDisambiguation,
"AutofillUseImprovedLabelDisambiguation",
base::FEATURE_DISABLED_BY_DEFAULT);

// Controls whether to use the same icon for the settings section in the popup
// footer.
BASE_FEATURE(kAutofillUseConsistentPopupSettingsIcons,
"AutofillUseConsistentPopupSettingsIcons",
base::FEATURE_ENABLED_BY_DEFAULT);

// Controls whether to use the combined heuristic and the autocomplete section
// implementation for section splitting or not. See https://crbug.com/1076175.
BASE_FEATURE(kAutofillUseNewSectioningMethod,
Expand Down
2 changes: 0 additions & 2 deletions components/autofill/core/common/autofill_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,6 @@ COMPONENT_EXPORT(AUTOFILL)
extern const base::FeatureParam<bool> kAutofillSectioningModeExpand;
COMPONENT_EXPORT(AUTOFILL)
BASE_DECLARE_FEATURE(kAutofillRefillByFormRendererId);
COMPONENT_EXPORT(AUTOFILL)
BASE_DECLARE_FEATURE(kAutofillUseConsistentPopupSettingsIcons);
COMPONENT_EXPORT(AUTOFILL) BASE_DECLARE_FEATURE(kAutofillEnableAblationStudy);
COMPONENT_EXPORT(AUTOFILL)
extern const base::FeatureParam<bool>
Expand Down
6 changes: 0 additions & 6 deletions testing/variations/fieldtrial_testing_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -5711,9 +5711,6 @@
"name": "Full Color - Profile Icon Disabled",
"enable_features": [
"AutofillBrandingIOS"
],
"disable_features": [
"AutofillUseConsistentPopupSettingsIcons"
]
},
{
Expand All @@ -5732,9 +5729,6 @@
},
"enable_features": [
"AutofillBrandingIOS"
],
"disable_features": [
"AutofillUseConsistentPopupSettingsIcons"
]
}
]
Expand Down

0 comments on commit 7197927

Please sign in to comment.