Preserving first/middle/last names when an Autofill profile is submitted

chrome/browser/ui/webui/options/autofill_options_handler.cc

Index: chrome/browser/ui/webui/options/autofill_options_handler.cc
diff --git a/chrome/browser/ui/webui/options/autofill_options_handler.cc b/chrome/browser/ui/webui/options/autofill_options_handler.cc
index 4b512d5e5257215df44024e48df7789d4e48783d..aa0c0da3fec26735b3dd2d5190279df626fe761d 100644
--- a/chrome/browser/ui/webui/options/autofill_options_handler.cc
+++ b/chrome/browser/ui/webui/options/autofill_options_handler.cc
@@ -394,8 +394,8 @@ void AutofillOptionsHandler::LoadAddressEditor(const base::ListValue* args) {
     return;
   }
 
-  AutofillProfile* profile = personal_data_->GetProfileByGUID(guid);
-  if (!profile) {
+  prior_profile_ = personal_data_->GetProfileByGUID(guid);
+  if (!prior_profile_) {
     // There is a race where a user can click once on the close button and
     // quickly click again on the list item before the item is removed (since
     // the list is not updated until the model tells the list an item has been
@@ -405,7 +405,7 @@ void AutofillOptionsHandler::LoadAddressEditor(const base::ListValue* args) {
   }
 
   base::DictionaryValue address;
-  AutofillProfileToDictionary(*profile, &address);
+  AutofillProfileToDictionary(*prior_profile_, &address);
 
   web_ui()->CallJavascriptFunction("AutofillOptions.editAddress", address);
 }
@@ -519,6 +519,24 @@ void AutofillOptionsHandler::SetAddress(const base::ListValue* args) {
   if (args->GetString(arg_counter++, &value))
     profile.set_language_code(base::UTF16ToUTF8(value));
 
+  // Although First/Middle/Last are not displayed on the form, we transfer this
+  // information when they match the full name given. This is because it may not
+  // be possible later to correctly tokenize the concatenated full name--e.g.,
+  // when the last name contains a space, the first word would be treated as a
+  // middle name.
+  if (prior_profile_ &&
+      ShouldTransferNameComponents(profile, *prior_profile_)) {
+    profile.SetRawInfo(autofill::NAME_FIRST,
+                       prior_profile_->GetRawInfo(autofill::NAME_FIRST));
+    profile.SetRawInfo(autofill::NAME_MIDDLE,
+                       prior_profile_->GetRawInfo(autofill::NAME_MIDDLE));
+    profile.SetRawInfo(autofill::NAME_LAST,
+                       prior_profile_->GetRawInfo(autofill::NAME_LAST));
+    profile.SetRawInfo(
+        autofill::NAME_MIDDLE_INITIAL,

[Mathieu, 2016/04/11 15:01:21]

curious: I'm not familiar with middle initial. In your test, is it always filled
alongside NAME_MIDDLE (i.e. the first letter is taken)?

[tmartino, 2016/04/12 20:48:27]

It appears that it's not stored anywhere:

https://code.google.com/p/chromium/codesearch#chromium/src/components/autofil...

I removed this setter call, but kept the references in ProfileMatchesFullName.

+        prior_profile_->GetRawInfo(autofill::NAME_MIDDLE_INITIAL));
+  }
+
   if (!base::IsValidGUID(profile.guid())) {
     profile.set_guid(base::GenerateGUID());
     personal_data_->AddProfile(profile);
@@ -614,4 +632,44 @@ void AutofillOptionsHandler::AutofillProfileToDictionary(
   address->Set(kComponents, components.release());
 }
 
+// Concatenates two strings, adding a space and, optionally, a period.
+base::string16 StringConcatenationHelper(base::string16 first,
+                                         base::string16 second,
+                                         bool period) {
+  base::string16 delimiter =
+      period ? base::UTF8ToUTF16(". ") : base::UTF8ToUTF16(" ");
+  return first + delimiter + second;
+}
+
+// static
+bool AutofillOptionsHandler::ShouldTransferNameComponents(

[Mathieu, 2016/04/11 15:01:21]

How about renaming this to ProfileExpressesFullName[1] and integrating it into
autofill_data_util as a static function? 

[1] Note: you may pick a better name :)

[tmartino, 2016/04/12 20:48:27]

Done.

+    const autofill::AutofillProfile& profile,
+    const autofill::AutofillProfile& prior_profile) {
+  base::string16 full_name =
+      profile.GetInfo(AutofillType(autofill::NAME_FULL),
+                      g_browser_process->GetApplicationLocale());
+  base::string16 first_last = StringConcatenationHelper(

[Mathieu, 2016/04/11 15:01:21]

should probably return early if the comparison is true to avoid extra work.

if (!full_name.compare(first_last))
  return true;

etc.

[tmartino, 2016/04/12 20:48:27]

Done.

+      prior_profile.GetRawInfo(autofill::NAME_FIRST),
+      prior_profile.GetRawInfo(autofill::NAME_LAST), false);
+  base::string16 first_middle_last = StringConcatenationHelper(
+      StringConcatenationHelper(prior_profile.GetRawInfo(autofill::NAME_FIRST),
+                                prior_profile.GetRawInfo(autofill::NAME_MIDDLE),
+                                false),
+      prior_profile.GetRawInfo(autofill::NAME_LAST), false);
+  base::string16 first_m_last = StringConcatenationHelper(
+      StringConcatenationHelper(
+          prior_profile.GetRawInfo(autofill::NAME_FIRST),
+          prior_profile.GetRawInfo(autofill::NAME_MIDDLE_INITIAL), false),
+      prior_profile.GetRawInfo(autofill::NAME_LAST), false);
+  base::string16 first_m_period_last = StringConcatenationHelper(
+      StringConcatenationHelper(
+          prior_profile.GetRawInfo(autofill::NAME_FIRST),
+          prior_profile.GetRawInfo(autofill::NAME_MIDDLE_INITIAL), false),
+      prior_profile.GetRawInfo(autofill::NAME_LAST), true);
+  return !(full_name.compare(first_last) &&
+           full_name.compare(first_middle_last) &&
+           full_name.compare(first_m_last) &&
+           full_name.compare(first_m_period_last));
+}
+
 }  // namespace options