[autofill] Suppress the use of invalid profile data.

This CL adds feature flags that causes Chrome to suppress suggestions,
votes and quality metrics based on profile data which has been flagged
as likely invalid.

This will allow Chrome Team to evaluate the utility of attempting to
filter out this data.

Bug: 835984
Change-Id: I63f51ac893d2bf657af9eb136d920cb0b7f2bddd
Reviewed-on: https://chromium-review.googlesource.com/998132
Commit-Queue: Roger McFarlane <rogerm@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556900}

components/autofill/core/browser/autofill_experiments.cc

@@ -55,6 +55,8 @@ const base::Feature kAutofillPreferServerNamePredictions{
 const base::Feature kAutofillRationalizeFieldTypePredictions{
     "AutofillRationalizeFieldTypePredictions",
     base::FEATURE_ENABLED_BY_DEFAULT};
+const base::Feature kAutofillSuggestInvalidProfileData{
+    "AutofillSuggestInvalidProfileData", base::FEATURE_ENABLED_BY_DEFAULT};
 const base::Feature kAutofillSuppressDisusedAddresses{
     "AutofillSuppressDisusedAddresses", base::FEATURE_ENABLED_BY_DEFAULT};
 const base::Feature kAutofillSuppressDisusedCreditCards{
@@ -68,6 +70,8 @@ const base::Feature kAutofillUpstreamSendPanFirstSix{
 const base::Feature kAutofillUpstreamUpdatePromptExplanation{
     "AutofillUpstreamUpdatePromptExplanation",
     base::FEATURE_DISABLED_BY_DEFAULT};
+const base::Feature kAutofillVoteUsingInvalidProfileData{
+    "AutofillVoteUsingInvalidProfileData", base::FEATURE_ENABLED_BY_DEFAULT};
 
 const char kCreditCardSigninPromoImpressionLimitParamKey[] = "impression_limit";
 const char kAutofillCreditCardPopupBackgroundColorKey[] = "background_color";

components/autofill/core/browser/autofill_experiments.h

@@ -39,12 +39,14 @@ extern const base::Feature kAutofillDeleteDisusedCreditCards;
 extern const base::Feature kAutofillExpandedPopupViews;
 extern const base::Feature kAutofillPreferServerNamePredictions;
 extern const base::Feature kAutofillRationalizeFieldTypePredictions;
+extern const base::Feature kAutofillSuggestInvalidProfileData;
 extern const base::Feature kAutofillSuppressDisusedAddresses;
 extern const base::Feature kAutofillSuppressDisusedCreditCards;
 extern const base::Feature kAutofillUpstreamAllowAllEmailDomains;
 extern const base::Feature kAutofillUpstreamSendDetectedValues;
 extern const base::Feature kAutofillUpstreamSendPanFirstSix;
 extern const base::Feature kAutofillUpstreamUpdatePromptExplanation;
+extern const base::Feature kAutofillVoteUsingInvalidProfileData;
 extern const char kCreditCardSigninPromoImpressionLimitParamKey[];
 extern const char kAutofillCreditCardLastUsedDateShowExpirationDateKey[];
 extern const char kAutofillUpstreamMaxMinutesSinceAutofillProfileUseKey[];

components/autofill/core/browser/autofill_manager.h

@@ -604,7 +604,10 @@ class AutofillManager : public AutofillHandler,
 
   friend class AutofillManagerTest;
   friend class FormStructureBrowserTest;
+  friend class GetMatchingTypesTest;
   friend class SaveCardBubbleViewsBrowserTestBase;
+  FRIEND_TEST_ALL_PREFIXES(ProfileMatchingTypesTest,
+                           DeterminePossibleFieldTypesForUpload);
   FRIEND_TEST_ALL_PREFIXES(AutofillManagerTest,
                            DeterminePossibleFieldTypesForUpload);
   FRIEND_TEST_ALL_PREFIXES(AutofillManagerTest,

components/autofill/core/browser/autofill_profile.cc

@@ -25,6 +25,7 @@
 #include "components/autofill/core/browser/address.h"
 #include "components/autofill/core/browser/address_i18n.h"
 #include "components/autofill/core/browser/autofill_country.h"
+#include "components/autofill/core/browser/autofill_experiments.h"
 #include "components/autofill/core/browser/autofill_field.h"
 #include "components/autofill/core/browser/autofill_metrics.h"
 #include "components/autofill/core/browser/autofill_profile_comparator.h"
@@ -192,10 +193,10 @@ void GetFieldsForDistinguishingProfiles(
 }
 
 // Constants for the validity bitfield.
-static const size_t kValidityBitsPerType = 2;
+const size_t kValidityBitsPerType = 2;
 // The order is important to ensure a consistent bitfield value. New values
 // should be added at the end NOT at the start or middle.
-static const ServerFieldType kSupportedTypesForValidation[] = {
+const ServerFieldType kSupportedTypesForValidation[] = {
     ADDRESS_HOME_COUNTRY,
     ADDRESS_HOME_STATE,
     ADDRESS_HOME_ZIP,
@@ -204,13 +205,22 @@ static const ServerFieldType kSupportedTypesForValidation[] = {
     EMAIL_ADDRESS,
     PHONE_HOME_WHOLE_NUMBER};
 
-static const size_t kNumSupportedTypesForValidation =
+const size_t kNumSupportedTypesForValidation =
     sizeof(kSupportedTypesForValidation) /
     sizeof(kSupportedTypesForValidation[0]);
 
 static_assert(kNumSupportedTypesForValidation * kValidityBitsPerType <= 64,
               "Not enough bits to encode profile validity information!");
 
+// Some types are specializations of other types. Normalize these back to the
+// main stored type for used to mark field validity .
+ServerFieldType NormalizeTypeForValidityCheck(ServerFieldType type) {
+  auto field_type_group = AutofillType(type).group();
+  if (field_type_group == PHONE_HOME || field_type_group == PHONE_BILLING)
+    return PHONE_HOME_WHOLE_NUMBER;
+  return type;
+}
+
 }  // namespace
 
 AutofillProfile::AutofillProfile(const std::string& guid,
@@ -278,9 +288,22 @@ void AutofillProfile::GetMatchingTypes(
     const base::string16& text,
     const std::string& app_locale,
     ServerFieldTypeSet* matching_types) const {
+  ServerFieldTypeSet matching_types_in_this_profile;
   FormGroupList info = FormGroups();
   for (const auto* form_group : info) {
-    form_group->GetMatchingTypes(text, app_locale, matching_types);
+    form_group->GetMatchingTypes(text, app_locale,
+                                 &matching_types_in_this_profile);
+  }
+  for (auto type : matching_types_in_this_profile) {
+    if (GetValidityState(type) == INVALID) {
+      bool vote_using_invalid_data =
+          base::FeatureList::IsEnabled(kAutofillVoteUsingInvalidProfileData);
+      UMA_HISTOGRAM_BOOLEAN("Autofill.InvalidProfileData.UsedForMetrics",
+                            vote_using_invalid_data);
+      if (!vote_using_invalid_data)
+        continue;
+    }
+    matching_types->insert(type);
   }
 }
 
@@ -707,6 +730,7 @@ void AutofillProfile::RecordAndLogUse() {
 
 AutofillProfile::ValidityState AutofillProfile::GetValidityState(
     ServerFieldType type) const {
+  type = NormalizeTypeForValidityCheck(type);
   // Return UNSUPPORTED for types that autofill does not validate.
   if (!IsValidationSupportedForType(type))
     return UNSUPPORTED;
@@ -724,7 +748,8 @@ void AutofillProfile::SetValidityState(ServerFieldType type,
   validity_states_[type] = validity;
 }
 
-bool AutofillProfile::IsValidationSupportedForType(ServerFieldType type) const {
+// static
+bool AutofillProfile::IsValidationSupportedForType(ServerFieldType type) {
   for (auto supported_type : kSupportedTypesForValidation) {
     if (type == supported_type)
       return true;

components/autofill/core/browser/autofill_profile.h

@@ -203,7 +203,7 @@ class AutofillProfile : public AutofillDataModel,
   void SetValidityState(ServerFieldType type, ValidityState validity);
 
   // Returns whether autofill does the validation of the specified |type|.
-  bool IsValidationSupportedForType(ServerFieldType type) const;
+  static bool IsValidationSupportedForType(ServerFieldType type);
 
   // Returns the bitfield value representing the validity state of this profile.
   int GetValidityBitfieldValue() const;

components/autofill/core/browser/autofill_profile_unittest.cc

@@ -1127,7 +1127,7 @@ TEST(AutofillProfileTest, ValidityStates_UnsupportedTypes) {
             profile.GetValidityState(ADDRESS_HOME_LINE1));
   EXPECT_EQ(AutofillProfile::UNSUPPORTED,
             profile.GetValidityState(ADDRESS_HOME_LINE2));
-  EXPECT_EQ(AutofillProfile::UNSUPPORTED,
+  EXPECT_EQ(AutofillProfile::UNVALIDATED,
             profile.GetValidityState(PHONE_HOME_CITY_AND_NUMBER));
 }
 

components/autofill/core/browser/personal_data_manager.cc

@@ -16,6 +16,7 @@
 #include "base/feature_list.h"
 #include "base/i18n/case_conversion.h"
 #include "base/i18n/timezone.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
@@ -975,6 +976,32 @@ void PersonalDataManager::RemoveProfilesNotUsedSinceTimestamp(
       num_profiles_supressed);
 }
 
+// static
+void PersonalDataManager::MaybeRemoveInvalidSuggestions(
+    const AutofillType& type,
+    std::vector<AutofillProfile*>* profiles) {
+  const bool suggest_invalid =
+      base::FeatureList::IsEnabled(kAutofillSuggestInvalidProfileData);
+
+  for (size_t i = 0; i < profiles->size(); ++i) {
+    bool is_invalid = (*profiles)[i]->GetValidityState(
+                          type.GetStorableType()) == AutofillProfile::INVALID;
+    if (is_invalid) {
+      UMA_HISTOGRAM_BOOLEAN("Autofill.InvalidProfileData.UsedForSuggestion",
+                            suggest_invalid);
+      if (!suggest_invalid)
+        (*profiles)[i] = nullptr;
+    }
+  }
+
+  if (!suggest_invalid) {
+    profiles->erase(
+        std::stable_partition(profiles->begin(), profiles->end(),
+                              [](AutofillProfile* p) { return p != nullptr; }),
+        profiles->end());
+  }
+}
+
 std::vector<Suggestion> PersonalDataManager::GetProfileSuggestions(
     const AutofillType& type,
     const base::string16& field_contents,
@@ -990,13 +1017,15 @@ std::vector<Suggestion> PersonalDataManager::GetProfileSuggestions(
   // Get the profiles to suggest, which are already sorted.
   std::vector<AutofillProfile*> profiles = GetProfilesToSuggest();
 
-  // If enabled, suppress disused address profiles when triggered from an empty
-  // field.
-  if (field_contents_canon.empty() &&
-      base::FeatureList::IsEnabled(kAutofillSuppressDisusedAddresses)) {
-    const base::Time min_last_used =
-        AutofillClock::Now() - kDisusedProfileTimeDelta;
-    RemoveProfilesNotUsedSinceTimestamp(min_last_used, &profiles);
+  // When suggesting with no prefix to match, consider suppressing disused
+  // address suggestions as well as those based on invalid profile data.
+  if (field_contents_canon.empty()) {
+    if (base::FeatureList::IsEnabled(kAutofillSuppressDisusedAddresses)) {
+      const base::Time min_last_used =
+          AutofillClock::Now() - kDisusedProfileTimeDelta;
+      RemoveProfilesNotUsedSinceTimestamp(min_last_used, &profiles);
+    }
+    MaybeRemoveInvalidSuggestions(type, &profiles);
   }
 
   std::vector<Suggestion> suggestions;

components/autofill/core/browser/personal_data_manager.h

@@ -215,6 +215,12 @@ class PersonalDataManager : public KeyedService,
       base::Time min_last_used,
       std::vector<AutofillProfile*>* profiles);
 
+  // Remove profiles that whose |type| field is flagged as invalid, if Chrome
+  // is configured to not make suggestions based on invalid data.
+  static void MaybeRemoveInvalidSuggestions(
+      const AutofillType& type,
+      std::vector<AutofillProfile*>* profiles);
+
   // Loads profiles that can suggest data for |type|. |field_contents| is the
   // part the user has already typed. |field_is_autofilled| is true if the field
   // has already been autofilled. |other_field_types| represents the rest of

components/autofill/core/browser/personal_data_manager_unittest.cc

@@ -1925,6 +1925,54 @@ TEST_F(PersonalDataManagerTest,
   }
 }
 
+// Tests that suggestions based on invalid data are handled correctly.
+TEST_F(PersonalDataManagerTest, GetProfileSuggestions_InvalidData) {
+  // Set up 2 different profiles.
+  AutofillProfile profile1(base::GenerateGUID(), "https://www.example.com");
+  test::SetProfileInfo(&profile1, "Marion1", "Mitchell", "Morrison",
+                       "johnwayne@me.xyz", "Fox",
+                       "123 Zoo St.\nSecond Line\nThird line", "unit 5",
+                       "Hollywood", "CA", "91601", "US", "9876543210");
+  profile1.SetValidityState(PHONE_HOME_WHOLE_NUMBER, AutofillProfile::INVALID);
+  personal_data_->AddProfile(profile1);
+
+  AutofillProfile profile2(base::GenerateGUID(), "https://www.example.com");
+  test::SetProfileInfo(&profile2, "Marion2", "Mitchell", "Morrison",
+                       "johnwayne@me.xyz", "Fox",
+                       "456 Zoo St.\nSecond Line\nThird line", "unit 5",
+                       "Hollywood", "CA", "91601", "US", "1234567890");
+  profile1.set_use_date(AutofillClock::Now() - base::TimeDelta::FromDays(20));
+  personal_data_->AddProfile(profile2);
+
+  ResetPersonalDataManager(USER_MODE_NORMAL);
+  {
+    base::HistogramTester histogram_tester;
+    base::test::ScopedFeatureList scoped_features;
+    scoped_features.InitAndDisableFeature(kAutofillSuggestInvalidProfileData);
+    std::vector<Suggestion> suggestions = personal_data_->GetProfileSuggestions(
+        AutofillType(PHONE_HOME_WHOLE_NUMBER), base::string16(), false,
+        std::vector<ServerFieldType>());
+    ASSERT_EQ(1U, suggestions.size());
+    EXPECT_EQ(base::ASCIIToUTF16("1234567890"), suggestions[0].value);
+    histogram_tester.ExpectUniqueSample(
+        "Autofill.InvalidProfileData.UsedForSuggestion", false, 1);
+  }
+
+  {
+    base::HistogramTester histogram_tester;
+    base::test::ScopedFeatureList scoped_features;
+    scoped_features.InitAndEnableFeature(kAutofillSuggestInvalidProfileData);
+    std::vector<Suggestion> suggestions = personal_data_->GetProfileSuggestions(
+        AutofillType(PHONE_HOME_WHOLE_NUMBER), base::string16(), false,
+        std::vector<ServerFieldType>());
+    ASSERT_EQ(2U, suggestions.size());
+    EXPECT_EQ(base::ASCIIToUTF16("1234567890"), suggestions[0].value);
+    EXPECT_EQ(base::ASCIIToUTF16("9876543210"), suggestions[1].value);
+    histogram_tester.ExpectUniqueSample(
+        "Autofill.InvalidProfileData.UsedForSuggestion", true, 1);
+  }
+}
+
 TEST_F(PersonalDataManagerTest, IsKnownCard_MatchesMaskedServerCard) {
   // Add a masked server card.
   std::vector<CreditCard> server_cards;

tools/metrics/histograms/histograms.xml

@@ -6078,6 +6078,26 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="Autofill.InvalidProfileData.UsedForMetrics" enum="Boolean">
+  <owner>rogerm@chromium.org</owner>
+  <summary>
+    Tracks whether or not autofill suppressed sending votes or calculating
+    quality metrics because the profile data was marked as invalid. Logged
+    during field-type validation if/when a field marked as invalid is found to
+    match the submitted data.
+  </summary>
+</histogram>
+
+<histogram name="Autofill.InvalidProfileData.UsedForSuggestion" enum="Boolean">
+  <owner>rogerm@chromium.org</owner>
+  <summary>
+    Tracks whether or not autofill suppressed offering an autofill suggestion
+    because the profile data was marked as invalid. Logged during autofill
+    suggestion generation when a suggestion is about to generated based on a
+    field marked as invalid.
+  </summary>
+</histogram>
+
 <histogram name="Autofill.IsEnabled.PageLoad" enum="BooleanEnabled">
   <owner>isherman@chromium.org</owner>
   <summary>