[Autofill][States] Determine State Votes on UI thread

AlternativeStateNameMap can only be accessed on the UI thread. In the
current implementation, the logic to determine state votes using
AlternativeStateNameMap does not run on the UI thread resulting in the
crash.
With this CL, the state votes are preprocessed before
|DeterminePossibleFieldTypesForUpload| is called.

Bug: 1111960
Change-Id: I470b7db684916c4c7fef35d2018af876992c0c27
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2617792
Commit-Queue: Vidhan Jain <vidhanj@google.com>
Reviewed-by: Matthias Körber <koerber@google.com>
Cr-Commit-Position: refs/heads/master@{#843477}

components/autofill/core/browser/autofill_field.h

@@ -186,6 +186,14 @@ class AutofillField : public FormFieldData {
     return password_requirements_;
   }
 
+  // Getter and Setter methods for |state_is_a_matching_type_|.
+  void set_state_is_a_matching_type(bool value = true) {
+    state_is_a_matching_type_ = value;
+  }
+  const bool& state_is_a_matching_type() const {
+    return state_is_a_matching_type_;
+  }
+
   // For each type in |possible_types_| that's missing from
   // |possible_types_validities_|, will add it to the
   // |possible_types_validities_| and will set its validity to UNVALIDATED. This
@@ -279,6 +287,9 @@ class AutofillField : public FormFieldData {
   AutofillUploadContents::Field::VoteType vote_type_ =
       AutofillUploadContents::Field::NO_INFORMATION;
 
+  // Denotes if |ADDRESS_HOME_STATE| should be added to |possible_types_|.
+  bool state_is_a_matching_type_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(AutofillField);
 };
 

components/autofill/core/browser/autofill_manager.cc

@@ -807,6 +807,11 @@ bool AutofillManager::MaybeStartVoteUploadProcess(
   form_structure->set_randomized_encoder(
       RandomizedEncoder::Create(client_->GetPrefs()));
 
+  // Determine |ADDRESS_HOME_STATE| as a possible types for the fields in the
+  // |form_structure| with the help of |AlternativeStateNameMap|.
+  // |AlternativeStateNameMap| can only be accessed on the main UI thread.
+  PreProcessStateMatchingTypes(copied_profiles, form_structure.get());
+
   // Note that ownership of |form_structure| is passed to the second task,
   // using |base::Owned|. We MUST temporarily hang on to the raw form pointer
   // so that we can safely pass the address to the first callback regardless of
@@ -2119,6 +2124,9 @@ void AutofillManager::DeterminePossibleFieldTypesForUpload(
     if (IsUPIVirtualPaymentAddress(value))
       matching_types.insert(UPI_VPA);
 
+    if (field->state_is_a_matching_type())
+      matching_types.insert(ADDRESS_HOME_STATE);
+
     if (matching_types.empty()) {
       matching_types.insert(UNKNOWN_TYPE);
       ServerFieldTypeValidityStateMap matching_types_validities;
@@ -2673,4 +2681,42 @@ LanguageCode AutofillManager::GetPageLanguage() const {
   return LanguageCode(language_state->current_language());
 }
 
+void AutofillManager::PreProcessStateMatchingTypes(
+    const std::vector<AutofillProfile>& profiles,
+    FormStructure* form_structure) {
+  if (!base::FeatureList::IsEnabled(
+          features::kAutofillUseAlternativeStateNameMap)) {
+    return;
+  }
+
+  for (const auto& profile : profiles) {
+    base::Optional<AlternativeStateNameMap::CanonicalStateName>
+        canonical_state_name_from_profile =
+            profile.GetAddress().GetCanonicalizedStateName();
+
+    if (!canonical_state_name_from_profile)
+      continue;
+
+    const AutofillType kCountryCode(HTML_TYPE_COUNTRY_CODE, HTML_MODE_NONE);
+    const base::string16& country_code =
+        profile.GetInfo(kCountryCode, app_locale_);
+
+    for (auto& field : *form_structure) {
+      if (field->state_is_a_matching_type())
+        continue;
+
+      base::Optional<AlternativeStateNameMap::CanonicalStateName>
+          canonical_state_name_from_text =
+              AlternativeStateNameMap::GetCanonicalStateName(
+                  base::UTF16ToUTF8(country_code), field->value);
+
+      if (canonical_state_name_from_text &&
+          canonical_state_name_from_text.value() ==
+              canonical_state_name_from_profile.value()) {
+        field->set_state_is_a_matching_type();
+      }
+    }
+  }
+}
+
 }  // namespace autofill

components/autofill/core/browser/autofill_manager.h

@@ -288,6 +288,14 @@ class AutofillManager : public AutofillHandler,
 
   // A public wrapper that calls |TriggerRefill| for testing purposes only.
   void TriggerRefillForTest(const FormData& form) { TriggerRefill(form); }
+
+  // A public wrapper that calls |PreProcessStateMatchingTypes| for testing
+  // purposes.
+  void PreProcessStateMatchingTypesForTest(
+      const std::vector<AutofillProfile>& profiles,
+      FormStructure* form_structure) {
+    PreProcessStateMatchingTypes(profiles, form_structure);
+  }
 #endif
 
  protected:
@@ -591,6 +599,14 @@ class AutofillManager : public AutofillHandler,
   // Retrieves the page language from |client_|
   LanguageCode GetPageLanguage() const override;
 
+  // For each submitted field in the |form_structure|, it determines whether
+  // |ADDRESS_HOME_STATE| is a possible matching type.
+  // This method is intended to run matching type detection on the browser UI
+  // thread.
+  void PreProcessStateMatchingTypes(
+      const std::vector<AutofillProfile>& profiles,
+      FormStructure* form_structure);
+
 #if !defined(OS_ANDROID) && !defined(OS_IOS)
   // Whether to show the option to use virtual card in the autofill popup.
   bool ShouldShowVirtualCardOption(FormStructure* form_structure);

components/autofill/core/browser/autofill_manager_unittest.cc

@@ -36,6 +36,7 @@
 #include "components/autofill/core/browser/autofill_test_utils.h"
 #include "components/autofill/core/browser/data_model/autofill_profile.h"
 #include "components/autofill/core/browser/data_model/credit_card.h"
+#include "components/autofill/core/browser/geo/alternative_state_name_map_test_utils.h"
 #include "components/autofill/core/browser/metrics/form_events.h"
 #include "components/autofill/core/browser/mock_autocomplete_history_manager.h"
 #include "components/autofill/core/browser/payments/test_credit_card_save_manager.h"
@@ -2161,6 +2162,86 @@ TEST_F(AutofillManagerTest,
                                      1);
 }
 
+// Test that we properly match typed values to stored state data.
+TEST_F(AutofillManagerTest, DetermineStateFieldTypeForUpload) {
+  base::test::ScopedFeatureList feature;
+  feature.InitAndEnableFeature(features::kAutofillUseAlternativeStateNameMap);
+
+  test::ClearAlternativeStateNameMapForTesting();
+  test::PopulateAlternativeStateNameMapForTesting();
+
+  AutofillProfile profile;
+  test::SetProfileInfo(&profile, "", "", "", "", "", "", "", "", "Bavaria", "",
+                       "DE", "");
+
+  const char* const kValidMatches[] = {"by", "Bavaria", "Bayern",
+                                       "BY", "B.Y",     "B-Y"};
+  for (const char* valid_match : kValidMatches) {
+    SCOPED_TRACE(valid_match);
+    FormData form;
+    FormFieldData field;
+
+    test::CreateTestFormField("Name", "Name", "Test", "text", &field);
+    form.fields.push_back(field);
+
+    test::CreateTestFormField("State", "state", valid_match, "text", &field);
+    form.fields.push_back(field);
+
+    FormStructure form_structure(form);
+    EXPECT_EQ(form_structure.field_count(), 2U);
+
+    autofill_manager_->PreProcessStateMatchingTypesForTest({profile},
+                                                           &form_structure);
+    EXPECT_TRUE(form_structure.field(1)->state_is_a_matching_type());
+  }
+
+  const char* const kInvalidMatches[] = {"Garbage", "BYA",   "BYA is a state",
+                                         "Bava",    "Empty", ""};
+  for (const char* invalid_match : kInvalidMatches) {
+    SCOPED_TRACE(invalid_match);
+    FormData form;
+    FormFieldData field;
+
+    test::CreateTestFormField("Name", "Name", "Test", "text", &field);
+    form.fields.push_back(field);
+
+    test::CreateTestFormField("State", "state", invalid_match, "text", &field);
+    form.fields.push_back(field);
+
+    FormStructure form_structure(form);
+    EXPECT_EQ(form_structure.field_count(), 2U);
+
+    autofill_manager_->PreProcessStateMatchingTypesForTest({profile},
+                                                           &form_structure);
+    EXPECT_FALSE(form_structure.field(1)->state_is_a_matching_type());
+  }
+
+  test::PopulateAlternativeStateNameMapForTesting(
+      "US", "California",
+      {{.canonical_name = "California",
+        .abbreviations = {"CA"},
+        .alternative_names = {}}});
+
+  test::SetProfileInfo(&profile, "", "", "", "", "", "", "", "", "California",
+                       "", "US", "");
+
+  FormData form;
+  FormFieldData field;
+
+  test::CreateTestFormField("Name", "Name", "Test", "text", &field);
+  form.fields.push_back(field);
+
+  test::CreateTestFormField("State", "state", "CA", "text", &field);
+  form.fields.push_back(field);
+
+  FormStructure form_structure(form);
+  EXPECT_EQ(form_structure.field_count(), 2U);
+
+  autofill_manager_->PreProcessStateMatchingTypesForTest({profile},
+                                                         &form_structure);
+  EXPECT_TRUE(form_structure.field(1)->state_is_a_matching_type());
+}
+
 // Test that we return normal Autofill suggestions when trying to autofill
 // already filled forms.
 TEST_P(SuggestionMatchingTest, GetFieldSuggestionsWhenFormIsAutofilled) {

components/autofill/core/browser/data_model/address.cc

@@ -345,40 +345,25 @@ void Address::GetMatchingTypes(const base::string16& text,
   if (!entered_country_code.empty() && country_code == entered_country_code)
     matching_types->insert(ADDRESS_HOME_COUNTRY);
 
-  if (base::FeatureList::IsEnabled(
-          features::kAutofillUseAlternativeStateNameMap)) {
-    base::Optional<AlternativeStateNameMap::CanonicalStateName>
-        canonical_state_name_from_text =
-            AlternativeStateNameMap::GetCanonicalStateName(country_code, text);
-    base::Optional<AlternativeStateNameMap::CanonicalStateName>
-        canonical_state_name_from_profile = GetCanonicalizedStateName();
-
-    if (canonical_state_name_from_text && canonical_state_name_from_profile &&
-        canonical_state_name_from_text.value() ==
-            canonical_state_name_from_profile.value()) {
+  l10n::CaseInsensitiveCompare compare;
+  AutofillProfileComparator comparator(app_locale);
+  // Check to see if the |text| could be the full name or abbreviation of a
+  // state.
+  base::string16 canon_text = comparator.NormalizeForComparison(text);
+  base::string16 state_name;
+  base::string16 state_abbreviation;
+  state_names::GetNameAndAbbreviation(canon_text, &state_name,
+                                      &state_abbreviation);
+
+  if (!state_name.empty() || !state_abbreviation.empty()) {
+    base::string16 canon_profile_state = comparator.NormalizeForComparison(
+        GetInfo(AutofillType(ADDRESS_HOME_STATE), app_locale));
+    if ((!state_name.empty() &&
+         compare.StringsEqual(state_name, canon_profile_state)) ||
+        (!state_abbreviation.empty() &&
+         compare.StringsEqual(state_abbreviation, canon_profile_state))) {
       matching_types->insert(ADDRESS_HOME_STATE);
     }
-  } else {
-    l10n::CaseInsensitiveCompare compare;
-    AutofillProfileComparator comparator(app_locale);
-    // Check to see if the |text| could be the full name or abbreviation of a
-    // state.
-    base::string16 canon_text = comparator.NormalizeForComparison(text);
-    base::string16 state_name;
-    base::string16 state_abbreviation;
-    state_names::GetNameAndAbbreviation(canon_text, &state_name,
-                                        &state_abbreviation);
-
-    if (!state_name.empty() || !state_abbreviation.empty()) {
-      base::string16 canon_profile_state = comparator.NormalizeForComparison(
-          GetInfo(AutofillType(ADDRESS_HOME_STATE), app_locale));
-      if ((!state_name.empty() &&
-           compare.StringsEqual(state_name, canon_profile_state)) ||
-          (!state_abbreviation.empty() &&
-           compare.StringsEqual(state_abbreviation, canon_profile_state))) {
-        matching_types->insert(ADDRESS_HOME_STATE);
-      }
-    }
   }
 }
 

components/autofill/core/browser/data_model/address_unittest.cc

@@ -441,51 +441,6 @@ TEST_P(AddressTest, TestAreStatesEqual) {
   }
 }
 
-// Test that we properly match typed values to stored state data.
-TEST_P(AddressTest, IsState) {
-  base::test::ScopedFeatureList feature;
-  // The feature
-  // |features::kAutofillEnableSupportForMoreStructureInAddresses| is disabled
-  // since it is incompatible with the feature
-  // |features::kAutofillUseStateMappingCache|.
-  feature.InitWithFeatures(
-      {features::kAutofillUseAlternativeStateNameMap},
-      {features::kAutofillEnableSupportForMoreStructureInAddresses});
-
-  test::ClearAlternativeStateNameMapForTesting();
-  test::PopulateAlternativeStateNameMapForTesting();
-
-  Address address;
-  address.SetRawInfo(ADDRESS_HOME_STATE, ASCIIToUTF16("Bavaria"));
-  address.SetRawInfo(ADDRESS_HOME_COUNTRY, ASCIIToUTF16("DE"));
-
-  const char* const kValidMatches[] = {"by", "Bavaria", "Bayern",
-                                       "BY", "B.Y",     "B-Y"};
-  for (const char* valid_match : kValidMatches) {
-    SCOPED_TRACE(valid_match);
-    ServerFieldTypeSet matching_types;
-    address.GetMatchingTypes(ASCIIToUTF16(valid_match), "", &matching_types);
-    ASSERT_EQ(1U, matching_types.size());
-    EXPECT_THAT(matching_types, testing::Contains(ADDRESS_HOME_STATE));
-  }
-
-  const char* const kInvalidMatches[] = {"Garbage", "BYA",   "BYA is a state",
-                                         "Bava",    "Empty", ""};
-  for (const char* invalid_match : kInvalidMatches) {
-    SCOPED_TRACE(invalid_match);
-    ServerFieldTypeSet matching_types;
-    address.GetMatchingTypes(ASCIIToUTF16(invalid_match), "", &matching_types);
-    EXPECT_THAT(matching_types.find(ADDRESS_HOME_STATE), matching_types.end());
-  }
-
-  address.SetRawInfo(ADDRESS_HOME_STATE, ASCIIToUTF16("California"));
-  address.SetRawInfo(ADDRESS_HOME_COUNTRY, ASCIIToUTF16("US"));
-
-  ServerFieldTypeSet matching_types;
-  address.GetMatchingTypes(ASCIIToUTF16("CA"), "", &matching_types);
-  EXPECT_THAT(matching_types.find(ADDRESS_HOME_STATE), matching_types.end());
-}
-
 // Verifies that Address::GetInfo() correctly combines address lines.
 TEST_P(AddressTest, GetStreetAddress) {
   const AutofillType type = AutofillType(ADDRESS_HOME_STREET_ADDRESS);