autofill names - dont parse when calling SetRawInfo(FULL_NAME)

components/autofill/core/browser/autofill_profile.cc

Index: components/autofill/core/browser/autofill_profile.cc
diff --git a/components/autofill/core/browser/autofill_profile.cc b/components/autofill/core/browser/autofill_profile.cc
index 3b242a953e44b315816de8d3b78d5aa84ae0e0f4..05641184ba2560a3bae1301967b7eb5d379947aa 100644
--- a/components/autofill/core/browser/autofill_profile.cc
+++ b/components/autofill/core/browser/autofill_profile.cc
@@ -165,10 +165,11 @@ void CollapseCompoundFieldTypes(ServerFieldTypeSet* type_set) {
   for (ServerFieldTypeSet::iterator it = type_set->begin();
        it != type_set->end(); ++it) {
     switch (*it) {
-      case NAME_FIRST:
       case NAME_MIDDLE:
-      case NAME_LAST:
       case NAME_MIDDLE_INITIAL:
+        collapsed_set.insert(NAME_MIDDLE);
+        break;
+
       case NAME_FULL:
       case NAME_SUFFIX:
         collapsed_set.insert(NAME_FULL);
@@ -577,7 +578,7 @@ void AutofillProfile::OverwriteWithOrAddTo(const AutofillProfile& profile,
   ServerFieldTypeSet field_types;
   profile.GetNonEmptyTypes(app_locale, &field_types);
 
-  // Only transfer "full" types (e.g. full name) and not fragments (e.g.
+  // Only transfer "full" types (e.g. full phone number) and not fragments (e.g.
   // first name, last name).

[Ilya Sherman, 2014/06/24 07:21:37]

This comment needs more updating.

[Evan Stade, 2014/06/27 00:06:18]

reverted

   CollapseCompoundFieldTypes(&field_types);

[Ilya Sherman, 2014/06/24 07:21:37]

I'm concerned that treating first, middle, last, and full names as separate
lists will lead to incorrect behavior when merging names that partially match. 
For example, suppose I start with a profile that has the name "John Smith", and
then try to merge in a profile that has the name "Johnny Smith".  I think this
code will arrive at a new profile that has two first names, and only one last
name, which isn't a valid state.  Am I missing something that would prevent this
issue?

[Evan Stade, 2014/06/27 00:06:19]

Quite right, I didn't look at this closely enough and I sent the CL out for
review before looking at the unit test results. Sorry. The CL should be a bit
more fleshed out now.

I modified the loop below to try to improve its readability (for example, it was
pretty hard to see that if group == NAME, the previous 30 lines of code are
basically ignored).

 
@@ -643,10 +644,11 @@ bool AutofillProfile::SupportsMultiValue(ServerFieldType type) {
 // static
 void AutofillProfile::CreateDifferentiatingLabels(
     const std::vector<AutofillProfile*>& profiles,
+    const std::string& app_locale,
     std::vector<base::string16>* labels) {
   const size_t kMinimalFieldsShown = 2;
   CreateInferredLabels(profiles, NULL, UNKNOWN_TYPE, kMinimalFieldsShown,
-                       labels);
+                       app_locale, labels);
   DCHECK_EQ(profiles.size(), labels->size());
 }
 
@@ -656,6 +658,7 @@ void AutofillProfile::CreateInferredLabels(
     const std::vector<ServerFieldType>* suggested_fields,
     ServerFieldType excluded_field,
     size_t minimal_fields_shown,
+    const std::string& app_locale,
     std::vector<base::string16>* labels) {
   std::vector<ServerFieldType> fields_to_use;
   GetFieldsForDistinguishingProfiles(suggested_fields, excluded_field,
@@ -668,7 +671,8 @@ void AutofillProfile::CreateInferredLabels(
   for (size_t i = 0; i < profiles.size(); ++i) {
     base::string16 label =
         profiles[i]->ConstructInferredLabel(fields_to_use,
-                                            minimal_fields_shown);
+                                            minimal_fields_shown,
+                                            app_locale);
     labels_to_profiles[label].push_back(i);
   }
 
@@ -685,7 +689,7 @@ void AutofillProfile::CreateInferredLabels(
       // We have more than one profile with the same label, so add
       // differentiating fields.
       CreateInferredLabelsHelper(profiles, it->second, fields_to_use,
-                                 minimal_fields_shown, labels);
+                                 minimal_fields_shown, app_locale, labels);
     }
   }
 }
@@ -737,7 +741,8 @@ void AutofillProfile::AddPhoneIfUnique(
 
 base::string16 AutofillProfile::ConstructInferredLabel(
     const std::vector<ServerFieldType>& included_fields,
-    size_t num_fields_to_use) const {
+    size_t num_fields_to_use,
+    const std::string& app_locale) const {
   const base::string16 separator =
       l10n_util::GetStringUTF16(IDS_AUTOFILL_ADDRESS_SUMMARY_SEPARATOR);
 
@@ -747,7 +752,7 @@ base::string16 AutofillProfile::ConstructInferredLabel(
            included_fields.begin();
        it != included_fields.end() && num_fields_used < num_fields_to_use;
        ++it) {
-    base::string16 field = GetRawInfo(*it);
+    base::string16 field = GetInfo(AutofillType(*it), app_locale);
     if (field.empty())
       continue;
 
@@ -772,6 +777,7 @@ void AutofillProfile::CreateInferredLabelsHelper(
     const std::list<size_t>& indices,
     const std::vector<ServerFieldType>& fields,
     size_t num_fields_to_include,
+    const std::string& app_locale,
     std::vector<base::string16>* labels) {
   // For efficiency, we first construct a map of fields to their text values and
   // each value's frequency.
@@ -837,8 +843,8 @@ void AutofillProfile::CreateInferredLabelsHelper(
         break;
     }
 
-    (*labels)[*it] =
-        profile->ConstructInferredLabel(label_fields, label_fields.size());
+    (*labels)[*it] = profile->ConstructInferredLabel(
+        label_fields, label_fields.size(), app_locale);
   }
 }