More SQL statement usage regularization. Back-touch some

chrome/browser/password_manager/login_database.cc

 // Copyright (c) 2011 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 "chrome/browser/password_manager/login_database.h"
 
 #include <algorithm>
 #include <limits>
 
 #include "base/file_path.h"
@@ skipping 120 matching lines @@
       return false;
     }
   }
   return true;
 }
 
 void LoginDatabase::ReportMetrics() {
   sql::Statement s(db_.GetCachedStatement(SQL_FROM_HERE,
       "SELECT signon_realm, COUNT(username_value) FROM logins "
       "GROUP BY signon_realm"));
-  if (!s) {

[Scott Hess - ex-Googler, 2012/02/13 22:04:04]

I'd leave the test, or guard the final histogram with s.Succeeded().

[Greg Billock, 2012/02/13 22:48:59]

Sounds good. I was thinking in terms of return values, but here the side effects
are UMA counts.

-    NOTREACHED() << "Statement prepare failed";
-    return;
-  }
 
   int total_accounts = 0;
   while (s.Step()) {
     int accounts_per_site = s.ColumnInt(1);
     total_accounts += accounts_per_site;
     UMA_HISTOGRAM_CUSTOM_COUNTS("PasswordManager.AccountsPerSite",
                                 accounts_per_site, 0, 32, 6);
   }
   UMA_HISTOGRAM_CUSTOM_COUNTS("PasswordManager.TotalAccounts",
                               total_accounts, 0, 32, 6);
-
-  return;
 }
 
 bool LoginDatabase::AddLogin(const PasswordForm& form) {
   // You *must* change LoginTableColumns if this query changes.
   sql::Statement s(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) "
       "VALUES "
       "(?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"));
-  if (!s) {
-    NOTREACHED() << "Statement prepare failed";
-    return false;
-  }
-
   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);
   std::string encrypted_password = EncryptedString(form.password_value);
   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);
-  if (!s.Run()) {
-    NOTREACHED();
-    return false;
-  }
-  return true;
+
+  return s.Run();
 }
 
 bool LoginDatabase::UpdateLogin(const PasswordForm& form, int* items_changed) {
   sql::Statement s(db_.GetCachedStatement(SQL_FROM_HERE,
       "UPDATE logins SET "
       "action_url = ?, "
       "password_value = ?, "
       "ssl_valid = ?, "
       "preferred = ? "
       "WHERE origin_url = ? AND "
       "username_element = ? AND "
       "username_value = ? AND "
       "password_element = ? AND "
       "signon_realm = ?"));
-  if (!s) {
-    NOTREACHED() << "Statement prepare failed";
-    return false;
-  }
-
   s.BindString(0, form.action.spec());
   std::string encrypted_password = EncryptedString(form.password_value);
   s.BindBlob(1, encrypted_password.data(),
              static_cast<int>(encrypted_password.length()));
   s.BindInt(2, form.ssl_valid);
   s.BindInt(3, form.preferred);
   s.BindString(4, form.origin.spec());
   s.BindString16(5, form.username_element);
   s.BindString16(6, form.username_value);
   s.BindString16(7, form.password_element);
   s.BindString(8, form.signon_realm);
 
-  if (!s.Run()) {
-    NOTREACHED();
+  if (!s.Run())
     return false;
-  }
-  if (items_changed) {
+
+  if (items_changed)
     *items_changed = db_.GetLastChangeCount();
-  }
+
   return true;
 }
 
 bool LoginDatabase::RemoveLogin(const PasswordForm& form) {
   // Remove a login by UNIQUE-constrained fields.
   sql::Statement s(db_.GetCachedStatement(SQL_FROM_HERE,
       "DELETE FROM logins WHERE "
       "origin_url = ? AND "
       "username_element = ? AND "
       "username_value = ? AND "
       "password_element = ? AND "
       "submit_element = ? AND "
       "signon_realm = ? "));
-  if (!s) {
-    NOTREACHED() << "Statement prepare failed";
-    return false;
-  }
-
   s.BindString(0, form.origin.spec());
   s.BindString16(1, form.username_element);
   s.BindString16(2, form.username_value);
   s.BindString16(3, form.password_element);
   s.BindString16(4, form.submit_element);
   s.BindString(5, form.signon_realm);
 
-  if (!s.Run()) {
-    NOTREACHED();
-    return false;
-  }
-  return true;
+  return s.Run();
 }
 
 bool LoginDatabase::RemoveLoginsCreatedBetween(const base::Time delete_begin,
                                                const base::Time delete_end) {
   sql::Statement s(db_.GetCachedStatement(SQL_FROM_HERE,
       "DELETE FROM logins WHERE "
       "date_created >= ? AND date_created < ?"));
-  if (!s) {
-    NOTREACHED() << "Statement prepare failed";
-    return false;
-  }
   s.BindInt64(0, delete_begin.ToTimeT());
   s.BindInt64(1, delete_end.is_null() ? std::numeric_limits<int64>::max()
                                       : delete_end.ToTimeT());
 
   return s.Run();
 }
 
 void LoginDatabase::InitPasswordFormFromStatement(PasswordForm* form,
                                                   sql::Statement& s) const {
   std::string tmp = s.ColumnString(COLUMN_ORIGIN_URL);
@@ skipping 23 matching lines @@
                               std::vector<PasswordForm*>* forms) const {
   DCHECK(forms);
   // You *must* change LoginTableColumns if this query changes.
   sql::Statement s(db_.GetCachedStatement(SQL_FROM_HERE,
       "SELECT 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 FROM logins "
       "WHERE signon_realm == ? "));
-  if (!s) {
-    NOTREACHED() << "Statement prepare failed";
-    return false;
-  }
-
   s.BindString(0, form.signon_realm);
 
   while (s.Step()) {
     PasswordForm* new_form = new PasswordForm();
     InitPasswordFormFromStatement(new_form, s);
 
     forms->push_back(new_form);
   }
   return s.Succeeded();
 }
 
 bool LoginDatabase::GetLoginsCreatedBetween(
     const base::Time begin,
     const base::Time end,
     std::vector<webkit::forms::PasswordForm*>* forms) const {
   DCHECK(forms);
   sql::Statement s(db_.GetCachedStatement(SQL_FROM_HERE,
       "SELECT 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 FROM logins "
       "WHERE date_created >= ? AND date_created < ?"
       "ORDER BY origin_url"));
-
-  if (!s) {
-    NOTREACHED() << "Statement prepare failed";
-    return false;
-  }
   s.BindInt64(0, begin.ToTimeT());
   s.BindInt64(1, end.is_null() ? std::numeric_limits<int64>::max()
                                : end.ToTimeT());
 
   while (s.Step()) {
     PasswordForm* new_form = new PasswordForm();
     InitPasswordFormFromStatement(new_form, s);
 
     forms->push_back(new_form);
   }
@@ skipping 15 matching lines @@
   DCHECK(forms);
   // You *must* change LoginTableColumns if this query changes.
   sql::Statement s(db_.GetCachedStatement(SQL_FROM_HERE,
       "SELECT 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 FROM logins "
       "WHERE blacklisted_by_user == ? "
       "ORDER BY origin_url"));
-
-  if (!s) {
-    NOTREACHED() << "Statement prepare failed";
-    return false;
-  }
   s.BindInt(0, blacklisted ? 1 : 0);
 
   while (s.Step()) {
     PasswordForm* new_form = new PasswordForm();
     InitPasswordFormFromStatement(new_form, s);
 
     forms->push_back(new_form);
   }
   return s.Succeeded();
 }
 
 bool LoginDatabase::DeleteAndRecreateDatabaseFile() {
   DCHECK(db_.is_open());
   meta_table_.Reset();
   db_.Close();
   file_util::Delete(db_path_, false);
   return Init(db_path_);
 }