diff options
author | gbillock@chromium.org <gbillock@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-12-22 19:25:51 +0000 |
---|---|---|
committer | gbillock@chromium.org <gbillock@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-12-22 19:25:51 +0000 |
commit | bed29d94d90663ab161e5832677dae9dd4d4be13 (patch) | |
tree | 0527ff3b39d74e69ac5c030fbb6ade0fad803e20 | |
parent | 443853c674eeabd922bfcda2caf0411f47c9a93c (diff) | |
download | chromium_src-bed29d94d90663ab161e5832677dae9dd4d4be13.zip chromium_src-bed29d94d90663ab161e5832677dae9dd4d4be13.tar.gz chromium_src-bed29d94d90663ab161e5832677dae9dd4d4be13.tar.bz2 |
Update webdata files to take advantage of DLOG(FATAL) in
sql/Statement and Connection.
R=shess@chromium.org
BUG=
TEST=webdata/*Test*.*
Review URL: http://codereview.chromium.org/8966003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@115574 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/browser/webdata/autofill_table.cc | 424 | ||||
-rw-r--r-- | chrome/browser/webdata/autofill_table.h | 5 | ||||
-rw-r--r-- | chrome/browser/webdata/autofill_table_unittest.cc | 46 | ||||
-rw-r--r-- | chrome/browser/webdata/keyword_table.cc | 24 | ||||
-rw-r--r-- | chrome/browser/webdata/logins_table.cc | 48 | ||||
-rw-r--r-- | chrome/browser/webdata/logins_table_win.cc | 28 | ||||
-rw-r--r-- | chrome/browser/webdata/token_service_table.cc | 27 | ||||
-rw-r--r-- | chrome/browser/webdata/web_apps_table.cc | 33 | ||||
-rw-r--r-- | chrome/browser/webdata/web_apps_table.h | 4 | ||||
-rw-r--r-- | chrome/browser/webdata/web_database_migration_unittest.cc | 5 | ||||
-rw-r--r-- | chrome/browser/webdata/web_intents_table.cc | 9 | ||||
-rw-r--r-- | sql/statement.cc | 5 | ||||
-rw-r--r-- | sql/statement.h | 2 |
13 files changed, 169 insertions, 491 deletions
diff --git a/chrome/browser/webdata/autofill_table.cc b/chrome/browser/webdata/autofill_table.cc index 4e9ea0b..60f3431 100644 --- a/chrome/browser/webdata/autofill_table.cc +++ b/chrome/browser/webdata/autofill_table.cc @@ -139,12 +139,11 @@ bool AddAutofillProfileNamesToProfile(sql::Connection* db, "SELECT guid, first_name, middle_name, last_name " "FROM autofill_profile_names " "WHERE guid=?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } s.BindString(0, profile->guid()); + if (!s.is_valid()) + return false; + std::vector<string16> first_names; std::vector<string16> middle_names; std::vector<string16> last_names; @@ -154,6 +153,9 @@ bool AddAutofillProfileNamesToProfile(sql::Connection* db, middle_names.push_back(s.ColumnString16(2)); last_names.push_back(s.ColumnString16(3)); } + if (!s.Succeeded()) + return false; + profile->SetMultiInfo(NAME_FIRST, first_names); profile->SetMultiInfo(NAME_MIDDLE, middle_names); profile->SetMultiInfo(NAME_LAST, last_names); @@ -166,17 +168,19 @@ bool AddAutofillProfileEmailsToProfile(sql::Connection* db, "SELECT guid, email " "FROM autofill_profile_emails " "WHERE guid=?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } s.BindString(0, profile->guid()); + if (!s.is_valid()) + return false; + std::vector<string16> emails; while (s.Step()) { DCHECK_EQ(profile->guid(), s.ColumnString(0)); emails.push_back(s.ColumnString16(1)); } + if (!s.Succeeded()) + return false; + profile->SetMultiInfo(EMAIL_ADDRESS, emails); return true; } @@ -187,19 +191,22 @@ bool AddAutofillProfilePhonesToProfile(sql::Connection* db, "SELECT guid, type, number " "FROM autofill_profile_phones " "WHERE guid=? AND type=?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindString(0, profile->guid()); + // Value used to be either [(0, phone), (1, fax)] but fax has been removed. + s.BindString(0, profile->guid()); s.BindInt(1, 0); + if (!s.is_valid()) + return false; + std::vector<string16> numbers; while (s.Step()) { DCHECK_EQ(profile->guid(), s.ColumnString(0)); numbers.push_back(s.ColumnString16(2)); } + if (!s.Succeeded()) + return false; + profile->SetMultiInfo(PHONE_HOME_WHOLE_NUMBER, numbers); return true; } @@ -221,19 +228,13 @@ bool AddAutofillProfileNames(const AutofillProfile& profile, "INSERT INTO autofill_profile_names" " (guid, first_name, middle_name, last_name) " "VALUES (?,?,?,?)")); - if (!s) { - NOTREACHED(); - return false; - } s.BindString(0, profile.guid()); s.BindString16(1, first_names[i]); s.BindString16(2, middle_names[i]); s.BindString16(3, last_names[i]); - if (!s.Run()) { - NOTREACHED(); + if (!s.Run()) return false; - } } return true; } @@ -249,18 +250,13 @@ bool AddAutofillProfileEmails(const AutofillProfile& profile, "INSERT INTO autofill_profile_emails" " (guid, email) " "VALUES (?,?)")); - if (!s) { - NOTREACHED(); - return false; - } s.BindString(0, profile.guid()); s.BindString16(1, emails[i]); - if (!s.Run()) { - NOTREACHED(); + if (!s.Run()) return false; - } } + return true; } @@ -275,19 +271,13 @@ bool AddAutofillProfilePhones(const AutofillProfile& profile, "INSERT INTO autofill_profile_phones" " (guid, type, number) " "VALUES (?,?,?)")); - if (!s) { - NOTREACHED(); - return false; - } s.BindString(0, profile.guid()); // Value used to be either [(0, phone), (1, fax)] but fax has been removed. s.BindInt(1, 0); s.BindString16(2, numbers[i]); - if (!s.Run()) { - NOTREACHED(); + if (!s.Run()) return false; - } } return true; @@ -310,34 +300,22 @@ bool AddAutofillProfilePieces(const AutofillProfile& profile, bool RemoveAutofillProfilePieces(const std::string& guid, sql::Connection* db) { sql::Statement s1(db->GetUniqueStatement( "DELETE FROM autofill_profile_names WHERE guid = ?")); - if (!s1) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s1.BindString(0, guid); + if (!s1.Run()) return false; sql::Statement s2(db->GetUniqueStatement( "DELETE FROM autofill_profile_emails WHERE guid = ?")); - if (!s2) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s2.BindString(0, guid); + if (!s2.Run()) return false; sql::Statement s3(db->GetUniqueStatement( "DELETE FROM autofill_profile_phones WHERE guid = ?")); - if (!s3) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s3.BindString(0, guid); + return s3.Run(); } @@ -387,11 +365,6 @@ bool AutofillTable::GetFormValuesForElementName(const string16& name, "WHERE name = ? " "ORDER BY count DESC " "LIMIT ?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindString16(0, name); s.BindInt(1, limit); } else { @@ -406,11 +379,6 @@ bool AutofillTable::GetFormValuesForElementName(const string16& name, "value_lower < ? " "ORDER BY count DESC " "LIMIT ?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindString16(0, name); s.BindString16(1, prefix_lower); s.BindString16(2, next_prefix); @@ -434,10 +402,6 @@ bool AutofillTable::RemoveFormElementsAddedBetween( "SELECT DISTINCT a.pair_id, a.name, a.value " "FROM autofill_dates ad JOIN autofill a ON ad.pair_id = a.pair_id " "WHERE ad.date_created >= ? AND ad.date_created < ?")); - if (!s) { - NOTREACHED() << "Statement 1 prepare failed"; - return false; - } s.BindInt64(0, delete_begin.ToTimeT()); s.BindInt64(1, delete_end.is_null() ? @@ -450,11 +414,8 @@ bool AutofillTable::RemoveFormElementsAddedBetween( s.ColumnString16(1), s.ColumnString16(2))); } - - if (!s.Succeeded()) { - NOTREACHED(); + if (!s.Succeeded()) return false; - } for (AutofillElementList::iterator itr = elements.begin(); itr != elements.end(); itr++) { @@ -482,10 +443,6 @@ bool AutofillTable::RemoveFormElementForTimeRange(int64 pair_id, sql::Statement s(db_->GetUniqueStatement( "DELETE FROM autofill_dates WHERE pair_id = ? AND " "date_created >= ? AND date_created < ?")); - if (!s) { - NOTREACHED() << "Statement 1 prepare failed"; - return false; - } s.BindInt64(0, pair_id); s.BindInt64(1, delete_begin.is_null() ? 0 : delete_begin.ToTimeT()); s.BindInt64(2, delete_end.is_null() ? std::numeric_limits<int64>::max() : @@ -523,17 +480,18 @@ bool AutofillTable::GetIDAndCountOfFormElement( const FormField& element, int64* pair_id, int* count) { + DCHECK(pair_id); + DCHECK(count); + sql::Statement s(db_->GetUniqueStatement( "SELECT pair_id, count FROM autofill " "WHERE name = ? AND value = ?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindString16(0, element.name); s.BindString16(1, element.value); + if (!s.is_valid()) + return false; + *pair_id = 0; *count = 0; @@ -546,13 +504,9 @@ bool AutofillTable::GetIDAndCountOfFormElement( } bool AutofillTable::GetCountOfFormElement(int64 pair_id, int* count) { + DCHECK(count); sql::Statement s(db_->GetUniqueStatement( "SELECT count FROM autofill WHERE pair_id = ?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindInt64(0, pair_id); if (s.Step()) { @@ -565,38 +519,23 @@ bool AutofillTable::GetCountOfFormElement(int64 pair_id, int* count) { bool AutofillTable::SetCountOfFormElement(int64 pair_id, int count) { sql::Statement s(db_->GetUniqueStatement( "UPDATE autofill SET count = ? WHERE pair_id = ?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindInt(0, count); s.BindInt64(1, pair_id); - if (!s.Run()) { - NOTREACHED(); - return false; - } - return true; + return s.Run(); } bool AutofillTable::InsertFormElement(const FormField& element, int64* pair_id) { + DCHECK(pair_id); sql::Statement s(db_->GetUniqueStatement( "INSERT INTO autofill (name, value, value_lower) VALUES (?,?,?)")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindString16(0, element.name); s.BindString16(1, element.value); s.BindString16(2, base::i18n::ToLower(element.value)); - if (!s.Run()) { - NOTREACHED(); + if (!s.Run()) return false; - } *pair_id = db_->GetLastInsertRowId(); return true; @@ -607,20 +546,10 @@ bool AutofillTable::InsertPairIDAndDate(int64 pair_id, sql::Statement s(db_->GetUniqueStatement( "INSERT INTO autofill_dates " "(pair_id, date_created) VALUES (?, ?)")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindInt64(0, pair_id); s.BindInt64(1, date_created.ToTimeT()); - if (!s.Run()) { - NOTREACHED(); - return false; - } - - return true; + return s.Run(); } bool AutofillTable::AddFormFieldValuesTime( @@ -649,14 +578,14 @@ bool AutofillTable::AddFormFieldValuesTime( bool AutofillTable::ClearAutofillEmptyValueElements() { sql::Statement s(db_->GetUniqueStatement( "SELECT pair_id FROM autofill WHERE TRIM(value)= \"\"")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; + if (!s.is_valid()) return false; - } std::set<int64> ids; while (s.Step()) ids.insert(s.ColumnInt64(0)); + if (!s.Succeeded()) + return false; bool success = true; for (std::set<int64>::const_iterator iter = ids.begin(); iter != ids.end(); @@ -674,11 +603,6 @@ bool AutofillTable::GetAllAutofillEntries(std::vector<AutofillEntry>* entries) { "SELECT name, value, date_created FROM autofill a JOIN " "autofill_dates ad ON a.pair_id=ad.pair_id")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - bool first_entry = true; AutofillKey* current_key_ptr = NULL; std::vector<Time>* timestamps_ptr = NULL; @@ -712,6 +636,10 @@ bool AutofillTable::GetAllAutofillEntries(std::vector<AutofillEntry>* entries) { timestamps_ptr->push_back(time); } } + + if (!s.Succeeded()) + return false; + // If there is at least one result returned, first_entry will be false. // For this case we need to do a final cleanup step. if (!first_entry) { @@ -732,14 +660,9 @@ bool AutofillTable::GetAutofillTimestamps(const string16& name, "SELECT date_created FROM autofill a JOIN " "autofill_dates ad ON a.pair_id=ad.pair_id " "WHERE a.name = ? AND a.value = ?")); - - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindString16(0, name); s.BindString16(1, value); + while (s.Step()) timestamps->push_back(Time::FromTimeT(s.ColumnInt64(0))); @@ -756,13 +679,12 @@ bool AutofillTable::UpdateAutofillEntries( std::string sql = "SELECT pair_id FROM autofill " "WHERE name = ? AND value = ?"; sql::Statement s(db_->GetUniqueStatement(sql.c_str())); - if (!s.is_valid()) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindString16(0, entries[i].key().name()); s.BindString16(1, entries[i].key().value()); + + if (!s.is_valid()) + return false; + if (s.Step()) { if (!RemoveFormElementForID(s.ColumnInt64(0))) return false; @@ -782,20 +704,13 @@ bool AutofillTable::InsertAutofillEntry(const AutofillEntry& entry) { std::string sql = "INSERT INTO autofill (name, value, value_lower, count) " "VALUES (?, ?, ?, ?)"; sql::Statement s(db_->GetUniqueStatement(sql.c_str())); - if (!s.is_valid()) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindString16(0, entry.key().name()); s.BindString16(1, entry.key().value()); s.BindString16(2, base::i18n::ToLower(entry.key().value())); s.BindInt(3, entry.timestamps().size()); - if (!s.Run()) { - NOTREACHED(); + if (!s.Run()) return false; - } int64 pair_id = db_->GetLastInsertRowId(); for (size_t i = 0; i < entry.timestamps().size(); i++) { @@ -838,10 +753,6 @@ bool AutofillTable::RemoveFormElement(const string16& name, // Find the id for that pair. sql::Statement s(db_->GetUniqueStatement( "SELECT pair_id FROM autofill WHERE name = ? AND value= ?")); - if (!s) { - NOTREACHED() << "Statement 1 prepare failed"; - return false; - } s.BindString16(0, name); s.BindString16(1, value); @@ -859,19 +770,9 @@ bool AutofillTable::AddAutofillProfile(const AutofillProfile& profile) { "(guid, company_name, address_line_1, address_line_2, city, state," " zipcode, country, country_code, date_modified)" "VALUES (?,?,?,?,?,?,?,?,?,?)")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - BindAutofillProfileToStatement(profile, &s); - if (!s.Run()) { - NOTREACHED(); - return false; - } - - if (!s.Succeeded()) + if (!s.Run()) return false; return AddAutofillProfilePieces(profile, db_); @@ -886,16 +787,9 @@ bool AutofillTable::GetAutofillProfile(const std::string& guid, " zipcode, country, country_code, date_modified " "FROM autofill_profiles " "WHERE guid=?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindString(0, guid); - if (!s.Step()) - return false; - if (!s.Succeeded()) + if (!s.Step()) return false; scoped_ptr<AutofillProfile> p(AutofillProfileFromStatement(s)); @@ -921,10 +815,6 @@ bool AutofillTable::GetAutofillProfiles( sql::Statement s(db_->GetUniqueStatement( "SELECT guid " "FROM autofill_profiles")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } while (s.Step()) { std::string guid = s.ColumnString(0); @@ -995,13 +885,9 @@ bool AutofillTable::UpdateAutofillProfileMulti(const AutofillProfile& profile) { " city=?, state=?, zipcode=?, country=?, country_code=?, " " date_modified=? " "WHERE guid=?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - BindAutofillProfileToStatement(profile, &s); s.BindString(10, profile.guid()); + bool result = s.Run(); DCHECK_GT(db_->GetLastChangeCount(), 0); if (!result) @@ -1020,27 +906,17 @@ bool AutofillTable::RemoveAutofillProfile(const std::string& guid) { if (IsAutofillGUIDInTrash(guid)) { sql::Statement s_trash(db_->GetUniqueStatement( "DELETE FROM autofill_profiles_trash WHERE guid = ?")); - if (!s_trash) { - NOTREACHED() << "Statement prepare failed"; - return false; - } s_trash.BindString(0, guid); - if (!s_trash.Run()) { - NOTREACHED() << "Expected item in trash."; - return false; - } - return true; + bool success = s_trash.Run(); + DCHECK_GT(db_->GetLastChangeCount(), 0) << "Expected item in trash"; + return success; } sql::Statement s(db_->GetUniqueStatement( "DELETE FROM autofill_profiles WHERE guid = ?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindString(0, guid); + if (!s.Run()) return false; @@ -1050,45 +926,26 @@ bool AutofillTable::RemoveAutofillProfile(const std::string& guid) { bool AutofillTable::ClearAutofillProfiles() { sql::Statement s1(db_->GetUniqueStatement( "DELETE FROM autofill_profiles")); - if (!s1) { - NOTREACHED() << "Statement prepare failed"; - return false; - } if (!s1.Run()) return false; sql::Statement s2(db_->GetUniqueStatement( "DELETE FROM autofill_profile_names")); - if (!s2) { - NOTREACHED() << "Statement prepare failed"; - return false; - } if (!s2.Run()) return false; sql::Statement s3(db_->GetUniqueStatement( "DELETE FROM autofill_profile_emails")); - if (!s3) { - NOTREACHED() << "Statement prepare failed"; - return false; - } if (!s3.Run()) return false; sql::Statement s4(db_->GetUniqueStatement( "DELETE FROM autofill_profile_phones")); - if (!s4) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - - if (!s4.Run()) - return false; - return true; + return s4.Run(); } bool AutofillTable::AddCreditCard(const CreditCard& credit_card) { @@ -1097,20 +954,13 @@ bool AutofillTable::AddCreditCard(const CreditCard& credit_card) { "(guid, name_on_card, expiration_month, expiration_year, " "card_number_encrypted, date_modified)" "VALUES (?,?,?,?,?,?)")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - BindCreditCardToStatement(credit_card, &s); - if (!s.Run()) { - NOTREACHED(); + if (!s.Run()) return false; - } DCHECK_GT(db_->GetLastChangeCount(), 0); - return s.Succeeded(); + return true; } bool AutofillTable::GetCreditCard(const std::string& guid, @@ -1121,18 +971,13 @@ bool AutofillTable::GetCreditCard(const std::string& guid, "card_number_encrypted, date_modified " "FROM credit_cards " "WHERE guid = ?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindString(0, guid); + if (!s.Step()) return false; *credit_card = CreditCardFromStatement(s); - - return s.Succeeded(); + return true; } bool AutofillTable::GetCreditCards( @@ -1143,10 +988,6 @@ bool AutofillTable::GetCreditCards( sql::Statement s(db_->GetUniqueStatement( "SELECT guid " "FROM credit_cards")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } while (s.Step()) { std::string guid = s.ColumnString(0); @@ -1176,13 +1017,9 @@ bool AutofillTable::UpdateCreditCard(const CreditCard& credit_card) { "SET guid=?, name_on_card=?, expiration_month=?, " " expiration_year=?, card_number_encrypted=?, date_modified=? " "WHERE guid=?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - BindCreditCardToStatement(credit_card, &s); s.BindString(6, credit_card.guid()); + bool result = s.Run(); DCHECK_GT(db_->GetLastChangeCount(), 0); return result; @@ -1192,12 +1029,8 @@ bool AutofillTable::RemoveCreditCard(const std::string& guid) { DCHECK(guid::IsValidGUID(guid)); sql::Statement s(db_->GetUniqueStatement( "DELETE FROM credit_cards WHERE guid = ?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindString(0, guid); + return s.Run(); } @@ -1217,73 +1050,50 @@ bool AutofillTable::RemoveAutofillProfilesAndCreditCardsModifiedBetween( sql::Statement s_profiles_get(db_->GetUniqueStatement( "SELECT guid FROM autofill_profiles " "WHERE date_modified >= ? AND date_modified < ?")); - if (!s_profiles_get) { - NOTREACHED() << "Autofill profiles statement prepare failed"; - return false; - } - s_profiles_get.BindInt64(0, delete_begin_t); s_profiles_get.BindInt64(1, delete_end_t); + profile_guids->clear(); while (s_profiles_get.Step()) { std::string guid = s_profiles_get.ColumnString(0); profile_guids->push_back(guid); } + if (!s_profiles_get.Succeeded()) + return false; // Remove Autofill profiles in the time range. sql::Statement s_profiles(db_->GetUniqueStatement( "DELETE FROM autofill_profiles " "WHERE date_modified >= ? AND date_modified < ?")); - if (!s_profiles) { - NOTREACHED() << "Autofill profiles statement prepare failed"; - return false; - } - s_profiles.BindInt64(0, delete_begin_t); s_profiles.BindInt64(1, delete_end_t); - s_profiles.Run(); - if (!s_profiles.Succeeded()) { - NOTREACHED(); + if (!s_profiles.Run()) return false; - } // Remember Autofill credit cards in the time range. sql::Statement s_credit_cards_get(db_->GetUniqueStatement( "SELECT guid FROM credit_cards " "WHERE date_modified >= ? AND date_modified < ?")); - if (!s_credit_cards_get) { - NOTREACHED() << "Autofill profiles statement prepare failed"; - return false; - } - s_credit_cards_get.BindInt64(0, delete_begin_t); s_credit_cards_get.BindInt64(1, delete_end_t); + credit_card_guids->clear(); while (s_credit_cards_get.Step()) { std::string guid = s_credit_cards_get.ColumnString(0); credit_card_guids->push_back(guid); } + if (!s_credit_cards_get.Succeeded()) + return false; // Remove Autofill credit cards in the time range. sql::Statement s_credit_cards(db_->GetUniqueStatement( "DELETE FROM credit_cards " "WHERE date_modified >= ? AND date_modified < ?")); - if (!s_credit_cards) { - NOTREACHED() << "Autofill credit cards statement prepare failed"; - return false; - } - s_credit_cards.BindInt64(0, delete_begin_t); s_credit_cards.BindInt64(1, delete_end_t); - s_credit_cards.Run(); - - if (!s_credit_cards.Succeeded()) { - NOTREACHED(); - return false; - } - return true; + return s_credit_cards.Run(); } bool AutofillTable::GetAutofillProfilesInTrash( @@ -1293,10 +1103,6 @@ bool AutofillTable::GetAutofillProfilesInTrash( sql::Statement s(db_->GetUniqueStatement( "SELECT guid " "FROM autofill_profiles_trash")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } while (s.Step()) { std::string guid = s.ColumnString(0); @@ -1309,10 +1115,6 @@ bool AutofillTable::GetAutofillProfilesInTrash( bool AutofillTable::EmptyAutofillProfilesTrash() { sql::Statement s(db_->GetUniqueStatement( "DELETE FROM autofill_profiles_trash")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } return s.Run(); } @@ -1321,11 +1123,8 @@ bool AutofillTable::EmptyAutofillProfilesTrash() { bool AutofillTable::RemoveFormElementForID(int64 pair_id) { sql::Statement s(db_->GetUniqueStatement( "DELETE FROM autofill WHERE pair_id = ?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } s.BindInt64(0, pair_id); + if (s.Run()) return RemoveFormElementForTimeRange(pair_id, Time(), Time(), NULL); @@ -1337,27 +1136,15 @@ bool AutofillTable::AddAutofillGUIDToTrash(const std::string& guid) { "INSERT INTO autofill_profiles_trash" " (guid) " "VALUES (?)")); - if (!s) { - NOTREACHED(); - return sql::INIT_FAILURE; - } - s.BindString(0, guid); - if (!s.Run()) { - NOTREACHED(); - return false; - } - return true; + + return s.Run(); } bool AutofillTable::IsAutofillProfilesTrashEmpty() { sql::Statement s(db_->GetUniqueStatement( "SELECT guid " "FROM autofill_profiles_trash")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } return !s.Step(); } @@ -1367,12 +1154,8 @@ bool AutofillTable::IsAutofillGUIDInTrash(const std::string& guid) { "SELECT guid " "FROM autofill_profiles_trash " "WHERE guid = ?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - s.BindString(0, guid); + return s.Step(); } @@ -1635,26 +1418,26 @@ bool AutofillTable::MigrateToVersion27UpdateLegacyCreditCards() { "FROM autofill_profiles, credit_cards " "WHERE credit_cards.billing_address = autofill_profiles.label"; sql::Statement s(db_->GetUniqueStatement(stmt.c_str())); - if (!s) - return false; std::map<int, int> cc_billing_map; while (s.Step()) cc_billing_map[s.ColumnInt(0)] = s.ColumnInt(1); + if (!s.Succeeded()) + return false; // Windows already stores the IDs as strings in |billing_address|. Try // to convert those. if (cc_billing_map.empty()) { std::string stmt = "SELECT unique_id,billing_address FROM credit_cards"; sql::Statement s(db_->GetUniqueStatement(stmt.c_str())); - if (!s) - return false; while (s.Step()) { int id = 0; if (base::StringToInt(s.ColumnString(1), &id)) cc_billing_map[s.ColumnInt(0)] = id; } + if (!s.Succeeded()) + return false; } if (!db_->Execute("CREATE TABLE credit_cards_temp ( " @@ -1693,9 +1476,6 @@ bool AutofillTable::MigrateToVersion27UpdateLegacyCreditCards() { sql::Statement s(db_->GetCachedStatement( SQL_FROM_HERE, "UPDATE credit_cards SET billing_address=? WHERE unique_id=?")); - if (!s) - return false; - s.BindInt(0, (*iter).second); s.BindInt(1, (*iter).first); @@ -1716,9 +1496,6 @@ bool AutofillTable::MigrateToVersion30AddDateModifed() { sql::Statement s(db_->GetUniqueStatement( "UPDATE autofill_profiles SET date_modified=?")); - if (!s) - return false; - s.BindInt64(0, Time::Now().ToTimeT()); if (!s.Run()) @@ -1734,9 +1511,6 @@ bool AutofillTable::MigrateToVersion30AddDateModifed() { sql::Statement s(db_->GetUniqueStatement( "UPDATE credit_cards SET date_modified=?")); - if (!s) - return false; - s.BindInt64(0, Time::Now().ToTimeT()); if (!s.Run()) @@ -1760,21 +1534,19 @@ bool AutofillTable::MigrateToVersion31AddGUIDToCreditCardsAndProfiles() { sql::Statement s(db_->GetUniqueStatement("SELECT unique_id " "FROM autofill_profiles")); - if (!s) - return false; while (s.Step()) { sql::Statement update_s( db_->GetUniqueStatement("UPDATE autofill_profiles " "SET guid=? WHERE unique_id=?")); - if (!update_s) - return false; update_s.BindString(0, guid::GenerateGUID()); update_s.BindInt(1, s.ColumnInt(0)); if (!update_s.Run()) return false; } + if (!s.Succeeded()) + return false; } // Note that we need to check for the guid column's existence due to the @@ -1790,21 +1562,19 @@ bool AutofillTable::MigrateToVersion31AddGUIDToCreditCardsAndProfiles() { sql::Statement s(db_->GetUniqueStatement("SELECT unique_id " "FROM credit_cards")); - if (!s) - return false; while (s.Step()) { sql::Statement update_s( db_->GetUniqueStatement("UPDATE credit_cards " "set guid=? WHERE unique_id=?")); - if (!update_s) - return false; update_s.BindString(0, guid::GenerateGUID()); update_s.BindInt(1, s.ColumnInt(0)); if (!update_s.Run()) return false; } + if (!s.Succeeded()) + return false; } return true; @@ -1906,6 +1676,7 @@ bool AutofillTable::MigrateToVersion33ProfilesBasedOnFirstName() { "company_name, address_line_1, address_line_2, city, state, " "zipcode, country, phone, date_modified " "FROM autofill_profiles")); + while (s.Step()) { AutofillProfile profile; profile.set_guid(s.ColumnString(0)); @@ -1930,9 +1701,6 @@ bool AutofillTable::MigrateToVersion33ProfilesBasedOnFirstName() { "(guid, company_name, address_line_1, address_line_2, city," " state, zipcode, country, date_modified)" "VALUES (?,?,?,?,?,?,?,?,?)")); - if (!s) - return false; - s_insert.BindString(0, profile.guid()); s_insert.BindString16(1, profile.GetInfo(COMPANY_NAME)); s_insert.BindString16(2, profile.GetInfo(ADDRESS_HOME_LINE1)); @@ -1949,7 +1717,9 @@ bool AutofillTable::MigrateToVersion33ProfilesBasedOnFirstName() { // Add the other bits: names, emails, and phone numbers. if (!AddAutofillProfilePieces(profile, db_)) return false; - } + } // endwhile + if (!s.Succeeded()) + return false; if (!db_->Execute("DROP TABLE autofill_profiles")) return false; @@ -2005,15 +1775,10 @@ bool AutofillTable::MigrateToVersion34ProfilesBasedOnCountryCode() { sql::Statement s(db_->GetUniqueStatement("SELECT guid, country " "FROM autofill_profiles")); - if (!s) - return false; - while (s.Step()) { sql::Statement update_s( db_->GetUniqueStatement("UPDATE autofill_profiles " "SET country_code=? WHERE guid=?")); - if (!update_s) - return false; string16 country = s.ColumnString16(1); std::string app_locale = AutofillCountry::ApplicationLocale(); @@ -2024,6 +1789,8 @@ bool AutofillTable::MigrateToVersion34ProfilesBasedOnCountryCode() { if (!update_s.Run()) return false; } + if (!s.Succeeded()) + return false; } return true; @@ -2044,8 +1811,6 @@ bool AutofillTable::MigrateToVersion35GreatBritainCountryCodes() { bool AutofillTable::MigrateToVersion37MergeAndCullOlderProfiles() { sql::Statement s(db_->GetUniqueStatement( "SELECT guid, date_modified FROM autofill_profiles")); - if (!s) - return false; // Accumulate the good profiles. std::vector<AutofillProfile> accumulated_profiles; @@ -2084,7 +1849,9 @@ bool AutofillTable::MigrateToVersion37MergeAndCullOlderProfiles() { // An invalid profile, so trash it. AddAutofillGUIDToTrash(p->guid()); } - } + } // endwhile + if (!s.Succeeded()) + return false; // Drop the current profiles. if (!ClearAutofillProfiles()) @@ -2109,6 +1876,7 @@ bool AutofillTable::MigrateToVersion37MergeAndCullOlderProfiles() { "WHERE guid=?")); s_date.BindInt64(0, date_item->second); s_date.BindString(1, iter->guid()); + if (!s_date.Run()) return false; } diff --git a/chrome/browser/webdata/autofill_table.h b/chrome/browser/webdata/autofill_table.h index b2212c3..4430f17 100644 --- a/chrome/browser/webdata/autofill_table.h +++ b/chrome/browser/webdata/autofill_table.h @@ -246,7 +246,10 @@ class AutofillTable : public WebDatabaseTable { // Removes rows from autofill_profiles and credit_cards if they were created // on or after |delete_begin| and strictly before |delete_end|. Returns lists - // of deleted guids in |profile_guids| and |credit_card_guids|. + // of deleted guids in |profile_guids| and |credit_card_guids|. Return value + // is true if all rows were successfully removed. Returns false on database + // error. In that case, the output vector state is undefined, and may be + // partially filled. bool RemoveAutofillProfilesAndCreditCardsModifiedBetween( const base::Time& delete_begin, const base::Time& delete_end, diff --git a/chrome/browser/webdata/autofill_table_unittest.cc b/chrome/browser/webdata/autofill_table_unittest.cc index 0dccfb7..52d457a 100644 --- a/chrome/browser/webdata/autofill_table_unittest.cc +++ b/chrome/browser/webdata/autofill_table_unittest.cc @@ -573,7 +573,7 @@ TEST_F(AutofillTableTest, AutofillProfile) { "SELECT date_modified " "FROM autofill_profiles WHERE guid=?")); s_home.BindString(0, home_profile.guid()); - ASSERT_TRUE(s_home); + ASSERT_TRUE(s_home.is_valid()); ASSERT_TRUE(s_home.Step()); EXPECT_GE(s_home.ColumnInt64(0), pre_creation_time.ToTimeT()); EXPECT_LE(s_home.ColumnInt64(0), post_creation_time.ToTimeT()); @@ -598,7 +598,7 @@ TEST_F(AutofillTableTest, AutofillProfile) { sql::Statement s_billing(db.GetSQLConnection()->GetUniqueStatement( "SELECT date_modified FROM autofill_profiles WHERE guid=?")); s_billing.BindString(0, billing_profile.guid()); - ASSERT_TRUE(s_billing); + ASSERT_TRUE(s_billing.is_valid()); ASSERT_TRUE(s_billing.Step()); EXPECT_GE(s_billing.ColumnInt64(0), pre_creation_time.ToTimeT()); EXPECT_LE(s_billing.ColumnInt64(0), post_creation_time.ToTimeT()); @@ -617,7 +617,7 @@ TEST_F(AutofillTableTest, AutofillProfile) { sql::Statement s_billing_updated(db.GetSQLConnection()->GetUniqueStatement( "SELECT date_modified FROM autofill_profiles WHERE guid=?")); s_billing_updated.BindString(0, billing_profile.guid()); - ASSERT_TRUE(s_billing_updated); + ASSERT_TRUE(s_billing_updated.is_valid()); ASSERT_TRUE(s_billing_updated.Step()); EXPECT_GE(s_billing_updated.ColumnInt64(0), pre_modification_time.ToTimeT()); @@ -649,7 +649,7 @@ TEST_F(AutofillTableTest, AutofillProfile) { sql::Statement s_billing_updated_2(db.GetSQLConnection()->GetUniqueStatement( "SELECT date_modified FROM autofill_profiles WHERE guid=?")); s_billing_updated_2.BindString(0, billing_profile.guid()); - ASSERT_TRUE(s_billing_updated_2); + ASSERT_TRUE(s_billing_updated_2.is_valid()); ASSERT_TRUE(s_billing_updated_2.Step()); EXPECT_GE(s_billing_updated_2.ColumnInt64(0), pre_modification_time_2.ToTimeT()); @@ -950,7 +950,7 @@ TEST_F(AutofillTableTest, CreditCard) { "card_number_encrypted, date_modified " "FROM credit_cards WHERE guid=?")); s_work.BindString(0, work_creditcard.guid()); - ASSERT_TRUE(s_work); + ASSERT_TRUE(s_work.is_valid()); ASSERT_TRUE(s_work.Step()); EXPECT_GE(s_work.ColumnInt64(5), pre_creation_time.ToTimeT()); EXPECT_LE(s_work.ColumnInt64(5), post_creation_time.ToTimeT()); @@ -976,7 +976,7 @@ TEST_F(AutofillTableTest, CreditCard) { "card_number_encrypted, date_modified " "FROM credit_cards WHERE guid=?")); s_target.BindString(0, target_creditcard.guid()); - ASSERT_TRUE(s_target); + ASSERT_TRUE(s_target.is_valid()); ASSERT_TRUE(s_target.Step()); EXPECT_GE(s_target.ColumnInt64(5), pre_creation_time.ToTimeT()); EXPECT_LE(s_target.ColumnInt64(5), post_creation_time.ToTimeT()); @@ -996,7 +996,7 @@ TEST_F(AutofillTableTest, CreditCard) { "card_number_encrypted, date_modified " "FROM credit_cards WHERE guid=?")); s_target_updated.BindString(0, target_creditcard.guid()); - ASSERT_TRUE(s_target_updated); + ASSERT_TRUE(s_target_updated.is_valid()); ASSERT_TRUE(s_target_updated.Step()); EXPECT_GE(s_target_updated.ColumnInt64(5), pre_modification_time.ToTimeT()); EXPECT_LE(s_target_updated.ColumnInt64(5), post_modification_time.ToTimeT()); @@ -1034,7 +1034,7 @@ TEST_F(AutofillTableTest, UpdateAutofillProfile) { const time_t mock_creation_date = Time::Now().ToTimeT() - 13; sql::Statement s_mock_creation_date(db.GetSQLConnection()->GetUniqueStatement( "UPDATE autofill_profiles SET date_modified = ?")); - ASSERT_TRUE(s_mock_creation_date); + ASSERT_TRUE(s_mock_creation_date.is_valid()); s_mock_creation_date.BindInt64(0, mock_creation_date); ASSERT_TRUE(s_mock_creation_date.Run()); @@ -1046,7 +1046,7 @@ TEST_F(AutofillTableTest, UpdateAutofillProfile) { EXPECT_EQ(profile, *db_profile); sql::Statement s_original(db.GetSQLConnection()->GetUniqueStatement( "SELECT date_modified FROM autofill_profiles")); - ASSERT_TRUE(s_original); + ASSERT_TRUE(s_original.is_valid()); ASSERT_TRUE(s_original.Step()); EXPECT_EQ(mock_creation_date, s_original.ColumnInt64(0)); EXPECT_FALSE(s_original.Step()); @@ -1063,7 +1063,7 @@ TEST_F(AutofillTableTest, UpdateAutofillProfile) { EXPECT_EQ(profile, *db_profile); sql::Statement s_updated(db.GetSQLConnection()->GetUniqueStatement( "SELECT date_modified FROM autofill_profiles")); - ASSERT_TRUE(s_updated); + ASSERT_TRUE(s_updated.is_valid()); ASSERT_TRUE(s_updated.Step()); EXPECT_LT(mock_creation_date, s_updated.ColumnInt64(0)); EXPECT_FALSE(s_updated.Step()); @@ -1073,7 +1073,7 @@ TEST_F(AutofillTableTest, UpdateAutofillProfile) { sql::Statement s_mock_modification_date( db.GetSQLConnection()->GetUniqueStatement( "UPDATE autofill_profiles SET date_modified = ?")); - ASSERT_TRUE(s_mock_modification_date); + ASSERT_TRUE(s_mock_modification_date.is_valid()); s_mock_modification_date.BindInt64(0, mock_modification_date); ASSERT_TRUE(s_mock_modification_date.Run()); @@ -1088,7 +1088,7 @@ TEST_F(AutofillTableTest, UpdateAutofillProfile) { EXPECT_EQ(profile, *db_profile); sql::Statement s_unchanged(db.GetSQLConnection()->GetUniqueStatement( "SELECT date_modified FROM autofill_profiles")); - ASSERT_TRUE(s_unchanged); + ASSERT_TRUE(s_unchanged.is_valid()); ASSERT_TRUE(s_unchanged.Step()); EXPECT_EQ(mock_modification_date, s_unchanged.ColumnInt64(0)); EXPECT_FALSE(s_unchanged.Step()); @@ -1110,7 +1110,7 @@ TEST_F(AutofillTableTest, UpdateCreditCard) { const time_t mock_creation_date = Time::Now().ToTimeT() - 13; sql::Statement s_mock_creation_date(db.GetSQLConnection()->GetUniqueStatement( "UPDATE credit_cards SET date_modified = ?")); - ASSERT_TRUE(s_mock_creation_date); + ASSERT_TRUE(s_mock_creation_date.is_valid()); s_mock_creation_date.BindInt64(0, mock_creation_date); ASSERT_TRUE(s_mock_creation_date.Run()); @@ -1122,7 +1122,7 @@ TEST_F(AutofillTableTest, UpdateCreditCard) { EXPECT_EQ(credit_card, *db_credit_card); sql::Statement s_original(db.GetSQLConnection()->GetUniqueStatement( "SELECT date_modified FROM credit_cards")); - ASSERT_TRUE(s_original); + ASSERT_TRUE(s_original.is_valid()); ASSERT_TRUE(s_original.Step()); EXPECT_EQ(mock_creation_date, s_original.ColumnInt64(0)); EXPECT_FALSE(s_original.Step()); @@ -1139,7 +1139,7 @@ TEST_F(AutofillTableTest, UpdateCreditCard) { EXPECT_EQ(credit_card, *db_credit_card); sql::Statement s_updated(db.GetSQLConnection()->GetUniqueStatement( "SELECT date_modified FROM credit_cards")); - ASSERT_TRUE(s_updated); + ASSERT_TRUE(s_updated.is_valid()); ASSERT_TRUE(s_updated.Step()); EXPECT_LT(mock_creation_date, s_updated.ColumnInt64(0)); EXPECT_FALSE(s_updated.Step()); @@ -1149,7 +1149,7 @@ TEST_F(AutofillTableTest, UpdateCreditCard) { sql::Statement s_mock_modification_date( db.GetSQLConnection()->GetUniqueStatement( "UPDATE credit_cards SET date_modified = ?")); - ASSERT_TRUE(s_mock_modification_date); + ASSERT_TRUE(s_mock_modification_date.is_valid()); s_mock_modification_date.BindInt64(0, mock_modification_date); ASSERT_TRUE(s_mock_modification_date.Run()); @@ -1164,7 +1164,7 @@ TEST_F(AutofillTableTest, UpdateCreditCard) { EXPECT_EQ(credit_card, *db_credit_card); sql::Statement s_unchanged(db.GetSQLConnection()->GetUniqueStatement( "SELECT date_modified FROM credit_cards")); - ASSERT_TRUE(s_unchanged); + ASSERT_TRUE(s_unchanged.is_valid()); ASSERT_TRUE(s_unchanged.Step()); EXPECT_EQ(mock_modification_date, s_unchanged.ColumnInt64(0)); EXPECT_FALSE(s_unchanged.Step()); @@ -1213,7 +1213,7 @@ TEST_F(AutofillTableTest, RemoveAutofillProfilesAndCreditCardsModifiedBetween) { sql::Statement s_autofill_profiles_bounded( db.GetSQLConnection()->GetUniqueStatement( "SELECT date_modified FROM autofill_profiles")); - ASSERT_TRUE(s_autofill_profiles_bounded); + ASSERT_TRUE(s_autofill_profiles_bounded.is_valid()); ASSERT_TRUE(s_autofill_profiles_bounded.Step()); EXPECT_EQ(11, s_autofill_profiles_bounded.ColumnInt64(0)); ASSERT_TRUE(s_autofill_profiles_bounded.Step()); @@ -1230,7 +1230,7 @@ TEST_F(AutofillTableTest, RemoveAutofillProfilesAndCreditCardsModifiedBetween) { sql::Statement s_credit_cards_bounded( db.GetSQLConnection()->GetUniqueStatement( "SELECT date_modified FROM credit_cards")); - ASSERT_TRUE(s_credit_cards_bounded); + ASSERT_TRUE(s_credit_cards_bounded.is_valid()); ASSERT_TRUE(s_credit_cards_bounded.Step()); EXPECT_EQ(47, s_credit_cards_bounded.ColumnInt64(0)); ASSERT_TRUE(s_credit_cards_bounded.Step()); @@ -1249,7 +1249,7 @@ TEST_F(AutofillTableTest, RemoveAutofillProfilesAndCreditCardsModifiedBetween) { sql::Statement s_autofill_profiles_unbounded( db.GetSQLConnection()->GetUniqueStatement( "SELECT date_modified FROM autofill_profiles")); - ASSERT_TRUE(s_autofill_profiles_unbounded); + ASSERT_TRUE(s_autofill_profiles_unbounded.is_valid()); ASSERT_TRUE(s_autofill_profiles_unbounded.Step()); EXPECT_EQ(11, s_autofill_profiles_unbounded.ColumnInt64(0)); ASSERT_TRUE(s_autofill_profiles_unbounded.Step()); @@ -1261,7 +1261,7 @@ TEST_F(AutofillTableTest, RemoveAutofillProfilesAndCreditCardsModifiedBetween) { sql::Statement s_credit_cards_unbounded( db.GetSQLConnection()->GetUniqueStatement( "SELECT date_modified FROM credit_cards")); - ASSERT_TRUE(s_credit_cards_unbounded); + ASSERT_TRUE(s_credit_cards_unbounded.is_valid()); ASSERT_TRUE(s_credit_cards_unbounded.Step()); EXPECT_EQ(47, s_credit_cards_unbounded.ColumnInt64(0)); EXPECT_FALSE(s_credit_cards_unbounded.Step()); @@ -1276,14 +1276,14 @@ TEST_F(AutofillTableTest, RemoveAutofillProfilesAndCreditCardsModifiedBetween) { sql::Statement s_autofill_profiles_empty( db.GetSQLConnection()->GetUniqueStatement( "SELECT date_modified FROM autofill_profiles")); - ASSERT_TRUE(s_autofill_profiles_empty); + ASSERT_TRUE(s_autofill_profiles_empty.is_valid()); EXPECT_FALSE(s_autofill_profiles_empty.Step()); ASSERT_EQ(1UL, credit_card_guids.size()); EXPECT_EQ("00000000-0000-0000-0000-000000000009", credit_card_guids[0]); sql::Statement s_credit_cards_empty( db.GetSQLConnection()->GetUniqueStatement( "SELECT date_modified FROM credit_cards")); - ASSERT_TRUE(s_credit_cards_empty); + ASSERT_TRUE(s_credit_cards_empty.is_valid()); EXPECT_FALSE(s_credit_cards_empty.Step()); } diff --git a/chrome/browser/webdata/keyword_table.cc b/chrome/browser/webdata/keyword_table.cc index 9243902..51ded3b 100644 --- a/chrome/browser/webdata/keyword_table.cc +++ b/chrome/browser/webdata/keyword_table.cc @@ -148,14 +148,10 @@ bool KeywordTable::AddKeyword(const TemplateURL& url) { "autogenerate_keyword, logo_id, created_by_policy, instant_url, " "last_modified, sync_guid, id) VALUES " "(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } BindURLToStatement(url, &s); s.BindInt64(kUrlIdPosition, url.id()); + if (!s.Run()) { - NOTREACHED(); return false; } return UpdateBackupSignature(); @@ -163,12 +159,10 @@ bool KeywordTable::AddKeyword(const TemplateURL& url) { bool KeywordTable::RemoveKeyword(TemplateURLID id) { DCHECK(id); - sql::Statement s(db_->GetUniqueStatement("DELETE FROM keywords WHERE id=?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } + sql::Statement s( + db_->GetUniqueStatement("DELETE FROM keywords WHERE id = ?")); s.BindInt64(0, id); + return s.Run() && UpdateBackupSignature(); } @@ -180,10 +174,7 @@ bool KeywordTable::GetKeywords(std::vector<TemplateURL*>* urls) { "suggest_url, prepopulate_id, autogenerate_keyword, logo_id, " "created_by_policy, instant_url, last_modified, sync_guid " "FROM keywords ORDER BY id ASC")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } + while (s.Step()) { TemplateURL* template_url = new TemplateURL(); GetURLFromStatement(s, template_url); @@ -203,12 +194,9 @@ bool KeywordTable::UpdateKeyword(const TemplateURL& url) { "suggest_url=?, prepopulate_id=?, autogenerate_keyword=?, " "logo_id=?, created_by_policy=?, instant_url=?, last_modified=?, " "sync_guid=? WHERE id=?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } BindURLToStatement(url, &s); s.BindInt64(kUrlIdPosition, url.id()); + return s.Run() && UpdateBackupSignature(); } diff --git a/chrome/browser/webdata/logins_table.cc b/chrome/browser/webdata/logins_table.cc index 4ba1d2f..5ee7b77 100644 --- a/chrome/browser/webdata/logins_table.cc +++ b/chrome/browser/webdata/logins_table.cc @@ -114,10 +114,6 @@ bool LoginsTable::AddLogin(const PasswordForm& form) { " blacklisted_by_user, scheme) " "VALUES " "(?,?,?,?,?,?,?,?,?,?,?,?,?)")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } std::string encrypted_password; s.BindString(0, form.origin.spec()); @@ -135,11 +131,8 @@ bool LoginsTable::AddLogin(const PasswordForm& form) { s.BindInt64(10, form.date_created.ToTimeT()); s.BindInt(11, form.blacklisted_by_user); s.BindInt(12, form.scheme); - if (!s.Run()) { - NOTREACHED(); - return false; - } - return true; + + return s.Run(); } bool LoginsTable::UpdateLogin(const PasswordForm& form) { @@ -154,10 +147,6 @@ bool LoginsTable::UpdateLogin(const PasswordForm& form) { "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; @@ -172,11 +161,7 @@ bool LoginsTable::UpdateLogin(const PasswordForm& form) { s.BindString16(7, form.password_element); s.BindString(8, form.signon_realm); - if (!s.Run()) { - NOTREACHED(); - return false; - } - return true; + return s.Run(); } bool LoginsTable::RemoveLogin(const PasswordForm& form) { @@ -189,10 +174,6 @@ bool LoginsTable::RemoveLogin(const PasswordForm& form) { "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); @@ -200,11 +181,7 @@ bool LoginsTable::RemoveLogin(const PasswordForm& form) { s.BindString16(4, form.submit_element); s.BindString(5, form.signon_realm); - if (!s.Run()) { - NOTREACHED(); - return false; - } - return true; + return s.Run(); } bool LoginsTable::RemoveLoginsCreatedBetween(base::Time delete_begin, @@ -212,10 +189,6 @@ bool LoginsTable::RemoveLoginsCreatedBetween(base::Time delete_begin, sql::Statement s1(db_->GetUniqueStatement( "DELETE FROM logins WHERE " "date_created >= ? AND date_created < ?")); - if (!s1) { - NOTREACHED() << "Statement 1 prepare failed"; - return false; - } s1.BindInt64(0, delete_begin.ToTimeT()); s1.BindInt64(1, delete_end.is_null() ? @@ -226,10 +199,6 @@ bool LoginsTable::RemoveLoginsCreatedBetween(base::Time delete_begin, #if defined(OS_WIN) sql::Statement s2(db_->GetUniqueStatement( "DELETE FROM ie7_logins WHERE date_created >= ? AND date_created < ?")); - if (!s2) { - NOTREACHED() << "Statement 2 prepare failed"; - return false; - } s2.BindInt64(0, delete_begin.ToTimeT()); s2.BindInt64(1, delete_end.is_null() ? @@ -252,11 +221,6 @@ bool LoginsTable::GetLogins(const PasswordForm& form, "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()) { @@ -281,10 +245,6 @@ bool LoginsTable::GetAllLogins(std::vector<PasswordForm*>* forms, stmt.append("ORDER BY origin_url"); sql::Statement s(db_->GetUniqueStatement(stmt.c_str())); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } while (s.Step()) { PasswordForm* new_form = new PasswordForm(); diff --git a/chrome/browser/webdata/logins_table_win.cc b/chrome/browser/webdata/logins_table_win.cc index 581c176..8a751fe 100644 --- a/chrome/browser/webdata/logins_table_win.cc +++ b/chrome/browser/webdata/logins_table_win.cc @@ -15,37 +15,21 @@ bool LoginsTable::AddIE7Login(const IE7PasswordInfo& info) { "INSERT OR REPLACE INTO ie7_logins " "(url_hash, password_value, date_created) " "VALUES (?,?,?)")); - if (!s) { - NOTREACHED() << db_->GetErrorMessage(); - return false; - } - s.BindString(0, WideToUTF8(info.url_hash)); s.BindBlob(1, &info.encrypted_data.front(), static_cast<int>(info.encrypted_data.size())); s.BindInt64(2, info.date_created.ToTimeT()); - if (!s.Run()) { - NOTREACHED(); - return false; - } - return true; + + return s.Run(); } bool LoginsTable::RemoveIE7Login(const IE7PasswordInfo& info) { // Remove a login by UNIQUE-constrained fields. sql::Statement s(db_->GetUniqueStatement( "DELETE FROM ie7_logins WHERE url_hash = ?")); - if (!s) { - NOTREACHED() << db_->GetErrorMessage(); - return false; - } s.BindString(0, WideToUTF8(info.url_hash)); - if (!s.Run()) { - NOTREACHED(); - return false; - } - return true; + return s.Run(); } bool LoginsTable::GetIE7Login(const IE7PasswordInfo& info, @@ -54,12 +38,8 @@ bool LoginsTable::GetIE7Login(const IE7PasswordInfo& info, sql::Statement s(db_->GetUniqueStatement( "SELECT password_value, date_created FROM ie7_logins " "WHERE url_hash == ? ")); - if (!s) { - NOTREACHED() << db_->GetErrorMessage(); - return false; - } + s.BindString16(0, info.url_hash); - s.BindString(0, WideToUTF8(info.url_hash)); if (s.Step()) { s.ColumnBlobAsVector(0, &result->encrypted_data); result->date_created = base::Time::FromTimeT(s.ColumnInt64(1)); diff --git a/chrome/browser/webdata/token_service_table.cc b/chrome/browser/webdata/token_service_table.cc index 6d9a737..14857c8 100644 --- a/chrome/browser/webdata/token_service_table.cc +++ b/chrome/browser/webdata/token_service_table.cc @@ -30,36 +30,27 @@ bool TokenServiceTable::IsSyncable() { bool TokenServiceTable::RemoveAllTokens() { sql::Statement s(db_->GetUniqueStatement( "DELETE FROM token_service")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } return s.Run(); } bool TokenServiceTable::SetTokenForService(const std::string& service, const std::string& token) { - // Don't bother with a cached statement since this will be a relatively - // infrequent operation. - sql::Statement s(db_->GetUniqueStatement( - "INSERT OR REPLACE INTO token_service " - "(service, encrypted_token) VALUES (?, ?)")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } - std::string encrypted_token; - bool encrypted = Encryptor::EncryptString(token, &encrypted_token); if (!encrypted) { return false; } + // Don't bother with a cached statement since this will be a relatively + // infrequent operation. + sql::Statement s(db_->GetUniqueStatement( + "INSERT OR REPLACE INTO token_service " + "(service, encrypted_token) VALUES (?, ?)")); s.BindString(0, service); s.BindBlob(1, encrypted_token.data(), static_cast<int>(encrypted_token.length())); + return s.Run(); } @@ -67,10 +58,9 @@ bool TokenServiceTable::GetAllTokens( std::map<std::string, std::string>* tokens) { sql::Statement s(db_->GetUniqueStatement( "SELECT service, encrypted_token FROM token_service")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; + + if (!s.is_valid()) return false; - } while (s.Step()) { std::string encrypted_token; @@ -89,4 +79,3 @@ bool TokenServiceTable::GetAllTokens( } return true; } - diff --git a/chrome/browser/webdata/web_apps_table.cc b/chrome/browser/webdata/web_apps_table.cc index 7df48f7..a503925 100644 --- a/chrome/browser/webdata/web_apps_table.cc +++ b/chrome/browser/webdata/web_apps_table.cc @@ -55,16 +55,13 @@ bool WebAppsTable::SetWebAppImage(const GURL& url, const SkBitmap& image) { sql::Statement s(db_->GetUniqueStatement( "INSERT OR REPLACE INTO web_app_icons " "(url, width, height, image) VALUES (?, ?, ?, ?)")); - if (!s) - return false; - std::vector<unsigned char> image_data; gfx::PNGCodec::EncodeBGRASkBitmap(image, false, &image_data); - s.BindString(0, history::HistoryDatabase::GURLToDatabaseURL(url)); s.BindInt(1, image.width()); s.BindInt(2, image.height()); s.BindBlob(3, &image_data.front(), static_cast<int>(image_data.size())); + return s.Run(); } @@ -72,11 +69,8 @@ bool WebAppsTable::GetWebAppImages(const GURL& url, std::vector<SkBitmap>* images) { sql::Statement s(db_->GetUniqueStatement( "SELECT image FROM web_app_icons WHERE url=?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } s.BindString(0, history::HistoryDatabase::GURLToDatabaseURL(url)); + while (s.Step()) { SkBitmap image; int col_bytes = s.ColumnByteLength(0); @@ -91,51 +85,38 @@ bool WebAppsTable::GetWebAppImages(const GURL& url, } } } - return true; + return s.Succeeded(); } bool WebAppsTable::SetWebAppHasAllImages(const GURL& url, bool has_all_images) { sql::Statement s(db_->GetUniqueStatement( "INSERT OR REPLACE INTO web_apps (url, has_all_images) VALUES (?, ?)")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } s.BindString(0, history::HistoryDatabase::GURLToDatabaseURL(url)); s.BindInt(1, has_all_images ? 1 : 0); + return s.Run(); } bool WebAppsTable::GetWebAppHasAllImages(const GURL& url) { sql::Statement s(db_->GetUniqueStatement( "SELECT has_all_images FROM web_apps WHERE url=?")); - if (!s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } s.BindString(0, history::HistoryDatabase::GURLToDatabaseURL(url)); + return (s.Step() && s.ColumnInt(0) == 1); } bool WebAppsTable::RemoveWebApp(const GURL& url) { sql::Statement delete_s(db_->GetUniqueStatement( "DELETE FROM web_app_icons WHERE url = ?")); - if (!delete_s) { - NOTREACHED() << "Statement prepare failed"; - return false; - } delete_s.BindString(0, history::HistoryDatabase::GURLToDatabaseURL(url)); + if (!delete_s.Run()) return false; sql::Statement delete_s2(db_->GetUniqueStatement( "DELETE FROM web_apps WHERE url = ?")); - if (!delete_s2) { - NOTREACHED() << "Statement prepare failed"; - return false; - } delete_s2.BindString(0, history::HistoryDatabase::GURLToDatabaseURL(url)); + return delete_s2.Run(); } - diff --git a/chrome/browser/webdata/web_apps_table.h b/chrome/browser/webdata/web_apps_table.h index 1222f5d..d49b6acb 100644 --- a/chrome/browser/webdata/web_apps_table.h +++ b/chrome/browser/webdata/web_apps_table.h @@ -38,6 +38,10 @@ class WebAppsTable : public WebDatabaseTable { virtual bool IsSyncable() OVERRIDE; bool SetWebAppImage(const GURL& url, const SkBitmap& image); + + // Returns true if all images are retrieved. Returns false if there is a + // database error. In this case, the state of images is undefined; it may have + // partial results or no results from the call. bool GetWebAppImages(const GURL& url, std::vector<SkBitmap>* images); bool SetWebAppHasAllImages(const GURL& url, bool has_all_images); diff --git a/chrome/browser/webdata/web_database_migration_unittest.cc b/chrome/browser/webdata/web_database_migration_unittest.cc index 83adc00..874d572 100644 --- a/chrome/browser/webdata/web_database_migration_unittest.cc +++ b/chrome/browser/webdata/web_database_migration_unittest.cc @@ -697,7 +697,7 @@ TEST_F(WebDatabaseMigrationTest, MigrateVersion29ToCurrent) { sql::Statement s_profiles(connection.GetUniqueStatement( "SELECT date_modified FROM autofill_profiles ")); - ASSERT_TRUE(s_profiles); + ASSERT_TRUE(s_profiles.is_valid()); while (s_profiles.Step()) { EXPECT_GE(s_profiles.ColumnInt64(0), pre_creation_time.ToTimeT()); @@ -708,7 +708,7 @@ TEST_F(WebDatabaseMigrationTest, MigrateVersion29ToCurrent) { sql::Statement s_credit_cards(connection.GetUniqueStatement( "SELECT date_modified FROM credit_cards ")); - ASSERT_TRUE(s_credit_cards); + ASSERT_TRUE(s_credit_cards.is_valid()); while (s_credit_cards.Step()) { EXPECT_GE(s_credit_cards.ColumnInt64(0), pre_creation_time.ToTimeT()); @@ -1875,4 +1875,3 @@ TEST_F(WebDatabaseMigrationTest, MigrateVersion42ToCurrent) { EXPECT_FALSE(s.Step()); } } - diff --git a/chrome/browser/webdata/web_intents_table.cc b/chrome/browser/webdata/web_intents_table.cc index 61bf89a..b717ba4 100644 --- a/chrome/browser/webdata/web_intents_table.cc +++ b/chrome/browser/webdata/web_intents_table.cc @@ -34,7 +34,7 @@ bool ExtractIntents(sql::Statement* s, service.disposition = WebIntentServiceData::DISPOSITION_INLINE; services->push_back(service); } - return true; + return s->Succeeded(); } } // namespace @@ -79,8 +79,8 @@ bool WebIntentsTable::GetWebIntentServices( sql::Statement s(db_->GetUniqueStatement( "SELECT service_url, action, type, title, disposition FROM web_intents " "WHERE action=?")); - s.BindString16(0, action); + return ExtractIntents(&s, services); } @@ -94,8 +94,8 @@ bool WebIntentsTable::GetWebIntentServicesForURL( sql::Statement s(db_->GetUniqueStatement( "SELECT service_url, action, type, title, disposition FROM web_intents " "WHERE service_url=?")); - s.BindString16(0, service_url); + return ExtractIntents(&s, services); } @@ -123,6 +123,7 @@ bool WebIntentsTable::SetWebIntentService(const WebIntentServiceData& service) { s.BindString16(2, service.action); s.BindString16(3, service.title); s.BindString16(4, disposition); + return s.Run(); } @@ -134,9 +135,9 @@ bool WebIntentsTable::RemoveWebIntentService( sql::Statement s(db_->GetUniqueStatement( "DELETE FROM web_intents " "WHERE service_url = ? AND action = ? AND type = ?")); - s.BindString(0, service.service_url.spec()); s.BindString16(1, service.action); s.BindString16(2, service.type); + return s.Run(); } diff --git a/sql/statement.cc b/sql/statement.cc index fd73b0f..a5daae4 100644 --- a/sql/statement.cc +++ b/sql/statement.cc @@ -256,6 +256,11 @@ const char* Statement::GetSQLStatement() { } bool Statement::CheckOk(int err) const { + // Binding to a non-existent variable is evidence of a serious error. + // TODO(gbillock,shess): make this invalidate the statement so it + // can't wreak havoc. + if (err == SQLITE_RANGE) + DLOG(FATAL) << "Bind value out of range"; return err == SQLITE_OK; } diff --git a/sql/statement.h b/sql/statement.h index 97fdb5e..c7e2c40 100644 --- a/sql/statement.h +++ b/sql/statement.h @@ -58,7 +58,7 @@ class SQL_EXPORT Statement { // Returns true if the statement can be executed. All functions can still // be used if the statement is invalid, but they will return failure or some // default value. This is because the statement can become invalid in the - // middle of executing a command if there is a serioud error and the database + // middle of executing a command if there is a serious error and the database // has to be reset. bool is_valid() const { return ref_->is_valid(); } |