Password Login Database: report correct changes from AddLogin().

components/password_manager/core/browser/login_database.cc

 // Copyright 2014 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/password_manager/core/browser/login_database.h"
 
 #include <algorithm>
 #include <limits>
 
+#include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/pickle.h"
 #include "base/strings/string_util.h"
 #include "base/time/time.h"
 #include "components/autofill/core/common/password_form.h"
 #include "sql/connection.h"
 #include "sql/statement.h"
 #include "sql/transaction.h"
@@ skipping 22 matching lines @@
   COLUMN_DATE_CREATED,
   COLUMN_BLACKLISTED_BY_USER,
   COLUMN_SCHEME,
   COLUMN_PASSWORD_TYPE,
   COLUMN_POSSIBLE_USERNAMES,
   COLUMN_TIMES_USED,
   COLUMN_FORM_DATA,
   COLUMN_USE_ADDITIONAL_AUTH
 };
 
+void BindAddStatement(const PasswordForm& form,
+                      const std::string& encrypted_password,
+                      const Pickle& usernames_pickle,

[Garrett Casto, 2014/05/14 07:46:35]

Any reason why you have |username_pickle| as a separate param? Not sure why it
shouldn't be deduced from |form|. If it's just because SerializeVector is
currently a member of LoginDatabase, I would just make it a free function.

[vasilii, 2014/05/14 11:20:18]

Done.

+                      sql::Statement* s) {
+  s->BindString(COLUMN_ORIGIN_URL, form.origin.spec());
+  s->BindString(COLUMN_ACTION_URL, form.action.spec());
+  s->BindString16(COLUMN_USERNAME_ELEMENT, form.username_element);
+  s->BindString16(COLUMN_USERNAME_VALUE, form.username_value);
+  s->BindString16(COLUMN_PASSWORD_ELEMENT, form.password_element);
+  s->BindBlob(COLUMN_PASSWORD_VALUE, encrypted_password.data(),
+              static_cast<int>(encrypted_password.length()));
+  s->BindString16(COLUMN_SUBMIT_ELEMENT, form.submit_element);
+  s->BindString(COLUMN_SIGNON_REALM, form.signon_realm);
+  s->BindInt(COLUMN_SSL_VALID, form.ssl_valid);
+  s->BindInt(COLUMN_PREFERRED, form.preferred);
+  s->BindInt64(COLUMN_DATE_CREATED, form.date_created.ToTimeT());
+  s->BindInt(COLUMN_BLACKLISTED_BY_USER, form.blacklisted_by_user);
+  s->BindInt(COLUMN_SCHEME, form.scheme);
+  s->BindInt(COLUMN_PASSWORD_TYPE, form.type);
+  s->BindBlob(COLUMN_POSSIBLE_USERNAMES,
+              usernames_pickle.data(),
+              usernames_pickle.size());
+  s->BindInt(COLUMN_TIMES_USED, form.times_used);
+  Pickle form_data_pickle;
+  autofill::SerializeFormData(form.form_data, &form_data_pickle);
+  s->BindBlob(COLUMN_FORM_DATA,
+              form_data_pickle.data(),
+              form_data_pickle.size());
+  s->BindInt(COLUMN_USE_ADDITIONAL_AUTH, form.use_additional_authentication);
+}
+
+void AddCallback(int err, sql::Statement* /*stmt*/) {
+  if (err == 19 /*SQLITE_CONSTRAINT*/)
+    DLOG(WARNING) << "LoginDatabase::AddLogin updated an existing form";
+}
+
 }  // namespace
 
 LoginDatabase::LoginDatabase() {
 }
 
 LoginDatabase::~LoginDatabase() {
 }
 
 bool LoginDatabase::Init(const base::FilePath& db_path) {
   // Set pragmas for a small, private database (based on WebDatabase).
@@ skipping 169 matching lines @@
           "PasswordManager.TimesGeneratedPasswordUsed",
           usage_statement.ColumnInt(1), 0, 100, 10);
     } else {
       UMA_HISTOGRAM_CUSTOM_COUNTS(
           "PasswordManager.TimesPasswordUsed",
           usage_statement.ColumnInt(1), 0, 100, 10);
     }
   }
 }
 
-bool LoginDatabase::AddLogin(const PasswordForm& form) {
+PasswordStoreChangeList LoginDatabase::AddLogin(const PasswordForm& form) {
+  PasswordStoreChangeList list;
   std::string encrypted_password;
   if (EncryptedString(form.password_value, &encrypted_password) !=
           ENCRYPTION_RESULT_SUCCESS)
-    return false;
+    return list;
 
   // You *must* change LoginTableColumns if this query changes.
   sql::Statement s(db_.GetCachedStatement(SQL_FROM_HERE,
+      "INSERT INTO logins "
+      "(origin_url, action_url, username_element, username_value, "
+      " password_element, password_value, submit_element, "
+      " signon_realm, ssl_valid, preferred, date_created, blacklisted_by_user, "
+      " scheme, password_type, possible_usernames, times_used, form_data, "
+      " use_additional_auth) VALUES "
+      "(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"));
+  BindAddStatement(form, encrypted_password,
+                   SerializeVector(form.other_possible_usernames),
+                   &s);
+  db_.set_error_callback(base::Bind(&AddCallback));
+  const bool success = s.Run();
+  db_.reset_error_callback();
+  if (success) {
+    list.push_back(PasswordStoreChange(PasswordStoreChange::ADD, form));
+    return list;
+  }
+  // Repeat the same statement but with REPLACE semantic.
+  s.Assign(db_.GetCachedStatement(SQL_FROM_HERE,
       "INSERT OR REPLACE INTO logins "
       "(origin_url, action_url, username_element, username_value, "
       " password_element, password_value, submit_element, "
       " signon_realm, ssl_valid, preferred, date_created, blacklisted_by_user, "
       " scheme, password_type, possible_usernames, times_used, form_data, "
       " use_additional_auth) VALUES "
       "(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"));
-  s.BindString(COLUMN_ORIGIN_URL, form.origin.spec());
-  s.BindString(COLUMN_ACTION_URL, form.action.spec());
-  s.BindString16(COLUMN_USERNAME_ELEMENT, form.username_element);
-  s.BindString16(COLUMN_USERNAME_VALUE, form.username_value);
-  s.BindString16(COLUMN_PASSWORD_ELEMENT, form.password_element);
-  s.BindBlob(COLUMN_PASSWORD_VALUE, encrypted_password.data(),
-              static_cast<int>(encrypted_password.length()));
-  s.BindString16(COLUMN_SUBMIT_ELEMENT, form.submit_element);
-  s.BindString(COLUMN_SIGNON_REALM, form.signon_realm);
-  s.BindInt(COLUMN_SSL_VALID, form.ssl_valid);
-  s.BindInt(COLUMN_PREFERRED, form.preferred);
-  s.BindInt64(COLUMN_DATE_CREATED, form.date_created.ToTimeT());
-  s.BindInt(COLUMN_BLACKLISTED_BY_USER, form.blacklisted_by_user);
-  s.BindInt(COLUMN_SCHEME, form.scheme);
-  s.BindInt(COLUMN_PASSWORD_TYPE, form.type);
-  Pickle usernames_pickle = SerializeVector(form.other_possible_usernames);
-  s.BindBlob(COLUMN_POSSIBLE_USERNAMES,
-             usernames_pickle.data(),
-             usernames_pickle.size());
-  s.BindInt(COLUMN_TIMES_USED, form.times_used);
-  Pickle form_data_pickle;
-  autofill::SerializeFormData(form.form_data, &form_data_pickle);
-  s.BindBlob(COLUMN_FORM_DATA,
-             form_data_pickle.data(),
-             form_data_pickle.size());
-  s.BindInt(COLUMN_USE_ADDITIONAL_AUTH, form.use_additional_authentication);
-
-  return s.Run();
+  BindAddStatement(form, encrypted_password,
+                   SerializeVector(form.other_possible_usernames),
+                   &s);
+  if (s.Run()) {
+    list.push_back(PasswordStoreChange(PasswordStoreChange::REMOVE, form));
+    list.push_back(PasswordStoreChange(PasswordStoreChange::ADD, form));
+  }
+  return list;
 }
 
 bool LoginDatabase::UpdateLogin(const PasswordForm& form, int* items_changed) {
   std::string encrypted_password;
   if (EncryptedString(form.password_value, &encrypted_password) !=
           ENCRYPTION_RESULT_SUCCESS)
     return false;
 
   // Replacement is necessary to deal with updating imported credentials. See
   // crbug.com/349138 for details.
@@ skipping 306 matching lines @@
   base::string16 str;
 
   PickleIterator iterator(p);
   while (iterator.ReadString16(&str)) {
     ret.push_back(str);
   }
   return ret;
 }
 
 }  // namespace password_manager