autofill names - dont parse when calling SetRawInfo(FULL_NAME)

components/autofill/core/browser/autofill_profile.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/autofill/core/browser/autofill_profile.h"
 
 #include <algorithm>
 #include <functional>
 #include <map>
 #include <ostream>
@@ skipping 141 matching lines @@
                        const std::vector<T>& form_group_items,
                        const std::string& app_locale,
                        std::vector<base::string16>* values) {
   values->resize(form_group_items.size());
   for (size_t i = 0; i < values->size(); ++i) {
     (*values)[i] = GetFormGroupInfo(form_group_items[i], type, app_locale);
   }
 }
 
 // Collapse compound field types to their "full" type.  I.e. First name
 // collapses to full name, area code collapses to full phone, etc.

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

Please update this comment.

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

reverted the change

 void CollapseCompoundFieldTypes(ServerFieldTypeSet* type_set) {
   ServerFieldTypeSet collapsed_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);
         break;
 
       case PHONE_HOME_NUMBER:
       case PHONE_HOME_CITY_CODE:
       case PHONE_HOME_COUNTRY_CODE:
       case PHONE_HOME_CITY_AND_NUMBER:
       case PHONE_HOME_WHOLE_NUMBER:
@@ skipping 388 matching lines @@
 void AutofillProfile::OverwriteWithOrAddTo(const AutofillProfile& profile,
                                            const std::string& app_locale) {
   // Verified profiles should never be overwritten with unverified data.
   DCHECK(!IsVerified() || profile.IsVerified());
   set_origin(profile.origin());
   set_language_code(profile.language_code());
 
   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).

 
   // TODO(isherman): Revisit this decision in the context of i18n and storing
   // full addresses rather than storing 1-to-2 lines of an address.
   // For addresses, do the opposite: transfer individual address lines, rather
   // than full addresses.
   field_types.erase(ADDRESS_HOME_STREET_ADDRESS);
 
   for (ServerFieldTypeSet::const_iterator iter = field_types.begin();
        iter != field_types.end(); ++iter) {
     if (AutofillProfile::SupportsMultiValue(*iter)) {
@@ skipping 43 matching lines @@
   return group == NAME ||
          group == NAME_BILLING ||
          group == EMAIL ||
          group == PHONE_HOME ||
          group == PHONE_BILLING;
 }
 
 // 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());
 }
 
 // static
 void AutofillProfile::CreateInferredLabels(
     const std::vector<AutofillProfile*>& profiles,
     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,
                                      &fields_to_use);
 
   // Construct the default label for each profile. Also construct a map that
   // associates each label with the profiles that have this label. This map is
   // then used to detect which labels need further differentiating fields.
   std::map<base::string16, std::list<size_t> > labels_to_profiles;
   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);
   }
 
   labels->resize(profiles.size());
   for (std::map<base::string16, std::list<size_t> >::const_iterator it =
            labels_to_profiles.begin();
        it != labels_to_profiles.end(); ++it) {
     if (it->second.size() == 1) {
       // This label is unique, so use it without any further ado.
       base::string16 label = it->first;
       size_t profile_index = it->second.front();
       (*labels)[profile_index] = label;
     } else {
       // 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);
     }
   }
 }
 
 void AutofillProfile::GetSupportedTypes(
     ServerFieldTypeSet* supported_types) const {
   FormGroupList info = FormGroups();
   for (FormGroupList::const_iterator it = info.begin(); it != info.end(); ++it)
     (*it)->GetSupportedTypes(supported_types);
 }
@@ skipping 31 matching lines @@
       base::UTF16ToASCII(GetRawInfo(ADDRESS_HOME_COUNTRY));
   if (std::find_if(existing_phones->begin(), existing_phones->end(),
                    FindByPhone(phone, country_code, app_locale)) ==
       existing_phones->end()) {
     existing_phones->push_back(phone);
   }
 }
 
 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);
 
   base::string16 label;
   size_t num_fields_used = 0;
   for (std::vector<ServerFieldType>::const_iterator it =
            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;
 
     if (!label.empty())
       label.append(separator);
 
     label.append(field);
     ++num_fields_used;
   }
 
   // Flatten the label if need be.
   const base::string16& line_separator =
       l10n_util::GetStringUTF16(IDS_AUTOFILL_ADDRESS_LINE_SEPARATOR);
   base::ReplaceChars(label, base::ASCIIToUTF16("\n"), line_separator, &label);
 
   return label;
 }
 
 // static
 void AutofillProfile::CreateInferredLabelsHelper(
     const std::vector<AutofillProfile*>& profiles,
     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.
   std::map<ServerFieldType,
            std::map<base::string16, size_t> > field_text_frequencies_by_field;
   for (std::vector<ServerFieldType>::const_iterator field = fields.begin();
        field != fields.end(); ++field) {
     std::map<base::string16, size_t>& field_text_frequencies =
         field_text_frequencies_by_field[*field];
 
@@ skipping 45 matching lines @@
 
       label_fields.push_back(*field);
 
       // If we've (1) found a differentiating field and (2) found at least
       // |num_fields_to_include| non-empty fields, we're done!
       if (found_differentiating_field &&
           label_fields.size() >= num_fields_to_include)
         break;
     }
 
-    (*labels)[*it] =
-        profile->ConstructInferredLabel(label_fields, label_fields.size());
+    (*labels)[*it] = profile->ConstructInferredLabel(
+        label_fields, label_fields.size(), app_locale);
   }
 }
 
 AutofillProfile::FormGroupList AutofillProfile::FormGroups() const {
   FormGroupList v(5);
   v[0] = &name_[0];
   v[1] = &email_[0];
   v[2] = &company_;
   v[3] = &phone_number_[0];
   v[4] = &address_;
@@ skipping 67 matching lines @@
       << UTF16ToUTF8(profile.GetRawInfo(ADDRESS_HOME_SORTING_CODE))
       << " "
       << UTF16ToUTF8(profile.GetRawInfo(ADDRESS_HOME_COUNTRY))
       << " "
       << profile.language_code()
       << " "
       << UTF16ToUTF8(MultiString(profile, PHONE_HOME_WHOLE_NUMBER));
 }
 
 }  // namespace autofill