Fill in more name fields with requestAutocomplete

components/autofill/core/browser/webdata/autofill_table.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/webdata/autofill_table.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <map>
@@ skipping 151 matching lines @@
   credit_card->SetRawInfo(CREDIT_CARD_NUMBER, credit_card_number);
   // Intentionally skip column 5, which stores the modification date.
   index++;
   credit_card->set_origin(s.ColumnString(index++));
 
   return credit_card.Pass();
 }
 
 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())
     return false;
 
   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;
 
   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;
 }
 
 bool AddAutofillProfileEmailsToProfile(sql::Connection* db,
                                        AutofillProfile* profile) {
   sql::Statement s(db->GetUniqueStatement(
       "SELECT guid, email "
       "FROM autofill_profile_emails "
       "WHERE guid=?"));
   s.BindString(0, profile->guid());
@@ skipping 37 matching lines @@
 }
 
 bool AddAutofillProfileNames(const AutofillProfile& profile,
                              sql::Connection* db) {
   std::vector<base::string16> first_names;
   profile.GetRawMultiInfo(NAME_FIRST, &first_names);
   std::vector<base::string16> middle_names;
   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;
   }
   return true;
 }
 
 bool AddAutofillProfileEmails(const AutofillProfile& profile,
                               sql::Connection* db) {
   std::vector<base::string16> emails;
   profile.GetRawMultiInfo(EMAIL_ADDRESS, &emails);
@@ skipping 163 matching lines @@
       return MigrateToVersion51AddOriginColumn();
     case 54:
       *update_compatible_version = true;
       return MigrateToVersion54AddI18nFieldsAndRemoveDeprecatedFields();
     case 55:
       *update_compatible_version = true;
       return MigrateToVersion55MergeAutofillDatesTable();
     case 56:
       *update_compatible_version = true;
       return MigrateToVersion56AddProfileLanguageCodeForFormatting();
+    case 57:
+      *update_compatible_version = true;
+      return MigrateToVersion57AddFullNameField();
   }
   return true;
 }
 
 bool AutofillTable::AddFormFieldValues(
     const std::vector<FormFieldData>& elements,
     std::vector<AutofillChange>* changes) {
   return AddFormFieldValuesTime(elements, changes, Time::Now());
 }
 
@@ skipping 825 matching lines @@
   }
   return true;
 }
 
 bool AutofillTable::InitProfileNamesTable() {
   if (!db_->DoesTableExist("autofill_profile_names")) {
     if (!db_->Execute("CREATE TABLE autofill_profile_names ( "
                       "guid VARCHAR, "
                       "first_name VARCHAR, "
                       "middle_name VARCHAR, "
-                      "last_name VARCHAR)")) {
+                      "last_name VARCHAR, "
+                      "full_name VARCHAR)")) {
       NOTREACHED();
       return false;
     }
   }
   return true;
 }
 
 bool AutofillTable::InitProfileEmailsTable() {
   if (!db_->DoesTableExist("autofill_profile_emails")) {
     if (!db_->Execute("CREATE TABLE autofill_profile_emails ( "
@@ skipping 959 matching lines @@
 
 
   return transaction.Commit();
 }
 
 bool AutofillTable::MigrateToVersion56AddProfileLanguageCodeForFormatting() {
   return db_->Execute("ALTER TABLE autofill_profiles "
                       "ADD COLUMN language_code VARCHAR");
 }
 
+bool AutofillTable::MigrateToVersion57AddFullNameField() {
+  return db_->Execute("ALTER TABLE autofill_profile_names "
+                      "ADD COLUMN full_name VARCHAR");
+}
+
 }  // namespace autofill