Delete unmasked credit cards when clearing data.

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 f05a739e7c905e15bfed9dc2c66a0fa74ca64604..7fefa6b47926604bd6a0ad1fc7b88bc5935d46d9 100644
--- a/components/autofill/core/browser/webdata/autofill_table.cc
+++ b/components/autofill/core/browser/webdata/autofill_table.cc
@@ -472,6 +472,9 @@ bool AutofillTable::MigrateToVersion(int version,
     case 63:
       *update_compatible_version = false;
       return MigrateToVersion63AddServerRecipientName();
+    case 64:
+      *update_compatible_version = false;
+      return MigrateToVersion64AddUnmaskDate();
   }
   return true;
 }
@@ -1194,6 +1197,10 @@ bool AutofillTable::GetServerCreditCards(
 
 void AutofillTable::SetServerCreditCards(
     const std::vector<CreditCard>& credit_cards) {
+  sql::Transaction transaction(db_);
+  if (!transaction.Begin())
+    return;
+
   // Delete all old values.
   sql::Statement masked_delete(db_->GetUniqueStatement(
       "DELETE FROM masked_credit_cards"));
@@ -1202,6 +1209,7 @@ void AutofillTable::SetServerCreditCards(
   // Delete all items in the unmasked table that aren't in the new set.
   sql::Statement get_unmasked(db_->GetUniqueStatement(
       "SELECT id FROM unmasked_credit_cards"));
+  std::set<std::string> kept_unmasked_entries;
   while (get_unmasked.Step()) {
     // We expect relatively few cards, just do brute-force.
     std::string server_id = get_unmasked.ColumnString(0);
@@ -1212,7 +1220,9 @@ void AutofillTable::SetServerCreditCards(
         break;
       }
     }
-    if (!found_card) {
+    if (found_card) {
+      kept_unmasked_entries.insert(server_id);
+    } else {
       // This unmasked card in the DB isn't present in the input. The statement
       // is compiled every time because it's much more likely that this is never
       // executed than it runs more than once.
@@ -1236,9 +1246,10 @@ void AutofillTable::SetServerCreditCards(
       "VALUES (?,?,?,?,?,?,?)"));
   sql::Statement unmasked_insert(db_->GetUniqueStatement(
       "INSERT INTO unmasked_credit_cards("
-      "id,"                     // 0
-      "card_number_encrypted)"  // 1
-      "VALUES (?,?)"));
+      "id,"                      // 0
+      "card_number_encrypted, "  // 1
+      "unmask_date)"             // 2
+      "VALUES (?,?,?)"));
   for (const CreditCard& card : credit_cards) {
     DCHECK(card.record_type() != CreditCard::LOCAL_CARD);
 
@@ -1255,17 +1266,25 @@ void AutofillTable::SetServerCreditCards(
     masked_insert.Run();
     masked_insert.Reset(true);
 
-    if (card.record_type() == CreditCard::FULL_SERVER_CARD) {
-      // Unmasked cards also get an entry in the unmasked table. Note that the
-      // input card could be MASKED but if we have an UNMASKED entry for that
-      // card already, it will be preserved.
+    if (kept_unmasked_entries.find(card.server_id()) ==
+            kept_unmasked_entries.end() &&
+        card.record_type() == CreditCard::FULL_SERVER_CARD) {
+      // New unmasked cards also get an entry in the unmasked table. In
+      // practice this will only happen for tests since the server cards will
+      // all be marked masked (if a server card is manually unmasked, we will
+      // have kept the existing entry in the table and won't get here).
       unmasked_insert.BindString(0, card.server_id());
       BindEncryptedCardToColumn(&unmasked_insert, 1,
                                 card.GetRawInfo(CREDIT_CARD_NUMBER));
+      // Unmask time for this card is now.
+      unmasked_insert.BindInt64(2, Time::Now().ToInternalValue());
       unmasked_insert.Run();
       unmasked_insert.Reset(true);
     }
   }
+
+  if (!transaction.Commit())
+    return;

[Evan Stade, 2015/03/06 00:20:51]

nit: unnecessary return

 }
 
 bool AutofillTable::UnmaskServerCreditCard(const std::string& id,
@@ -1273,9 +1292,13 @@ bool AutofillTable::UnmaskServerCreditCard(const std::string& id,
   // Make sure there aren't duplicates for this card.
   MaskServerCreditCard(id);
   sql::Statement s(db_->GetUniqueStatement(
-      "INSERT INTO unmasked_credit_cards(id, card_number_encrypted,"
-      "            use_count, use_date) "
-      "VALUES (?,?,?,?)"));
+      "INSERT INTO unmasked_credit_cards("
+          "id,"
+          "card_number_encrypted,"
+          "use_count,"
+          "use_date,"
+          "unmask_date)"
+      "VALUES (?,?,?,?,?)"));
   s.BindString(0, id);
 
   std::string encrypted_data;
@@ -1284,8 +1307,11 @@ bool AutofillTable::UnmaskServerCreditCard(const std::string& id,
              static_cast<int>(encrypted_data.length()));
 
   // Unmasking counts as a usage, so set the stats accordingly.
-  s.BindInt64(2, 1);
-  s.BindInt64(3, base::Time::Now().ToInternalValue());
+  base::Time now = base::Time::Now();
+  s.BindInt64(2, 1);                      // use_count
+  s.BindInt64(3, now.ToInternalValue());  // use_date
+
+  s.BindInt64(4, now.ToInternalValue());  // unmask_date
 
   s.Run();
   return db_->GetLastChangeCount() > 0;
@@ -1327,8 +1353,8 @@ bool AutofillTable::UpdateCreditCard(const CreditCard& credit_card) {
   sql::Statement s(db_->GetUniqueStatement(
       "UPDATE credit_cards "
       "SET guid=?, name_on_card=?, expiration_month=?,"
-      "    expiration_year=?, card_number_encrypted=?, use_count=?, use_date=?,"
-      "    date_modified=?, origin=?"
+          "expiration_year=?, card_number_encrypted=?, use_count=?, use_date=?,"
+          "date_modified=?, origin=?"
       "WHERE guid=?"));
   BindCreditCardToStatement(
       credit_card,
@@ -1407,8 +1433,19 @@ bool AutofillTable::RemoveAutofillDataModifiedBetween(
       "WHERE date_modified >= ? AND date_modified < ?"));
   s_credit_cards.BindInt64(0, delete_begin_t);
   s_credit_cards.BindInt64(1, delete_end_t);
+  if (!s_credit_cards.Run())
+    return false;
 
-  return s_credit_cards.Run();
+  // Remove unmasked credit cards in the time range.
+  sql::Statement s_unmasked_cards(db_->GetUniqueStatement(
+      "DELETE FROM unmasked_credit_cards "
+      "WHERE unmask_date >= ? AND unmask_date < ?"));
+  s_unmasked_cards.BindInt64(0, delete_begin.ToInternalValue());
+  s_unmasked_cards.BindInt64(1, delete_end.ToInternalValue());
+  if (!s_unmasked_cards.Run())

[Evan Stade, 2015/03/06 00:20:51]

nit: return s_unmasked_cards.Run()

+    return false;
+
+  return true;
 }
 
 bool AutofillTable::RemoveOriginURLsModifiedBetween(
@@ -1672,7 +1709,8 @@ bool AutofillTable::InitUnmaskedCreditCardsTable() {
                       "id VARCHAR,"
                       "card_number_encrypted VARCHAR, "
                       "use_count INTEGER NOT NULL DEFAULT 0, "
-                      "use_date INTEGER NOT NULL DEFAULT 0)")) {
+                      "use_date INTEGER NOT NULL DEFAULT 0, "
+                      "unmask_date INTEGER NOT NULL DEFAULT 0)")) {
       NOTREACHED();
       return false;
     }
@@ -1696,7 +1734,8 @@ bool AutofillTable::InitServerAddressesTable() {
                       "sorting_code VARCHAR,"
                       "country_code VARCHAR,"
                       "language_code VARCHAR, "  // Space required.
-                      "recipient_name VARCHAR)")) {
+                      "recipient_name VARCHAR, "  // Ditto.
+                      "phone_number VARCHAR)")) {
       NOTREACHED();
       return false;
     }
@@ -1989,4 +2028,18 @@ bool AutofillTable::MigrateToVersion63AddServerRecipientName() {
   return true;
 }
 
+bool AutofillTable::MigrateToVersion64AddUnmaskDate() {

[Evan Stade, 2015/03/06 00:20:51]

transaction

+  if (!db_->DoesColumnExist("unmasked_credit_cards", "unmask_date") &&
+      !db_->Execute("ALTER TABLE unmasked_credit_cards ADD COLUMN "
+                    "unmask_date INTEGER NOT NULL DEFAULT 0")) {
+    return false;
+  }
+  if (!db_->DoesColumnExist("server_addresses", "phone_number") &&
+      !db_->Execute("ALTER TABLE server_addresses ADD COLUMN "
+                    "phone_number VARCHAR")) {
+    return false;
+  }
+  return true;
+}
+
 }  // namespace autofill