[Autofill] Delete features to turn off disable/delete unused data

Bug: 943700
Change-Id: I00df18d2ef02a106b622058b58a5b793ebdf6720
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1531383Reviewed-by: Roger McFarlane <rogerm@chromium.org>
Reviewed-by: Parastoo Geranmayeh <parastoog@google.com>
Commit-Queue: Fabio Tirelo <ftirelo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644872}

components/autofill/core/browser/personal_data_manager.cc

@@ -1138,16 +1138,13 @@ std::vector<Suggestion> PersonalDataManager::GetProfileSuggestions(
   // Get the profiles to suggest, which are already sorted.
   std::vector<AutofillProfile*> sorted_profiles = GetProfilesToSuggest();
 
-  // When suggesting with no prefix to match, consider suppressing disused
-  // address suggestions as well as those based on invalid profile data.
+  // When suggesting with no prefix to match, suppress disused address
+  // suggestions as well as those based on invalid profile data.
   if (field_contents_canon.empty()) {
-    if (base::FeatureList::IsEnabled(
-            features::kAutofillSuppressDisusedAddresses)) {
-      const base::Time min_last_used =
-          AutofillClock::Now() - kDisusedDataModelTimeDelta;
-      suggestion_selection::RemoveProfilesNotUsedSinceTimestamp(
-          min_last_used, &sorted_profiles);
-    }
+    const base::Time min_last_used =
+        AutofillClock::Now() - kDisusedDataModelTimeDelta;
+    suggestion_selection::RemoveProfilesNotUsedSinceTimestamp(min_last_used,
+                                                              &sorted_profiles);
   }
 
   std::vector<AutofillProfile*> matched_profiles;
@@ -1246,11 +1243,8 @@ std::vector<Suggestion> PersonalDataManager::GetCreditCardSuggestions(
     return std::vector<Suggestion>();
   std::vector<CreditCard*> cards =
       GetCreditCardsToSuggest(include_server_cards);
-  // If enabled, suppress disused address profiles when triggered from an empty
-  // field.
-  if (field_contents.empty() &&
-      base::FeatureList::IsEnabled(
-          features::kAutofillSuppressDisusedCreditCards)) {
+  // Suppress disused address profiles when triggered from an empty field.
+  if (field_contents.empty()) {
     const base::Time min_last_used =
         AutofillClock::Now() - kDisusedDataModelTimeDelta;
     RemoveExpiredCreditCardsNotUsedSinceTimestamp(AutofillClock::Now(),
@@ -2483,11 +2477,6 @@ std::string PersonalDataManager::MergeServerAddressesIntoProfiles(
 }
 
 bool PersonalDataManager::DeleteDisusedCreditCards() {
-  if (!base::FeatureList::IsEnabled(
-          features::kAutofillDeleteDisusedCreditCards)) {
-    return false;
-  }
-
   // Only delete local cards, as server cards are managed by Payments.
   auto cards = GetLocalCreditCards();
 
@@ -2519,12 +2508,6 @@ bool PersonalDataManager::DeleteDisusedCreditCards() {
 }
 
 bool PersonalDataManager::DeleteDisusedAddresses() {
-  if (!base::FeatureList::IsEnabled(
-          features::kAutofillDeleteDisusedAddresses)) {
-    DVLOG(1) << "Deletion is disabled";
-    return false;
-  }
-
   const std::vector<AutofillProfile*>& profiles = GetProfiles();
 
   // Early exit when there are no profiles.

components/autofill/core/browser/personal_data_manager_unittest.cc

@@ -2229,10 +2229,6 @@ TEST_F(PersonalDataManagerTest,
 
   ResetPersonalDataManager(USER_MODE_NORMAL);
 
-  base::test::ScopedFeatureList scoped_features;
-  scoped_features.InitAndEnableFeature(
-      features::kAutofillSuppressDisusedAddresses);
-
   // Query with empty string only returns profile2.
   {
     std::vector<Suggestion> suggestions = personal_data_->GetProfileSuggestions(
@@ -2270,23 +2266,6 @@ TEST_F(PersonalDataManagerTest,
         base::ASCIIToUTF16("456 Zoo St., Second Line, Third line, unit 5"),
         suggestions[0].value);
   }
-
-  // When suppression is disabled, returns all suggestions.
-  {
-    base::test::ScopedFeatureList scoped_features;
-    scoped_features.InitAndDisableFeature(
-        features::kAutofillSuppressDisusedAddresses);
-    std::vector<Suggestion> suggestions = personal_data_->GetProfileSuggestions(
-        AutofillType(ADDRESS_HOME_STREET_ADDRESS), base::string16(), false,
-        std::vector<ServerFieldType>());
-    ASSERT_EQ(2U, suggestions.size());
-    EXPECT_EQ(
-        base::ASCIIToUTF16("456 Zoo St., Second Line, Third line, unit 5"),
-        suggestions[0].value);
-    EXPECT_EQ(
-        base::ASCIIToUTF16("123 Zoo St., Second Line, Third line, unit 5"),
-        suggestions[1].value);
-  }
 }
 
 // Tests that suggestions based on invalid data are handled correctly.
@@ -2294,7 +2273,7 @@ TEST_F(PersonalDataManagerTest, GetProfileSuggestions_Validity) {
   // Set up 2 different profiles: one valid and one invalid.
   AutofillProfile valid_profile(test::GetFullValidProfileForCanada());
   valid_profile.set_use_date(AutofillClock::Now() -
-                             base::TimeDelta::FromDays(200));
+                             base::TimeDelta::FromDays(1));
   valid_profile.set_use_count(1);
   AddProfileToPersonalDataManager(valid_profile);
 
@@ -2317,7 +2296,7 @@ TEST_F(PersonalDataManagerTest, GetProfileSuggestions_Validity) {
     scoped_features.InitWithFeatures(
         /*enabled_features=*/{features::kAutofillProfileServerValidation,
                               features::kAutofillProfileClientValidation},
-        /*disabled_features=*/{features::kAutofillSuppressDisusedAddresses});
+        /*disabled_features=*/{});
     std::vector<Suggestion> email_suggestions =
         personal_data_->GetProfileSuggestions(AutofillType(EMAIL_ADDRESS),
                                               base::string16(), false,
@@ -2366,7 +2345,7 @@ TEST_F(PersonalDataManagerTest, GetProfileSuggestions_Validity) {
     scoped_features.InitWithFeatures(
         /*enabled_features=*/{features::kAutofillProfileClientValidation,
                               features::kAutofillProfileServerValidation},
-        /*disabled_features=*/{features::kAutofillSuppressDisusedAddresses});
+        /*disabled_features=*/{});
 
     std::vector<Suggestion> email_suggestions =
         personal_data_->GetProfileSuggestions(AutofillType(EMAIL_ADDRESS),
@@ -2396,8 +2375,7 @@ TEST_F(PersonalDataManagerTest, GetProfileSuggestions_Validity) {
     base::test::ScopedFeatureList scoped_features;
     scoped_features.InitWithFeatures(
         /*enabled_features=*/{features::kAutofillProfileClientValidation},
-        /*disabled_features=*/{features::kAutofillSuppressDisusedAddresses,
-                               features::kAutofillProfileServerValidation});
+        /*disabled_features=*/{features::kAutofillProfileServerValidation});
     std::vector<Suggestion> email_suggestions =
         personal_data_->GetProfileSuggestions(AutofillType(EMAIL_ADDRESS),
                                               base::string16(), false,
@@ -2427,8 +2405,7 @@ TEST_F(PersonalDataManagerTest, GetProfileSuggestions_Validity) {
     base::test::ScopedFeatureList scoped_features;
     scoped_features.InitWithFeatures(
         /*enabled_features=*/{features::kAutofillProfileServerValidation},
-        /*disabled_features=*/{features::kAutofillProfileClientValidation,
-                               features::kAutofillSuppressDisusedAddresses});
+        /*disabled_features=*/{features::kAutofillProfileClientValidation});
     LOG(ERROR) << __FUNCTION__;
     std::vector<Suggestion> email_suggestions =
         personal_data_->GetProfileSuggestions(AutofillType(EMAIL_ADDRESS),
@@ -3081,27 +3058,6 @@ TEST_F(PersonalDataManagerTest,
   WaitForOnPersonalDataChanged();
   ASSERT_EQ(4U, personal_data_->GetCreditCards().size());
 
-  // Verify no suppression if feature is disabled.
-  {
-    base::test::ScopedFeatureList scoped_features;
-    scoped_features.InitAndDisableFeature(
-        features::kAutofillSuppressDisusedCreditCards);
-
-    std::vector<Suggestion> suggestions =
-        personal_data_->GetCreditCardSuggestions(
-            AutofillType(CREDIT_CARD_NAME_FULL), base::string16(),
-            /*include_server_cards=*/true);
-    EXPECT_EQ(4U, suggestions.size());
-    EXPECT_EQ(base::ASCIIToUTF16("Bonnie Parker"), suggestions[0].value);
-    EXPECT_EQ(base::ASCIIToUTF16("Clyde Barrow"), suggestions[1].value);
-    EXPECT_EQ(base::ASCIIToUTF16("Jane Doe"), suggestions[2].value);
-    EXPECT_EQ(base::ASCIIToUTF16("John Dillinger"), suggestions[3].value);
-  }
-
-  base::test::ScopedFeatureList scoped_features;
-  scoped_features.InitAndEnableFeature(
-      features::kAutofillSuppressDisusedCreditCards);
-
   // Query with empty string only returns card0 and card1. Note expired
   // masked card2 is not suggested on empty fields.
   {
@@ -5079,31 +5035,10 @@ TEST_F(PersonalDataManagerTest, ApplyDedupingRoutine_OncePerVersion) {
   EXPECT_EQ(2U, personal_data_->GetProfiles().size());
 }
 
-// Tests that DeleteDisusedAddresses is not run if the feature is disabled.
-TEST_F(PersonalDataManagerTest, DeleteDisusedAddresses_DoNothingWhenDisabled) {
-  base::test::ScopedFeatureList scoped_features;
-  scoped_features.InitAndDisableFeature(
-      features::kAutofillDeleteDisusedAddresses);
-
-  CreateDeletableDisusedProfile();
-
-  // DeleteDisusedCreditCards should return false to indicate it was not run.
-  EXPECT_FALSE(personal_data_->DeleteDisusedAddresses());
-
-  personal_data_->Refresh();
-
-  EXPECT_EQ(1U, personal_data_->GetProfiles().size());
-}
-
 // Tests that DeleteDisusedAddresses only deletes the addresses that are
 // supposed to be deleted.
 TEST_F(PersonalDataManagerTest,
        DeleteDisusedAddresses_DeleteDesiredAddressesOnly) {
-  // Enable the feature.
-  base::test::ScopedFeatureList scoped_features;
-  scoped_features.InitAndEnableFeature(
-      features::kAutofillDeleteDisusedAddresses);
-
   auto now = AutofillClock::Now();
 
   // Create unverified/disused/not-used-by-valid-credit-card
@@ -5182,31 +5117,9 @@ TEST_F(PersonalDataManagerTest,
             personal_data_->GetProfiles()[2]->GetRawInfo(NAME_LAST));
 }
 
-// Tests that DeleteDisusedCreditCards is not run if the feature is disabled.
-TEST_F(PersonalDataManagerTest,
-       DeleteDisusedCreditCards_DoNothingWhenDisabled) {
-  base::test::ScopedFeatureList scoped_features;
-  scoped_features.InitAndDisableFeature(
-      features::kAutofillDeleteDisusedCreditCards);
-
-  CreateDeletableExpiredAndDisusedCreditCard();
-
-  // DeleteDisusedCreditCards should return false to indicate it was not run.
-  EXPECT_FALSE(personal_data_->DeleteDisusedCreditCards());
-
-  personal_data_->Refresh();
-
-  EXPECT_EQ(1U, personal_data_->GetCreditCards().size());
-}
-
 // Tests that DeleteDisusedCreditCards deletes desired credit cards only.
 TEST_F(PersonalDataManagerTest,
        DeleteDisusedCreditCards_OnlyDeleteExpiredDisusedLocalCards) {
-  // Enable the feature.
-  base::test::ScopedFeatureList scoped_features;
-  scoped_features.InitAndEnableFeature(
-      features::kAutofillDeleteDisusedCreditCards);
-
   const char kHistogramName[] = "Autofill.CreditCardsDeletedForDisuse";
   auto now = AutofillClock::Now();
 

components/autofill/core/common/autofill_features.cc

@@ -55,12 +55,6 @@ const base::Feature kAutofillCreateDataForTest{
 const base::Feature kAutofillCreditCardAssist{
     "AutofillCreditCardAssist", base::FEATURE_DISABLED_BY_DEFAULT};
 
-const base::Feature kAutofillDeleteDisusedAddresses{
-    "AutofillDeleteDisusedAddresses", base::FEATURE_ENABLED_BY_DEFAULT};
-
-const base::Feature kAutofillDeleteDisusedCreditCards{
-    "AutofillDeleteDisusedCreditCards", base::FEATURE_ENABLED_BY_DEFAULT};
-
 // Controls whether we download server credit cards to the ephemeral
 // account-based storage when sync the transport is enabled.
 const base::Feature kAutofillEnableAccountWalletStorage{
@@ -196,15 +190,9 @@ const base::Feature kAutofillShowTypePredictions{
 const base::Feature kAutofillSkipComparingInferredLabels{
     "AutofillSkipComparingInferredLabels", base::FEATURE_DISABLED_BY_DEFAULT};
 
-const base::Feature kAutofillSuppressDisusedAddresses{
-    "AutofillSuppressDisusedAddresses", base::FEATURE_ENABLED_BY_DEFAULT};
-
 const base::Feature kAutofillProfileClientValidation{
     "AutofillProfileClientValidation", base::FEATURE_DISABLED_BY_DEFAULT};
 
-const base::Feature kAutofillSuppressDisusedCreditCards{
-    "AutofillSuppressDisusedCreditCards", base::FEATURE_ENABLED_BY_DEFAULT};
-
 // Controls whether Autofill should search prefixes of all words/tokens when
 // filtering profiles, or only on prefixes of the whole string.
 const base::Feature kAutofillTokenPrefixMatching{

components/autofill/core/common/autofill_features.h

@@ -29,8 +29,6 @@ extern const base::Feature kAutofillAlwaysShowServerCardsInSyncTransport;
 extern const base::Feature kAutofillCacheQueryResponses;
 extern const base::Feature kAutofillCreateDataForTest;
 extern const base::Feature kAutofillCreditCardAssist;
-extern const base::Feature kAutofillDeleteDisusedAddresses;
-extern const base::Feature kAutofillDeleteDisusedCreditCards;
 extern const base::Feature kAutofillEnableAccountWalletStorage;
 extern const base::Feature kAutofillEnableAccountWalletStorageUpload;
 extern const base::Feature kAutofillEnableCompanyName;
@@ -57,8 +55,6 @@ extern const base::Feature kAutofillShowAutocompleteConsoleWarnings;
 extern const base::Feature kAutofillUseImprovedLabelDisambiguation;
 extern const base::Feature kAutofillShowTypePredictions;
 extern const base::Feature kAutofillSkipComparingInferredLabels;
-extern const base::Feature kAutofillSuppressDisusedAddresses;
-extern const base::Feature kAutofillSuppressDisusedCreditCards;
 extern const base::Feature kAutofillTokenPrefixMatching;
 extern const base::Feature kAutofillUploadThrottling;
 extern const base::Feature kAutofillUseApi;