[AF] Remove InvalidProfileData.UsedForMetrics feature

The feature "use invalid data" for possible type uploads should
always be on, meaning that the client will send up votes regardless
of the validity states. This is because we are uploading the validity
states of each possible type, and we can evaluate the relevance of
invalid data on the server side.

Also, the validity of UNKNOWN types should be set to UNVALIDATED for
the consistency.

Change-Id: Ia7234a128b04b975f6fb1bfd4d4dc131547530a8
Reviewed-on: https://chromium-review.googlesource.com/1249907Reviewed-by: Roger McFarlane <rogerm@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Parastoo Geranmayeh <parastoog@google.com>
Cr-Commit-Position: refs/heads/master@{#594851}

components/autofill/core/browser/autofill_manager.cc

@@ -1680,8 +1680,13 @@ void AutofillManager::DeterminePossibleFieldTypesForUpload(
       card.GetMatchingTypes(value, app_locale, &matching_types);
     }
 
-    if (matching_types.empty())
+    if (matching_types.empty()) {
       matching_types.insert(UNKNOWN_TYPE);
+      std::map<ServerFieldType, AutofillProfile::ValidityState>
+          matching_types_validities;
+      matching_types_validities[UNKNOWN_TYPE] = AutofillProfile::UNVALIDATED;
+      field->add_possible_types_validities(matching_types_validities);
+    }
 
     field->set_possible_types(matching_types);
   }

components/autofill/core/browser/autofill_manager_unittest.cc

@@ -4472,7 +4472,6 @@ class ProfileMatchingTypesTest
       public ::testing::WithParamInterface<
           std::tuple<ProfileMatchingTypesTestCase,
                      int,        // AutofillProfile::ValidityState
-                     bool,       // Enable AutofillVoteUsingInvalidProfileValues
                      bool>> {};  // AutofillProfile::ValidationSource
 
 const ProfileMatchingTypesTestCase kProfileMatchingTypesTestCases[] = {
@@ -4568,28 +4567,19 @@ TEST_P(ProfileMatchingTypesTest, DeterminePossibleFieldTypesForUpload) {
   const auto& test_case = std::get<0>(GetParam());
   auto validity_state =
       static_cast<AutofillProfile::ValidityState>(std::get<1>(GetParam()));
-  const auto& vote_using_invalid_profile_data = std::get<2>(GetParam());
   const auto& validation_source =
-      static_cast<AutofillProfile::ValidationSource>(std::get<3>(GetParam()));
+      static_cast<AutofillProfile::ValidationSource>(std::get<2>(GetParam()));
 
   SCOPED_TRACE(base::StringPrintf(
       "Test: input_value='%s', field_type=%s, validity_state=%d, "
-      "validation_source=%d, vote_using_invalid_profile_data=%d",
+      "validation_source=%d ",
       test_case.input_value,
       AutofillType(test_case.field_type).ToString().c_str(), validity_state,
-      validation_source, vote_using_invalid_profile_data));
+      validation_source));
 
   ASSERT_LE(AutofillProfile::UNVALIDATED, validity_state);
   ASSERT_LE(validity_state, AutofillProfile::UNSUPPORTED);
 
-  // Enable/Disable ignoring invalid profile data for the scope of this test.
-  base::test::ScopedFeatureList sfl;
-  if (vote_using_invalid_profile_data) {
-    sfl.InitAndEnableFeature(features::kAutofillVoteUsingInvalidProfileData);
-  } else {
-    sfl.InitAndDisableFeature(features::kAutofillVoteUsingInvalidProfileData);
-  }
-
   // Set up the test profiles.
   std::vector<AutofillProfile> profiles;
   profiles.resize(3);
@@ -4611,10 +4601,14 @@ TEST_P(ProfileMatchingTypesTest, DeterminePossibleFieldTypesForUpload) {
   profiles[2].set_guid("00000000-0000-0000-0000-000000000001");
 
   // Set the validity state for the matching field type.
-  auto field_type_group = AutofillType(test_case.field_type).group();
-  if (field_type_group != CREDIT_CARD) {
+  if (GroupTypeOfServerFieldType(test_case.field_type) != CREDIT_CARD) {
     for (auto& profile : profiles) {
-      if (profile.IsAnInvalidPhoneNumber(test_case.field_type)) {
+      if (test_case.field_type == UNKNOWN_TYPE) {
+        // An UNKNOWN type is always UNVALIDATED
+        validity_state = AutofillProfile::UNVALIDATED;
+      } else if (profile.IsAnInvalidPhoneNumber(test_case.field_type)) {
+        // a phone field is a compound field, an invalid part would make it
+        // invalid.
         validity_state = AutofillProfile::INVALID;
       }
       profile.SetValidityState(test_case.field_type, validity_state,
@@ -4647,45 +4641,16 @@ TEST_P(ProfileMatchingTypesTest, DeterminePossibleFieldTypesForUpload) {
       profiles, credit_cards, "en-us", &form_structure);
 
   ASSERT_EQ(1U, form_structure.field_count());
-  ServerFieldTypeSet possible_types = form_structure.field(0)->possible_types();
-  EXPECT_EQ(1U, possible_types.size());
 
-  if (test_case.field_type != EMPTY_TYPE &&
-      test_case.field_type != UNKNOWN_TYPE &&
-      ((validation_source == AutofillProfile::SERVER &&
-        GroupTypeOfServerFieldType(test_case.field_type) != CREDIT_CARD) ||
-       (validation_source == AutofillProfile::CLIENT &&
-        AutofillProfile::IsClientValidationSupportedForType(
-            test_case.field_type))) &&
-      validity_state == AutofillProfile::INVALID) {
-    // For the phone types we get more than one possible type match for one
-    // piece of data, therefore the count is not clear.
-    if (AutofillType(test_case.field_type).group() != PHONE_HOME) {
-      // There are two addresses in the US, every other type/value pair is
-      // unique.
-      int expected_count =
-          (test_case.field_type == ADDRESS_HOME_COUNTRY &&
-           base::StartsWith(test_case.input_value, "U",
-                            base::CompareCase::INSENSITIVE_ASCII))
-              ? 2
-              : 1;
-      histogram_tester.ExpectBucketCount(
-          "Autofill.InvalidProfileData.UsedForMetrics",
-          vote_using_invalid_profile_data, expected_count);
-    }
-    EXPECT_EQ(*possible_types.begin(), vote_using_invalid_profile_data
-                                           ? test_case.field_type
-                                           : UNKNOWN_TYPE);
+  ServerFieldTypeSet possible_types = form_structure.field(0)->possible_types();
+  ASSERT_EQ(1U, possible_types.size());
+  EXPECT_EQ(*possible_types.begin(), test_case.field_type);
 
+  // We don't add validity states for credit card fields.
+  if (GroupTypeOfServerFieldType(test_case.field_type) != CREDIT_CARD) {
     ServerFieldTypeValidityStatesMap possible_types_validities =
         form_structure.field(0)->possible_types_validities();
-    bool has_possible_types_validities =
-        field_type_group != CREDIT_CARD &&
-        (vote_using_invalid_profile_data ||
-         validity_state != AutofillProfile::INVALID);
-    EXPECT_EQ(has_possible_types_validities ? 1U : 0U,
-              possible_types_validities.size());
-    if (has_possible_types_validities) {
+    ASSERT_EQ(1U, possible_types_validities.size());
     EXPECT_NE(possible_types_validities.end(),
               possible_types_validities.find(test_case.field_type));
     EXPECT_EQ(possible_types_validities[test_case.field_type][0],
@@ -4693,9 +4658,6 @@ TEST_P(ProfileMatchingTypesTest, DeterminePossibleFieldTypesForUpload) {
                   ? validity_state
                   : AutofillProfile::UNVALIDATED);
   }
-  } else {
-    EXPECT_EQ(*possible_types.begin(), test_case.field_type);
-  }
 }
 
 INSTANTIATE_TEST_CASE_P(
@@ -4705,7 +4667,6 @@ INSTANTIATE_TEST_CASE_P(
         testing::ValuesIn(kProfileMatchingTypesTestCases),
         testing::Range(static_cast<int>(AutofillProfile::UNVALIDATED),
                        static_cast<int>(AutofillProfile::UNSUPPORTED) + 1),
-        testing::Bool(),
         testing::Bool()));
 
 // Tests that DeterminePossibleFieldTypesForUpload is called when a form is
@@ -4793,7 +4754,7 @@ TEST_F(AutofillManagerTest, DeterminePossibleFieldTypesWithMultipleValidities) {
     std::vector<AutofillProfile::ValidityState> expected_validity_states;
   } TestFieldData;
 
-  std::vector<TestFieldData> test_cases[2];
+  std::vector<TestFieldData> test_cases[3];
   // Tennessee appears in both of the user's profile as ADDRESS_HOME_STATE. In
   // the first one, it's VALID, and for the other, it's INVALID. Therefore, the
   // possible_field_types would only include the type ADDRESS_HOME_STATE, and
@@ -4806,6 +4767,9 @@ TEST_F(AutofillManagerTest, DeterminePossibleFieldTypesWithMultipleValidities) {
   // UNVALIDATED.
   test_cases[1].push_back(
       {"Alice", NAME_FIRST, {AutofillProfile::UNVALIDATED}});
+  // An UNKNOWN type is always UNVALIDATED.
+  test_cases[2].push_back(
+      {"What a beautiful day!", UNKNOWN_TYPE, {AutofillProfile::UNVALIDATED}});
 
   for (const std::vector<TestFieldData>& test_fields : test_cases) {
     FormData form;

components/autofill/core/browser/autofill_profile.cc

@@ -318,16 +318,6 @@ void AutofillProfile::GetMatchingTypes(
   }
 
   for (auto type : matching_types_in_this_profile) {
-    if (GetValidityState(type, CLIENT) == INVALID ||
-        GetValidityState(type, SERVER) == INVALID ||
-        IsAnInvalidPhoneNumber(type)) {
-      bool vote_using_invalid_data = base::FeatureList::IsEnabled(
-          features::kAutofillVoteUsingInvalidProfileData);
-      UMA_HISTOGRAM_BOOLEAN("Autofill.InvalidProfileData.UsedForMetrics",
-                            vote_using_invalid_data);
-      if (!vote_using_invalid_data)
-        continue;
-    }
     matching_types->insert(type);
   }
 }
@@ -349,16 +339,6 @@ void AutofillProfile::GetMatchingTypesAndValidities(
   }
 
   for (auto type : matching_types_in_this_profile) {
-    if (GetValidityState(type, CLIENT) == INVALID ||
-        GetValidityState(type, SERVER) == INVALID ||
-        IsAnInvalidPhoneNumber(type)) {
-      bool vote_using_invalid_data = base::FeatureList::IsEnabled(
-          features::kAutofillVoteUsingInvalidProfileData);
-      UMA_HISTOGRAM_BOOLEAN("Autofill.InvalidProfileData.UsedForMetrics",
-                            vote_using_invalid_data);
-      if (!vote_using_invalid_data)
-        continue;
-    }
     if (matching_types_validities) {
       // TODO(crbug.com/879655): Set the client validities and look them up when
       // the server validities are not available.

components/autofill/core/common/autofill_features.cc

@@ -264,9 +264,6 @@ const base::Feature kAutofillUpstreamUseGooglePayBrandingOnMobile{
 const base::Feature kAutofillUsePaymentsCustomerData{
     "AutofillUsePaymentsCustomerData", base::FEATURE_DISABLED_BY_DEFAULT};
 
-const base::Feature kAutofillVoteUsingInvalidProfileData{
-    "AutofillVoteUsingInvalidProfileData", base::FEATURE_ENABLED_BY_DEFAULT};
-
 // Controls whether password generation is offered automatically on fields
 // perceived as eligible for generation.
 #if defined(OS_ANDROID)

components/autofill/core/common/autofill_features.h

@@ -74,7 +74,6 @@ extern const base::Feature kAutofillUpstreamEditableCardholderName;
 extern const base::Feature kAutofillUpstreamUpdatePromptExplanation;
 extern const base::Feature kAutofillUpstreamUseGooglePayBrandingOnMobile;
 extern const base::Feature kAutofillUsePaymentsCustomerData;
-extern const base::Feature kAutofillVoteUsingInvalidProfileData;
 extern const base::Feature kAutomaticPasswordGeneration;
 extern const base::Feature kSingleClickAutofill;
 extern const base::Feature kAutofillCreditCardLocalCardMigration;