Fill in more name fields with requestAutocomplete

components/autofill/core/browser/webdata/autofill_table.cc

Index: components/autofill/core/browser/webdata/autofill_table.cc
diff --git a/components/autofill/core/browser/webdata/autofill_table.cc b/components/autofill/core/browser/webdata/autofill_table.cc
index e24ceca1d026d3d75eec6904ebaf145ba0bf1ab4..0d9159f0c7809b77ad0c15d38f9dcd7fb7662523 100644
--- a/components/autofill/core/browser/webdata/autofill_table.cc
+++ b/components/autofill/core/browser/webdata/autofill_table.cc
@@ -169,10 +169,14 @@ scoped_ptr<CreditCard> CreditCardFromStatement(const sql::Statement& s) {
 
 bool AddAutofillProfileNamesToProfile(sql::Connection* db,
                                       AutofillProfile* profile) {
-  sql::Statement s(db->GetUniqueStatement(
-      "SELECT guid, first_name, middle_name, last_name "
-      "FROM autofill_profile_names "
-      "WHERE guid=?"));
+  bool has_full_name =
+      db->DoesColumnExist("autofill_profile_names", "full_name");

[Scott Hess - ex-Googler, 2014/06/04 23:48:17]

I may be missing something, here.  How does the code get to this point without
going through MigrateToVersion57AddFullNameField()?

[Evan Stade, 2014/06/05 18:16:28]

MigrateToVersion37MergeAndCullOlderProfiles calls this function

[Scott Hess - ex-Googler, 2014/06/06 23:55:36]

Wow, you present me with a dilemma.  In general, I really dislike
conditionally-constructed SQL statements.  A lot.  Your's looks correct, but my
dislike stems from experience with such things, so I trust it.  I also dislike
ever modifying migration code, it's risky, once written it should be left alone.

How averse would you be to duplicating the current method to
AddAutofillProfileNamesToProfileForVersion36(), and then modify the existing
function in the obvious way?

One reason I suggest this is that Sqlite.Version.Web shows over 98% of your
databases in versions 52-56.  It would probably be reasonable to look at calling
sql::MetaTable::RazeIfDeprecated() with some suitable value.  Elsewhere I've
used anything older than about two years as the cut-off, but I think 18 or 12
months would be reasonable, too.  Some number of those old databases will
_never_ update, either because the users in question fell off the auto-update
train, or because there is a database or filesystem problem preventing it.  In
the latter case, Raze() might shake some of those people loose and let them make
progress.  Might not, the world is a crazy place.

[So I mean if you duplicated the function, then it would only stick around until
you circled back and deprecated a bunch of stuff.]

[Ilya Sherman, 2014/06/07 00:02:38]

+1.  I have found that, despite the ugliness, migration code should pretty much
always be written to not call out to any methods that aren't specifically
intended for migration.  Otherwise, either (a) core methods get very complex to
support rare migration code paths, or (b) someone comes along and changes a
method not thinking about the migration case, and accidentally breaks migration
in a subtle way (which ideally tests would catch, yes).

I think it would be great to deprecate some of the really old migration code we
have around in this file.  We'd probably want to get buy-in from all the WebData
owners, but the costs of maintaining super old migration code don't really seem
to merit the expected payoff.  Plus, all the things that Scott said.

[Evan Stade, 2014/06/09 17:48:40]

I agree, so I've filed crbug.com/382562

In the meantime, I took Scott's advice of making two separate functions.

+
+  sql::Statement s(db->GetUniqueStatement((
+      std::string("SELECT guid, first_name, middle_name, last_name") +
+      (has_full_name ? ", full_name " : " ") +
+      "FROM autofill_profile_names " +
+      "WHERE guid=?").c_str()));
   s.BindString(0, profile->guid());
 
   if (!s.is_valid())
@@ -181,11 +185,14 @@ bool AddAutofillProfileNamesToProfile(sql::Connection* db,
   std::vector<base::string16> first_names;
   std::vector<base::string16> middle_names;
   std::vector<base::string16> last_names;
+  std::vector<base::string16> full_names;
   while (s.Step()) {
     DCHECK_EQ(profile->guid(), s.ColumnString(0));
     first_names.push_back(s.ColumnString16(1));
     middle_names.push_back(s.ColumnString16(2));
     last_names.push_back(s.ColumnString16(3));
+    if (has_full_name)
+      full_names.push_back(s.ColumnString16(4));
   }
   if (!s.Succeeded())
     return false;
@@ -193,6 +200,8 @@ bool AddAutofillProfileNamesToProfile(sql::Connection* db,
   profile->SetRawMultiInfo(NAME_FIRST, first_names);
   profile->SetRawMultiInfo(NAME_MIDDLE, middle_names);
   profile->SetRawMultiInfo(NAME_LAST, last_names);
+  if (has_full_name)
+    profile->SetRawMultiInfo(NAME_FULL, full_names);
   return true;
 }
 
@@ -250,20 +259,37 @@ bool AddAutofillProfileNames(const AutofillProfile& profile,
   profile.GetRawMultiInfo(NAME_MIDDLE, &middle_names);
   std::vector<base::string16> last_names;
   profile.GetRawMultiInfo(NAME_LAST, &last_names);
+  std::vector<base::string16> full_names;
+  profile.GetRawMultiInfo(NAME_FULL, &full_names);
   DCHECK_EQ(first_names.size(), middle_names.size());
-  DCHECK_EQ(middle_names.size(), last_names.size());
+  DCHECK_EQ(first_names.size(), last_names.size());
+  DCHECK_EQ(first_names.size(), full_names.size());
 
   for (size_t i = 0; i < first_names.size(); ++i) {
+    bool has_full_name =
+        db->DoesColumnExist("autofill_profile_names", "full_name");
+
     // Add the new name.
-    sql::Statement s(db->GetUniqueStatement(
-      "INSERT INTO autofill_profile_names"
-      " (guid, first_name, middle_name, last_name) "
-      "VALUES (?,?,?,?)"));
+    sql::Statement s;
+    if (has_full_name) {
+      s.Assign(db->GetUniqueStatement(
+          "INSERT INTO autofill_profile_names"
+          " (guid, first_name, middle_name, last_name, full_name) "
+          "VALUES (?,?,?,?,?)"));
+    } else {
+      s.Assign(db->GetUniqueStatement(
+          "INSERT INTO autofill_profile_names"
+          " (guid, first_name, middle_name, last_name) "
+          "VALUES (?,?,?,?)"));
+    }
     s.BindString(0, profile.guid());
     s.BindString16(1, first_names[i]);
     s.BindString16(2, middle_names[i]);
     s.BindString16(3, last_names[i]);
 
+    if (has_full_name)
+      s.BindString16(4, full_names[i]);
+
     if (!s.Run())
       return false;
   }
@@ -447,6 +473,9 @@ bool AutofillTable::MigrateToVersion(int version,
     case 56:
       *update_compatible_version = true;
       return MigrateToVersion56AddProfileLanguageCodeForFormatting();
+    case 57:
+      *update_compatible_version = true;
+      return MigrateToVersion57AddFullNameField();
   }
   return true;
 }
@@ -1292,7 +1321,8 @@ bool AutofillTable::InitProfileNamesTable() {
                       "guid VARCHAR, "
                       "first_name VARCHAR, "
                       "middle_name VARCHAR, "
-                      "last_name VARCHAR)")) {
+                      "last_name VARCHAR, "
+                      "full_name VARCHAR)")) {
       NOTREACHED();
       return false;
     }
@@ -2272,4 +2302,9 @@ bool AutofillTable::MigrateToVersion56AddProfileLanguageCodeForFormatting() {
                       "ADD COLUMN language_code VARCHAR");
 }
 
+bool AutofillTable::MigrateToVersion57AddFullNameField() {
+  return db_->Execute("ALTER TABLE autofill_profile_names "
+                      "ADD COLUMN full_name VARCHAR");
+}
+
 }  // namespace autofill