[autofill] Check for and delete disused data on every startup.

This CL removes the last-version-... pref check/update that causes
deletion of disused data to be run at most once per major version.
Instead, it will be attempted at each startup.

Bug: 881443
Change-Id: Ie6fbae3a9c29d67af28d842f02014a1274d10540
Reviewed-on: https://chromium-review.googlesource.com/1211086
Commit-Queue: Roger McFarlane <rogerm@chromium.org>
Commit-Queue: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594781}

components/autofill/core/browser/personal_data_manager.cc

@@ -2605,22 +2605,6 @@ bool PersonalDataManager::DeleteDisusedCreditCards() {
     return false;
   }
 
-  // Check if credit cards deletion has already been performed this major
-  // version.
-  int current_major_version = atoi(version_info::GetVersionNumber().c_str());
-  if (pref_service_->GetInteger(
-          prefs::kAutofillLastVersionDisusedCreditCardsDeleted) >=
-      current_major_version) {
-    DVLOG(1)
-        << "Autofill credit cards deletion already performed for this version";
-    return false;
-  }
-
-  // Set the pref to the current major version.
-  pref_service_->SetInteger(
-      prefs::kAutofillLastVersionDisusedCreditCardsDeleted,
-      current_major_version);
-
   // Only delete local cards, as server cards are managed by Payments.
   auto cards = GetLocalCreditCards();
 
@@ -2669,20 +2653,6 @@ bool PersonalDataManager::DeleteDisusedAddresses() {
     return false;
   }
 
-  // Check if address deletion has already been performed this major version.
-  int current_major_version = atoi(version_info::GetVersionNumber().c_str());
-  if (pref_service_->GetInteger(
-          prefs::kAutofillLastVersionDisusedAddressesDeleted) >=
-      current_major_version) {
-    DVLOG(1)
-        << "Autofill addresses deletion already performed for this version";
-    return false;
-  }
-
-  // Set the pref to the current major version.
-  pref_service_->SetInteger(prefs::kAutofillLastVersionDisusedAddressesDeleted,
-                            current_major_version);
-
   const std::vector<AutofillProfile*>& profiles = GetProfiles();
 
   // Early exit when there are no profiles.
@@ -2723,14 +2693,14 @@ bool PersonalDataManager::DeleteDisusedAddresses() {
 void PersonalDataManager::ApplyAddressFixesAndCleanups() {
   RemoveOrphanAutofillTableRows();   // One-time fix, otherwise NOP.
   ApplyDedupingRoutine();            // Once per major version, otherwise NOP.
-  DeleteDisusedAddresses();          // Once per major version, otherwise NOP.
+  DeleteDisusedAddresses();
   MaybeCreateTestAddresses();        // Once per user profile startup.
   ClearProfileNonSettingsOrigins();  // Ran everytime it is called.
   MoveJapanCityToStreetAddress();    // One-time fix, otherwise NOP.
 }
 
 void PersonalDataManager::ApplyCardFixesAndCleanups() {
-  DeleteDisusedCreditCards();    // Once per major version, otherwise NOP.
+  DeleteDisusedCreditCards();
   MaybeCreateTestCreditCards();  // Once per user profile startup.
   ClearCreditCardNonSettingsOrigins();  // Ran everytime it is called.
 }

components/autofill/core/browser/personal_data_manager_unittest.cc

@@ -4714,32 +4714,6 @@ TEST_F(PersonalDataManagerTest, DeleteDisusedAddresses_DoNothingWhenDisabled) {
   EXPECT_EQ(1U, personal_data_->GetProfiles().size());
 }
 
-// Tests that DeleteDisusedAddresses is not run a second time on the same
-// major version.
-TEST_F(PersonalDataManagerTest, DeleteDisusedAddresses_OncePerVersion) {
-  // Enable the feature.
-  base::test::ScopedFeatureList scoped_features;
-  scoped_features.InitAndEnableFeature(
-      features::kAutofillDeleteDisusedAddresses);
-
-  CreateDeletableDisusedProfile();
-
-  EXPECT_TRUE(personal_data_->DeleteDisusedAddresses());
-  WaitForOnPersonalDataChanged();
-
-  EXPECT_EQ(0U, personal_data_->GetProfiles().size());
-
-  // Add the profile back.
-  CreateDeletableDisusedProfile();
-
-  // DeleteDisusedAddresses 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,
@@ -4844,33 +4818,6 @@ TEST_F(PersonalDataManagerTest,
   EXPECT_EQ(1U, personal_data_->GetCreditCards().size());
 }
 
-// Tests that DeleteDisusedCreditCards is not run a second time on the same
-// major version.
-TEST_F(PersonalDataManagerTest, DeleteDisusedCreditCards_OncePerVersion) {
-  // Enable the feature.
-  base::test::ScopedFeatureList scoped_features;
-  scoped_features.InitAndEnableFeature(
-      features::kAutofillDeleteDisusedCreditCards);
-
-  CreateDeletableExpiredAndDisusedCreditCard();
-
-  // The deletion should be run a first time.
-  EXPECT_TRUE(personal_data_->DeleteDisusedCreditCards());
-  WaitForOnPersonalDataChanged();
-
-  // The profiles should have been deleted.
-  EXPECT_EQ(0U, personal_data_->GetCreditCards().size());
-
-  // Add the card back.
-  CreateDeletableExpiredAndDisusedCreditCard();
-
-  // The cleanup should not be run.
-  EXPECT_FALSE(personal_data_->DeleteDisusedCreditCards());
-
-  // The card should still be present.
-  EXPECT_EQ(1U, personal_data_->GetCreditCards().size());
-}
-
 // Tests that DeleteDisusedCreditCards deletes desired credit cards only.
 TEST_F(PersonalDataManagerTest,
        DeleteDisusedCreditCards_OnlyDeleteExpiredDisusedLocalCards) {