autofill names - dont parse when calling SetRawInfo(FULL_NAME)

components/autofill/core/browser/contact_info.cc

Index: components/autofill/core/browser/contact_info.cc
diff --git a/components/autofill/core/browser/contact_info.cc b/components/autofill/core/browser/contact_info.cc
index 7571e5f2e37d4ee04b517d029904e6f84bb5eed9..7f67bdb27d8c6e11971eb4520ac072415295eece 100644
--- a/components/autofill/core/browser/contact_info.cc
+++ b/components/autofill/core/browser/contact_info.cc
@@ -159,9 +159,10 @@ NameInfo& NameInfo::operator=(const NameInfo& info) {
   return *this;
 }
 
-bool NameInfo::EqualsIgnoreCase(const NameInfo& info) {
+bool NameInfo::ParsedBitsAreEqual(const NameInfo& info) {
   return (StringToLowerASCII(given_) == StringToLowerASCII(info.given_) &&
           StringToLowerASCII(middle_) == StringToLowerASCII(info.middle_) &&
+          middle_.size() == info.middle_.size() &&

[Ilya Sherman, 2014/06/27 06:25:08]

Why is this check necessary?  How can the strings be equal without being the
same size?  If somehow that's possible, why is it only middle names that need
special treatment?

[Evan Stade, 2014/06/28 01:01:35]

this is totally not necessary. It was just me being paranoid  while I was
debugging a test failure, and then being forgetful when it came time to remove
it.

           StringToLowerASCII(family_) == StringToLowerASCII(info.family_));
 }
 
@@ -189,7 +190,7 @@ base::string16 NameInfo::GetRawInfo(ServerFieldType type) const {
       return MiddleInitial();
 
     case NAME_FULL:
-      return FullName();
+      return full_;
 
     default:
       return base::string16();
@@ -199,10 +200,6 @@ base::string16 NameInfo::GetRawInfo(ServerFieldType type) const {
 void NameInfo::SetRawInfo(ServerFieldType type, const base::string16& value) {
   DCHECK_EQ(NAME, AutofillType(type).group());
 
-  // Always clear out the full name if we're making a change.
-  if (value != GetRawInfo(type))
-    full_.clear();
-
   switch (type) {
     case NAME_FIRST:
       given_ = value;
@@ -218,9 +215,7 @@ void NameInfo::SetRawInfo(ServerFieldType type, const base::string16& value) {
       break;
 
     case NAME_FULL:
-      // TODO(estade): this should just set |full_|; only SetInfo should attempt
-      // to be smart. http://crbug.com/384640
-      SetFullName(value);
+      full_ = value;
       break;
 
     default:
@@ -228,6 +223,29 @@ void NameInfo::SetRawInfo(ServerFieldType type, const base::string16& value) {
   }
 }
 
+base::string16 NameInfo::GetInfo(const AutofillType& type,
+                                 const std::string& app_locale) const {
+  if (type.GetStorableType() == NAME_FULL)
+    return FullName();
+
+  return GetRawInfo(type.GetStorableType());
+}
+
+bool NameInfo::SetInfo(const AutofillType& type,
+                       const base::string16& value,
+                       const std::string& app_locale) {
+  // Always clear out the full name if we're making a change.
+  if (value != GetInfo(type, app_locale))
+    full_.clear();
+
+  if (type.GetStorableType() == NAME_FULL) {
+    SetFullName(value);
+    return true;
+  }
+
+  return FormGroup::SetInfo(type, value, app_locale);
+}
+
 base::string16 NameInfo::FullName() const {
   if (!full_.empty())
     return full_;
@@ -256,14 +274,6 @@ base::string16 NameInfo::MiddleInitial() const {
 }
 
 void NameInfo::SetFullName(const base::string16& full) {
-  // Hack: don't do anything if this wouldn't change the full, concatenated
-  // name. Otherwise when unpickling data from the database, "First|Middle|"
-  // will get parsed as "First||Middle".
-  // TODO(estade): we should be able to remove this when fixing the TODO in
-  // SetRawInfo. http://crbug.com/384640
-  if (FullName() == full)
-    return;
-
   full_ = full;
 
   // If |full| is empty, leave the other name parts alone. This might occur