diff options
| -rw-r--r-- | chrome/browser/history/thumbnail_database.cc | 618 | ||||
| -rw-r--r-- | chrome/browser/history/thumbnail_database.h | 35 | ||||
| -rw-r--r-- | chrome/browser/history/thumbnail_database_unittest.cc | 212 | ||||
| -rw-r--r-- | chrome/chrome_tests_unit.gypi | 1 | ||||
| -rw-r--r-- | sql/connection.cc | 4 | ||||
| -rw-r--r-- | sql/connection.h | 7 | ||||
| -rw-r--r-- | sql/recovery.cc | 19 | ||||
| -rw-r--r-- | sql/recovery.h | 10 |
8 files changed, 700 insertions, 206 deletions
diff --git a/chrome/browser/history/thumbnail_database.cc b/chrome/browser/history/thumbnail_database.cc index 3014338..a6274571 100644 --- a/chrome/browser/history/thumbnail_database.cc +++ b/chrome/browser/history/thumbnail_database.cc @@ -21,6 +21,7 @@ #include "chrome/browser/history/url_database.h" #include "chrome/common/chrome_version_info.h" #include "chrome/common/dump_without_crashing.h" +#include "sql/recovery.h" #include "sql/statement.h" #include "sql/transaction.h" #include "third_party/sqlite/sqlite3.h" @@ -205,66 +206,411 @@ void ReportError(sql::Connection* db, int error) { // TODO(shess): If this proves out, perhaps lift the code out to // chrome/browser/diagnostics/sqlite_diagnostics.{h,cc}. +void GenerateDiagnostics(sql::Connection* db, + size_t startup_kb, + int extended_error) { + int error = (extended_error & 0xFF); + + // Infrequently report information about the error up to the crash + // server. + static const uint64 kReportsPerMillion = 50000; + + // Since some/most errors will not resolve themselves, only report + // once per Chrome run. + static bool reported = false; + if (reported) + return; + + uint64 rand = base::RandGenerator(1000000); + if (error == SQLITE_CORRUPT) { + // Once the database is known to be corrupt, it will generate a + // stream of errors until someone fixes it, so give one chance. + // Set first in case of errors in generating the report. + reported = true; + + // Corrupt cases currently dominate, report them very infrequently. + static const uint64 kCorruptReportsPerMillion = 10000; + if (rand < kCorruptReportsPerMillion) + ReportCorrupt(db, startup_kb); + } else if (error == SQLITE_READONLY) { + // SQLITE_READONLY appears similar to SQLITE_CORRUPT - once it + // is seen, it is almost guaranteed to be seen again. + reported = true; + + if (rand < kReportsPerMillion) + ReportError(db, extended_error); + } else { + // Only set the flag when making a report. This should allow + // later (potentially different) errors in a stream of errors to + // be reported. + // + // TODO(shess): Would it be worthwile to audit for which cases + // want once-only handling? Sqlite.Error.Thumbnail shows + // CORRUPT and READONLY as almost 95% of all reports on these + // channels, so probably easier to just harvest from the field. + if (rand < kReportsPerMillion) { + reported = true; + ReportError(db, extended_error); + } + } +} + +bool InitTables(sql::Connection* db) { + const char kIconMappingSql[] = + "CREATE TABLE IF NOT EXISTS icon_mapping" + "(" + "id INTEGER PRIMARY KEY," + "page_url LONGVARCHAR NOT NULL," + "icon_id INTEGER" + ")"; + if (!db->Execute(kIconMappingSql)) + return false; + + const char kFaviconsSql[] = + "CREATE TABLE IF NOT EXISTS favicons" + "(" + "id INTEGER PRIMARY KEY," + "url LONGVARCHAR NOT NULL," + // Set the default icon_type as FAVICON to be consistent with + // table upgrade in UpgradeToVersion4(). + "icon_type INTEGER DEFAULT 1" + ")"; + if (!db->Execute(kFaviconsSql)) + return false; + + const char kFaviconBitmapsSql[] = + "CREATE TABLE IF NOT EXISTS favicon_bitmaps" + "(" + "id INTEGER PRIMARY KEY," + "icon_id INTEGER NOT NULL," + "last_updated INTEGER DEFAULT 0," + "image_data BLOB," + "width INTEGER DEFAULT 0," + "height INTEGER DEFAULT 0" + ")"; + if (!db->Execute(kFaviconBitmapsSql)) + return false; + + return true; +} + +bool InitIndices(sql::Connection* db) { + const char kIconMappingUrlIndexSql[] = + "CREATE INDEX IF NOT EXISTS icon_mapping_page_url_idx" + " ON icon_mapping(page_url)"; + const char kIconMappingIdIndexSql[] = + "CREATE INDEX IF NOT EXISTS icon_mapping_icon_id_idx" + " ON icon_mapping(icon_id)"; + if (!db->Execute(kIconMappingUrlIndexSql) || + !db->Execute(kIconMappingIdIndexSql)) { + return false; + } + + const char kFaviconsIndexSql[] = + "CREATE INDEX IF NOT EXISTS favicons_url ON favicons(url)"; + if (!db->Execute(kFaviconsIndexSql)) + return false; + + const char kFaviconBitmapsIndexSql[] = + "CREATE INDEX IF NOT EXISTS favicon_bitmaps_icon_id ON " + "favicon_bitmaps(icon_id)"; + if (!db->Execute(kFaviconBitmapsIndexSql)) + return false; + + return true; +} + +enum RecoveryEventType { + RECOVERY_EVENT_RECOVERED = 0, + RECOVERY_EVENT_FAILED_SCOPER, + RECOVERY_EVENT_FAILED_META_VERSION_ERROR, + RECOVERY_EVENT_FAILED_META_VERSION_NONE, + RECOVERY_EVENT_FAILED_META_WRONG_VERSION6, + RECOVERY_EVENT_FAILED_META_WRONG_VERSION5, + RECOVERY_EVENT_FAILED_META_WRONG_VERSION, + RECOVERY_EVENT_FAILED_RECOVER_META, + RECOVERY_EVENT_FAILED_META_INSERT, + RECOVERY_EVENT_FAILED_INIT, + RECOVERY_EVENT_FAILED_RECOVER_FAVICONS, + RECOVERY_EVENT_FAILED_FAVICONS_INSERT, + RECOVERY_EVENT_FAILED_RECOVER_FAVICON_BITMAPS, + RECOVERY_EVENT_FAILED_FAVICON_BITMAPS_INSERT, + RECOVERY_EVENT_FAILED_RECOVER_ICON_MAPPING, + RECOVERY_EVENT_FAILED_ICON_MAPPING_INSERT, + + // Always keep this at the end. + RECOVERY_EVENT_MAX, +}; + +void RecordRecoveryEvent(RecoveryEventType recovery_event) { + UMA_HISTOGRAM_ENUMERATION("History.FaviconsRecovery", + recovery_event, RECOVERY_EVENT_MAX); +} + +// Recover the database to the extent possible, razing it if recovery +// is not possible. +// TODO(shess): This is mostly just a safe proof of concept. In the +// real world, this database is probably not worthwhile recovering, as +// opposed to just razing it and starting over whenever corruption is +// detected. So this database is a good test subject. +void RecoverDatabaseOrRaze(sql::Connection* db, const base::FilePath& db_path) { + // TODO(shess): Reset back after? + db->reset_error_callback(); + + scoped_ptr<sql::Recovery> recovery = sql::Recovery::Begin(db, db_path); + if (!recovery) { + // TODO(shess): Unable to create recovery connection. This + // implies something substantial is wrong. At this point |db| has + // been poisoned so there is nothing really to do. + // + // Possible responses are unclear. If the failure relates to a + // problem somehow specific to the temporary file used to back the + // database, then an in-memory database could possibly be used. + // This could potentially allow recovering the main database, and + // might be simple to implement w/in Begin(). + RecordRecoveryEvent(RECOVERY_EVENT_FAILED_SCOPER); + return; + } + + // Setup the meta recovery table, and check that the version number + // is covered by the recovery code. + // TODO(shess): sql::Recovery should provide a helper to handle meta. + { + const char kRecoverySql[] = + "CREATE VIRTUAL TABLE temp.recover_meta USING recover" + "(" + "corrupt.meta," + "key TEXT NOT NULL," + "value TEXT" // Really? Never int? + ")"; + if (!recovery->db()->Execute(kRecoverySql)) { + // TODO(shess): Failure to create the recover_meta table could + // mean that the main database is too corrupt to access, or that + // the meta table doesn't exist. + sql::Recovery::Rollback(recovery.Pass()); + RecordRecoveryEvent(RECOVERY_EVENT_FAILED_RECOVER_META); + return; + } + + { + const char kRecoveryVersionSql[] = + "SELECT value FROM recover_meta WHERE key = 'version'"; + sql::Statement recovery_version( + recovery->db()->GetUniqueStatement(kRecoveryVersionSql)); + if (!recovery_version.Step()) { + if (!recovery_version.Succeeded()) { + RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_VERSION_ERROR); + // TODO(shess): An error while processing the statement is + // probably not recoverable. + } else { + RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_VERSION_NONE); + // TODO(shess): If a positive version lock cannot be achieved, + // the database could still be recovered by optimistically + // attempting to copy things. In the limit, the schema found + // could be inspected. Less clear is whether optimistic + // recovery really makes sense. + } + recovery_version.Clear(); + sql::Recovery::Rollback(recovery.Pass()); + return; + } else if (7 != recovery_version.ColumnInt(0)) { + // TODO(shess): Recovery code is generally schema-dependent. + // Version 6 should be easy, if the numbers warrant it. + // Version 5 is probably not warranted. + switch (recovery_version.ColumnInt(0)) { + case 6 : + RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_WRONG_VERSION6); + break; + case 5 : + RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_WRONG_VERSION5); + break; + default : + RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_WRONG_VERSION); + break; + } + recovery_version.Clear(); + sql::Recovery::Rollback(recovery.Pass()); + return; + } + } + + const char kCopySql[] = + "INSERT OR REPLACE INTO meta SELECT key, value FROM recover_meta"; + if (!recovery->db()->Execute(kCopySql)) { + // TODO(shess): Earlier the version was queried, unclear what + // this failure could mean. + sql::Recovery::Rollback(recovery.Pass()); + RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_INSERT); + return; + } + } + + // Create a fresh version of the database. The recovery code uses + // conflict-resolution to handle duplicates, so the indices are + // necessary. + if (!InitTables(recovery->db()) || !InitIndices(recovery->db())) { + // TODO(shess): Unable to create the new schema in the new + // database. The new database should be a temporary file, so + // being unable to work with it is pretty unclear. + // + // What are the potential responses, even? The recovery database + // could be opened as in-memory. If the temp database had a + // filesystem problem and the temp filesystem differs from the + // main database, then that could fix it. + sql::Recovery::Rollback(recovery.Pass()); + RecordRecoveryEvent(RECOVERY_EVENT_FAILED_INIT); + return; + } + + // Setup favicons table. + { + const char kRecoverySql[] = + "CREATE VIRTUAL TABLE temp.recover_favicons USING recover" + "(" + "corrupt.favicons," + "id ROWID," + "url TEXT NOT NULL," + "icon_type INTEGER" + ")"; + if (!recovery->db()->Execute(kRecoverySql)) { + // TODO(shess): Failure to create the recovery table probably + // means unrecoverable. + sql::Recovery::Rollback(recovery.Pass()); + RecordRecoveryEvent(RECOVERY_EVENT_FAILED_RECOVER_FAVICONS); + return; + } + + // TODO(shess): Check if the DEFAULT 1 will just cover the + // COALESCE(). Either way, the new code has a literal 1 rather + // than a NULL, right? + const char kCopySql[] = + "INSERT OR REPLACE INTO favicons " + "SELECT id, url, COALESCE(icon_type, 1) FROM recover_favicons"; + if (!recovery->db()->Execute(kCopySql)) { + // TODO(shess): The recover_favicons table should mask problems + // with the source file, so this implies failure to write to the + // recovery database. + sql::Recovery::Rollback(recovery.Pass()); + RecordRecoveryEvent(RECOVERY_EVENT_FAILED_FAVICONS_INSERT); + return; + } + } + + // Setup favicons_bitmaps table. + { + const char kRecoverySql[] = + "CREATE VIRTUAL TABLE temp.recover_favicons_bitmaps USING recover" + "(" + "corrupt.favicon_bitmaps," + "id ROWID," + "icon_id INTEGER STRICT NOT NULL," + "last_updated INTEGER," + "image_data BLOB," + "width INTEGER," + "height INTEGER" + ")"; + if (!recovery->db()->Execute(kRecoverySql)) { + // TODO(shess): Failure to create the recovery table probably + // means unrecoverable. + sql::Recovery::Rollback(recovery.Pass()); + RecordRecoveryEvent(RECOVERY_EVENT_FAILED_RECOVER_FAVICON_BITMAPS); + return; + } + + const char kCopySql[] = + "INSERT OR REPLACE INTO favicon_bitmaps " + "SELECT id, icon_id, COALESCE(last_updated, 0), image_data, " + " COALESCE(width, 0), COALESCE(height, 0) " + "FROM recover_favicons_bitmaps"; + if (!recovery->db()->Execute(kCopySql)) { + // TODO(shess): The recover_faviconbitmaps table should mask + // problems with the source file, so this implies failure to + // write to the recovery database. + sql::Recovery::Rollback(recovery.Pass()); + RecordRecoveryEvent(RECOVERY_EVENT_FAILED_FAVICON_BITMAPS_INSERT); + return; + } + } + + // Setup icon_mapping table. + { + const char kRecoverySql[] = + "CREATE VIRTUAL TABLE temp.recover_icon_mapping USING recover" + "(" + "corrupt.icon_mapping," + "id ROWID," + "page_url TEXT STRICT NOT NULL," + "icon_id INTEGER STRICT" + ")"; + if (!recovery->db()->Execute(kRecoverySql)) { + // TODO(shess): Failure to create the recovery table probably + // means unrecoverable. + sql::Recovery::Rollback(recovery.Pass()); + RecordRecoveryEvent(RECOVERY_EVENT_FAILED_RECOVER_ICON_MAPPING); + return; + } + + const char kCopySql[] = + "INSERT OR REPLACE INTO icon_mapping " + "SELECT id, page_url, icon_id FROM recover_icon_mapping"; + if (!recovery->db()->Execute(kCopySql)) { + // TODO(shess): The recover_icon_mapping table should mask + // problems with the source file, so this implies failure to + // write to the recovery database. + sql::Recovery::Rollback(recovery.Pass()); + RecordRecoveryEvent(RECOVERY_EVENT_FAILED_ICON_MAPPING_INSERT); + return; + } + } + + // TODO(shess): Is it possible/likely to have broken foreign-key + // issues with the tables? + // - icon_mapping.icon_id maps to no favicons.id + // - favicon_bitmaps.icon_id maps to no favicons.id + // - favicons.id is referenced by no icon_mapping.icon_id + // - favicons.id is referenced by no favicon_bitmaps.icon_id + // This step is possibly not worth the effort necessary to develop + // and sequence the statements, as it is basically a form of garbage + // collection. + + ignore_result(sql::Recovery::Recovered(recovery.Pass())); + RecordRecoveryEvent(RECOVERY_EVENT_RECOVERED); +} + void DatabaseErrorCallback(sql::Connection* db, + const base::FilePath& db_path, size_t startup_kb, - int error, + int extended_error, sql::Statement* stmt) { // TODO(shess): Assert that this is running on a safe thread. // AFAICT, should be the history thread, but at this level I can't // see how to reach that. - // Infrequently report information about the error up to the crash - // server. - static const uint64 kReportsPerMillion = 50000; - // TODO(shess): For now, don't report on beta or stable so as not to // overwhelm the crash server. Once the big fish are fried, // consider reporting at a reduced rate on the bigger channels. chrome::VersionInfo::Channel channel = chrome::VersionInfo::GetChannel(); - - // Since some/most errors will not resolve themselves, only report - // once per Chrome run. - static bool reported = false; - if (channel != chrome::VersionInfo::CHANNEL_STABLE && - channel != chrome::VersionInfo::CHANNEL_BETA && - !reported) { - uint64 rand = base::RandGenerator(1000000); - if (error == SQLITE_CORRUPT) { - // Once the database is known to be corrupt, it will generate a - // stream of errors until someone fixes it, so give one chance. - // Set first in case of errors in generating the report. - reported = true; - - // Corrupt cases currently dominate, report them very infrequently. - static const uint64 kCorruptReportsPerMillion = 10000; - if (rand < kCorruptReportsPerMillion) - ReportCorrupt(db, startup_kb); - } else if (error == SQLITE_READONLY) { - // SQLITE_READONLY appears similar to SQLITE_CORRUPT - once it - // is seen, it is almost guaranteed to be seen again. - reported = true; + channel != chrome::VersionInfo::CHANNEL_BETA) { + GenerateDiagnostics(db, startup_kb, extended_error); + } - if (rand < kReportsPerMillion) - ReportError(db, error); - } else { - // Only set the flag when making a report. This should allow - // later (potentially different) errors in a stream of errors to - // be reported. - // - // TODO(shess): Would it be worthwile to audit for which cases - // want once-only handling? Sqlite.Error.Thumbnail shows - // CORRUPT and READONLY as almost 95% of all reports on these - // channels, so probably easier to just harvest from the field. - if (rand < kReportsPerMillion) { - reported = true; - ReportError(db, error); - } + // Attempt to recover corrupt databases. + // TODO(shess): Remove the channel restriction once it becomes clear + // that the recovery code fails gracefully. + if (channel != chrome::VersionInfo::CHANNEL_STABLE && + channel != chrome::VersionInfo::CHANNEL_BETA) { + int error = (extended_error & 0xFF); + if (error == SQLITE_CORRUPT || + error == SQLITE_CANTOPEN || + error == SQLITE_NOTADB) { + RecoverDatabaseOrRaze(db, db_path); } } // The default handling is to assert on debug and to ignore on release. - DLOG(FATAL) << db->GetErrorMessage(); + if (!sql::Connection::ShouldIgnoreSqliteError(extended_error)) + DLOG(FATAL) << db->GetErrorMessage(); } } // namespace @@ -337,12 +683,8 @@ sql::InitStatus ThumbnailDatabase::Init( // Create the tables. if (!meta_table_.Init(&db_, kCurrentVersionNumber, kCompatibleVersionNumber) || - !InitFaviconBitmapsTable(&db_) || - !InitFaviconBitmapsIndex() || - !InitFaviconsTable(&db_) || - !InitFaviconsIndex() || - !InitIconMappingTable(&db_) || - !InitIconMappingIndex()) { + !InitTables(&db_) || + !InitIndices(&db_)) { db_.Close(); return sql::INIT_FAILURE; } @@ -435,7 +777,8 @@ sql::InitStatus ThumbnailDatabase::OpenDatabase(sql::Connection* db, startup_kb = static_cast<size_t>(size_64 / 1024); db->set_histogram_tag("Thumbnail"); - db->set_error_callback(base::Bind(&DatabaseErrorCallback, db, startup_kb)); + db->set_error_callback(base::Bind(&DatabaseErrorCallback, + db, db_name, startup_kb)); // Thumbnails db now only stores favicons, so we don't need that big a page // size or cache. @@ -467,45 +810,6 @@ bool ThumbnailDatabase::UpgradeToVersion3() { return true; } -bool ThumbnailDatabase::InitFaviconsTable(sql::Connection* db) { - const char kSql[] = - "CREATE TABLE IF NOT EXISTS favicons" - "(" - "id INTEGER PRIMARY KEY," - "url LONGVARCHAR NOT NULL," - // Set the default icon_type as FAVICON to be consistent with - // table upgrade in UpgradeToVersion4(). - "icon_type INTEGER DEFAULT 1" - ")"; - return db->Execute(kSql); -} - -bool ThumbnailDatabase::InitFaviconsIndex() { - // Add an index on the url column. - return - db_.Execute("CREATE INDEX IF NOT EXISTS favicons_url ON favicons(url)"); -} - -bool ThumbnailDatabase::InitFaviconBitmapsTable(sql::Connection* db) { - const char kSql[] = - "CREATE TABLE IF NOT EXISTS favicon_bitmaps" - "(" - "id INTEGER PRIMARY KEY," - "icon_id INTEGER NOT NULL," - "last_updated INTEGER DEFAULT 0," - "image_data BLOB," - "width INTEGER DEFAULT 0," - "height INTEGER DEFAULT 0" - ")"; - return db->Execute(kSql); -} - -bool ThumbnailDatabase::InitFaviconBitmapsIndex() { - // Add an index on the icon_id column. - return db_.Execute("CREATE INDEX IF NOT EXISTS favicon_bitmaps_icon_id ON " - "favicon_bitmaps(icon_id)"); -} - bool ThumbnailDatabase::IsFaviconDBStructureIncorrect() { return !db_.IsSQLValid("SELECT id, url, icon_type FROM favicons"); } @@ -938,72 +1242,72 @@ bool ThumbnailDatabase::RetainDataForPageUrls( } } - { - const char kRenameIconMappingTable[] = - "ALTER TABLE icon_mapping RENAME TO old_icon_mapping"; - const char kCopyIconMapping[] = - "INSERT INTO icon_mapping (page_url, icon_id) " - "SELECT old.page_url, mapping.new_icon_id " - "FROM old_icon_mapping AS old " - "JOIN temp.icon_id_mapping AS mapping " - "ON (old.icon_id = mapping.old_icon_id)"; - const char kDropOldIconMappingTable[] = "DROP TABLE old_icon_mapping"; - if (!db_.Execute(kRenameIconMappingTable) || - !InitIconMappingTable(&db_) || - !db_.Execute(kCopyIconMapping) || - !db_.Execute(kDropOldIconMappingTable)) { - return false; - } + const char kRenameIconMappingTable[] = + "ALTER TABLE icon_mapping RENAME TO old_icon_mapping"; + const char kCopyIconMapping[] = + "INSERT INTO icon_mapping (page_url, icon_id) " + "SELECT old.page_url, mapping.new_icon_id " + "FROM old_icon_mapping AS old " + "JOIN temp.icon_id_mapping AS mapping " + "ON (old.icon_id = mapping.old_icon_id)"; + const char kDropOldIconMappingTable[] = "DROP TABLE old_icon_mapping"; + + const char kRenameFaviconsTable[] = + "ALTER TABLE favicons RENAME TO old_favicons"; + const char kCopyFavicons[] = + "INSERT INTO favicons (id, url, icon_type) " + "SELECT mapping.new_icon_id, old.url, old.icon_type " + "FROM old_favicons AS old " + "JOIN temp.icon_id_mapping AS mapping " + "ON (old.id = mapping.old_icon_id)"; + const char kDropOldFaviconsTable[] = "DROP TABLE old_favicons"; + + const char kRenameFaviconBitmapsTable[] = + "ALTER TABLE favicon_bitmaps RENAME TO old_favicon_bitmaps"; + const char kCopyFaviconBitmaps[] = + "INSERT INTO favicon_bitmaps " + " (icon_id, last_updated, image_data, width, height) " + "SELECT mapping.new_icon_id, old.last_updated, " + " old.image_data, old.width, old.height " + "FROM old_favicon_bitmaps AS old " + "JOIN temp.icon_id_mapping AS mapping " + "ON (old.icon_id = mapping.old_icon_id)"; + const char kDropOldFaviconBitmapsTable[] = + "DROP TABLE old_favicon_bitmaps"; + + // Rename existing tables to new location. + if (!db_.Execute(kRenameIconMappingTable) || + !db_.Execute(kRenameFaviconsTable) || + !db_.Execute(kRenameFaviconBitmapsTable)) { + return false; } - { - const char kRenameFaviconsTable[] = - "ALTER TABLE favicons RENAME TO old_favicons"; - const char kCopyFavicons[] = - "INSERT INTO favicons (id, url, icon_type) " - "SELECT mapping.new_icon_id, old.url, old.icon_type " - "FROM old_favicons AS old " - "JOIN temp.icon_id_mapping AS mapping " - "ON (old.id = mapping.old_icon_id)"; - const char kDropOldFaviconsTable[] = "DROP TABLE old_favicons"; - if (!db_.Execute(kRenameFaviconsTable) || - !InitFaviconsTable(&db_) || - !db_.Execute(kCopyFavicons) || - !db_.Execute(kDropOldFaviconsTable)) { - return false; - } - } + // Initialize the replacement tables. At this point the old indices + // still exist (pointing to the old_* tables), so do not initialize + // the indices. + if (!InitTables(&db_)) + return false; - { - const char kRenameFaviconBitmapsTable[] = - "ALTER TABLE favicon_bitmaps RENAME TO old_favicon_bitmaps"; - const char kCopyFaviconBitmaps[] = - "INSERT INTO favicon_bitmaps " - " (icon_id, last_updated, image_data, width, height) " - "SELECT mapping.new_icon_id, old.last_updated, " - " old.image_data, old.width, old.height " - "FROM old_favicon_bitmaps AS old " - "JOIN temp.icon_id_mapping AS mapping " - "ON (old.icon_id = mapping.old_icon_id)"; - const char kDropOldFaviconBitmapsTable[] = - "DROP TABLE old_favicon_bitmaps"; - if (!db_.Execute(kRenameFaviconBitmapsTable) || - !InitFaviconBitmapsTable(&db_) || - !db_.Execute(kCopyFaviconBitmaps) || - !db_.Execute(kDropOldFaviconBitmapsTable)) { - return false; - } + // Copy all of the data over. + if (!db_.Execute(kCopyIconMapping) || + !db_.Execute(kCopyFavicons) || + !db_.Execute(kCopyFaviconBitmaps)) { + return false; } - // Renaming the tables adjusts the indices to reference the new - // name, BUT DOES NOT RENAME THE INDICES. The DROP will drop the - // indices, now re-create them against the new tables. - if (!InitIconMappingIndex() || - !InitFaviconsIndex() || - !InitFaviconBitmapsIndex()) { + // Drop the old_* tables, which also drops the indices. + if (!db_.Execute(kDropOldIconMappingTable) || + !db_.Execute(kDropOldFaviconsTable) || + !db_.Execute(kDropOldFaviconBitmapsTable)) { return false; } + // Recreate the indices. + // TODO(shess): UNIQUE indices could fail due to duplication. This + // could happen in case of corruption. + if (!InitIndices(&db_)) + return false; + const char kIconMappingDrop[] = "DROP TABLE temp.icon_id_mapping"; if (!db_.Execute(kIconMappingDrop)) return false; @@ -1011,26 +1315,6 @@ bool ThumbnailDatabase::RetainDataForPageUrls( return transaction.Commit(); } -bool ThumbnailDatabase::InitIconMappingTable(sql::Connection* db) { - const char kSql[] = - "CREATE TABLE IF NOT EXISTS icon_mapping" - "(" - "id INTEGER PRIMARY KEY," - "page_url LONGVARCHAR NOT NULL," - "icon_id INTEGER" - ")"; - return db->Execute(kSql); -} - -bool ThumbnailDatabase::InitIconMappingIndex() { - // Add an index on the url column. - return - db_.Execute("CREATE INDEX IF NOT EXISTS icon_mapping_page_url_idx" - " ON icon_mapping(page_url)") && - db_.Execute("CREATE INDEX IF NOT EXISTS icon_mapping_icon_id_idx" - " ON icon_mapping(icon_id)"); -} - IconMappingID ThumbnailDatabase::AddIconMapping(const GURL& page_url, chrome::FaviconID icon_id) { const char kSql[] = diff --git a/chrome/browser/history/thumbnail_database.h b/chrome/browser/history/thumbnail_database.h index 6db3b9c..0c8d230 100644 --- a/chrome/browser/history/thumbnail_database.h +++ b/chrome/browser/history/thumbnail_database.h @@ -286,41 +286,6 @@ class ThumbnailDatabase { // Return whether the migration succeeds. bool MigrateIconMappingData(URLDatabase* url_db); - // Creates the favicon table, returning true if the table already exists, - // or was successfully created. - // |db| is the connection to use for initializing the table. - // A different connection is used in RenameAndDropThumbnails, when we - // need to copy the favicons between two database files. - bool InitFaviconsTable(sql::Connection* db); - - // Creates the index over the favicon table. This will be called during - // initialization after the table is created. This is a separate function - // because it is used by SwapFaviconTables to create an index over the - // newly-renamed favicons table (formerly the temporary table with no index). - bool InitFaviconsIndex(); - - // Creates the favicon_bitmaps table, return true if the table already exists - // or was successfully created. - bool InitFaviconBitmapsTable(sql::Connection* db); - - // Creates the index over the favicon_bitmaps table. This will be called - // during initialization after the table is created. This is a separate - // function because it is used by CommitTemporaryTables to create an - // index over the newly-renamed favicon_bitmaps table (formerly the temporary - // table with no index). - bool InitFaviconBitmapsIndex(); - - // Creates the icon_map table, return true if the table already exists or was - // successfully created. - bool InitIconMappingTable(sql::Connection* db); - - // Creates the index over the icon_mapping table, This will be called during - // initialization after the table is created. This is a separate function - // because it is used by CommitTemporaryIconMappingTable to create an index - // over the newly-renamed icon_mapping table (formerly the temporary table - // with no index). - bool InitIconMappingIndex(); - // Returns true if the |favicons| database is missing a column. bool IsFaviconDBStructureIncorrect(); diff --git a/chrome/browser/history/thumbnail_database_unittest.cc b/chrome/browser/history/thumbnail_database_unittest.cc index 6d420b0..30e347a 100644 --- a/chrome/browser/history/thumbnail_database_unittest.cc +++ b/chrome/browser/history/thumbnail_database_unittest.cc @@ -6,6 +6,7 @@ #include <vector> #include "base/basictypes.h" +#include "base/file_util.h" #include "base/files/file_path.h" #include "base/files/scoped_temp_dir.h" #include "base/memory/ref_counted_memory.h" @@ -17,8 +18,10 @@ #include "chrome/test/base/testing_profile.h" #include "sql/connection.h" #include "sql/statement.h" +#include "sql/test/scoped_error_ignorer.h" #include "sql/test/test_helpers.h" #include "testing/gtest/include/gtest/gtest.h" +#include "third_party/sqlite/sqlite3.h" #include "url/gurl.h" namespace history { @@ -57,6 +60,49 @@ WARN_UNUSED_RESULT bool CreateDatabaseFromSQL(const base::FilePath &db_path, return sql::test::CreateDatabaseFromSQL(db_path, sql_path); } +int GetPageSize(sql::Connection* db) { + sql::Statement s(db->GetUniqueStatement("PRAGMA page_size")); + EXPECT_TRUE(s.Step()); + return s.ColumnInt(0); +} + +// Get |name|'s root page number in the database. +int GetRootPage(sql::Connection* db, const char* name) { + const char kPageSql[] = "SELECT rootpage FROM sqlite_master WHERE name = ?"; + sql::Statement s(db->GetUniqueStatement(kPageSql)); + s.BindString(0, name); + EXPECT_TRUE(s.Step()); + return s.ColumnInt(0); +} + +// Helper to read a SQLite page into a buffer. |page_no| is 1-based +// per SQLite usage. +bool ReadPage(const base::FilePath& path, size_t page_no, + char* buf, size_t page_size) { + file_util::ScopedFILE file(file_util::OpenFile(path, "rb")); + if (!file.get()) + return false; + if (0 != fseek(file.get(), (page_no - 1) * page_size, SEEK_SET)) + return false; + if (1u != fread(buf, page_size, 1, file.get())) + return false; + return true; +} + +// Helper to write a SQLite page into a buffer. |page_no| is 1-based +// per SQLite usage. +bool WritePage(const base::FilePath& path, size_t page_no, + const char* buf, size_t page_size) { + file_util::ScopedFILE file(file_util::OpenFile(path, "rb+")); + if (!file.get()) + return false; + if (0 != fseek(file.get(), (page_no - 1) * page_size, SEEK_SET)) + return false; + if (1u != fwrite(buf, page_size, 1, file.get())) + return false; + return true; +} + // Verify that the up-to-date database has the expected tables and // columns. Functional tests only check whether the things which // should be there are, but do not check if extraneous items are @@ -792,4 +838,170 @@ TEST_F(ThumbnailDatabaseTest, Version7) { kIconUrl3, kLargeSize, sizeof(kBlob2), kBlob2)); } +TEST_F(ThumbnailDatabaseTest, Recovery) { + chrome::FaviconID id1, id2; + GURL page_url1("http://www.google.com"); + GURL page_url2("http://news.google.com"); + GURL favicon_url("http://www.google.com/favicon.png"); + + // Create an example database. + // TODO(shess): Merge with the load-dump code when that lands. + { + ThumbnailDatabase db; + ASSERT_EQ(sql::INIT_OK, db.Init(file_name_, NULL, NULL)); + db.BeginTransaction(); + + std::vector<unsigned char> data(kBlob1, kBlob1 + sizeof(kBlob1)); + scoped_refptr<base::RefCountedBytes> favicon( + new base::RefCountedBytes(data)); + + id1 = db.AddFavicon(favicon_url, chrome::TOUCH_ICON); + base::Time time = base::Time::Now(); + db.AddFaviconBitmap(id1, favicon, time, kSmallSize); + db.AddFaviconBitmap(id1, favicon, time, kLargeSize); + EXPECT_LT(0, db.AddIconMapping(page_url1, id1)); + EXPECT_LT(0, db.AddIconMapping(page_url2, id1)); + + id2 = db.AddFavicon(favicon_url, chrome::FAVICON); + EXPECT_NE(id1, id2); + db.AddFaviconBitmap(id2, favicon, time, kSmallSize); + EXPECT_LT(0, db.AddIconMapping(page_url1, id2)); + db.CommitTransaction(); + } + + // Test that the contents make sense after clean open. + { + ThumbnailDatabase db; + ASSERT_EQ(sql::INIT_OK, db.Init(file_name_, NULL, NULL)); + + std::vector<IconMapping> icon_mappings; + EXPECT_TRUE(db.GetIconMappingsForPageURL(page_url1, &icon_mappings)); + ASSERT_EQ(2u, icon_mappings.size()); + EXPECT_EQ(id1, icon_mappings[0].icon_id); + EXPECT_EQ(id2, icon_mappings[1].icon_id); + } + + { + sql::Connection raw_db; + EXPECT_TRUE(raw_db.Open(file_name_)); + { + sql::Statement statement( + raw_db.GetUniqueStatement("PRAGMA integrity_check")); + EXPECT_TRUE(statement.Step()); + ASSERT_EQ("ok", statement.ColumnString(0)); + } + + const char kIndexName[] = "icon_mapping_page_url_idx"; + const int idx_root_page = GetRootPage(&raw_db, kIndexName); + const int page_size = GetPageSize(&raw_db); + scoped_ptr<char[]> buf(new char[page_size]); + EXPECT_TRUE(ReadPage(file_name_, idx_root_page, buf.get(), page_size)); + + { + const char kDeleteSql[] = "DELETE FROM icon_mapping WHERE page_url = ?"; + sql::Statement statement(raw_db.GetUniqueStatement(kDeleteSql)); + statement.BindString(0, URLDatabase::GURLToDatabaseURL(page_url2)); + EXPECT_TRUE(statement.Run()); + } + raw_db.Close(); + + EXPECT_TRUE(WritePage(file_name_, idx_root_page, buf.get(), page_size)); + } + + // Database should be corrupt. + { + sql::Connection raw_db; + EXPECT_TRUE(raw_db.Open(file_name_)); + sql::Statement statement( + raw_db.GetUniqueStatement("PRAGMA integrity_check")); + EXPECT_TRUE(statement.Step()); + ASSERT_NE("ok", statement.ColumnString(0)); + } + + // Open the database and access the corrupt index. + { + sql::ScopedErrorIgnorer ignore_errors; + ignore_errors.IgnoreError(SQLITE_CORRUPT); + ThumbnailDatabase db; + ASSERT_EQ(sql::INIT_OK, db.Init(file_name_, NULL, NULL)); + + // Data for page_url2 was deleted, but the index entry remains, + // this will throw SQLITE_CORRUPT. The corruption handler will + // recover the database and poison the handle, so the outer call + // fails. + std::vector<IconMapping> icon_mappings; + EXPECT_FALSE(db.GetIconMappingsForPageURL(page_url2, &icon_mappings)); + ASSERT_TRUE(ignore_errors.CheckIgnoredErrors()); + } + + // Check that the database is recovered at a SQLite level. + { + sql::Connection raw_db; + EXPECT_TRUE(raw_db.Open(file_name_)); + sql::Statement statement( + raw_db.GetUniqueStatement("PRAGMA integrity_check")); + EXPECT_TRUE(statement.Step()); + EXPECT_EQ("ok", statement.ColumnString(0)); + } + + // Database should also be recovered at higher levels. + { + ThumbnailDatabase db; + ASSERT_EQ(sql::INIT_OK, db.Init(file_name_, NULL, NULL)); + + std::vector<IconMapping> icon_mappings; + + EXPECT_FALSE(db.GetIconMappingsForPageURL(page_url2, &icon_mappings)); + + EXPECT_TRUE(db.GetIconMappingsForPageURL(page_url1, &icon_mappings)); + ASSERT_EQ(2u, icon_mappings.size()); + EXPECT_EQ(id1, icon_mappings[0].icon_id); + EXPECT_EQ(id2, icon_mappings[1].icon_id); + } + + // Corrupt the database again by making the actual file shorter than + // the header expects. + { + int64 db_size = 0; + EXPECT_TRUE(file_util::GetFileSize(file_name_, &db_size)); + { + sql::Connection raw_db; + EXPECT_TRUE(raw_db.Open(file_name_)); + EXPECT_TRUE(raw_db.Execute("CREATE TABLE t(x)")); + } + file_util::ScopedFILE file(file_util::OpenFile(file_name_, "rb+")); + ASSERT_TRUE(file.get() != NULL); + EXPECT_EQ(0, fseek(file.get(), static_cast<long>(db_size), SEEK_SET)); + EXPECT_TRUE(file_util::TruncateFile(file.get())); + } + + // Database is unusable at the SQLite level. + { + sql::ScopedErrorIgnorer ignore_errors; + ignore_errors.IgnoreError(SQLITE_CORRUPT); + sql::Connection raw_db; + EXPECT_TRUE(raw_db.Open(file_name_)); + EXPECT_FALSE(raw_db.IsSQLValid("PRAGMA integrity_check")); + ASSERT_TRUE(ignore_errors.CheckIgnoredErrors()); + } + + // Database should be recovered during open. + { + sql::ScopedErrorIgnorer ignore_errors; + ignore_errors.IgnoreError(SQLITE_CORRUPT); + ThumbnailDatabase db; + ASSERT_EQ(sql::INIT_OK, db.Init(file_name_, NULL, NULL)); + + std::vector<IconMapping> icon_mappings; + + EXPECT_FALSE(db.GetIconMappingsForPageURL(page_url2, &icon_mappings)); + + EXPECT_TRUE(db.GetIconMappingsForPageURL(page_url1, &icon_mappings)); + ASSERT_EQ(2u, icon_mappings.size()); + EXPECT_EQ(id1, icon_mappings[0].icon_id); + EXPECT_EQ(id2, icon_mappings[1].icon_id); + ASSERT_TRUE(ignore_errors.CheckIgnoredErrors()); + } +} + } // namespace history diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 04d0ddd..25d9868 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -307,6 +307,7 @@ '../ipc/ipc.gyp:test_support_ipc', '../media/media.gyp:media_test_support', '../ppapi/ppapi_internal.gyp:ppapi_shared', + '../sql/sql.gyp:test_support_sql', '../third_party/leveldatabase/leveldatabase.gyp:leveldatabase', '../webkit/glue/webkit_glue.gyp:glue_child', ], diff --git a/sql/connection.cc b/sql/connection.cc index 097edd7..0b49786 100644 --- a/sql/connection.cc +++ b/sql/connection.cc @@ -133,7 +133,7 @@ namespace sql { Connection::ErrorIgnorerCallback* Connection::current_ignorer_cb_ = NULL; // static -bool Connection::ShouldIgnore(int error) { +bool Connection::ShouldIgnoreSqliteError(int error) { if (!current_ignorer_cb_) return false; return current_ignorer_cb_->Run(error); @@ -1040,7 +1040,7 @@ int Connection::OnSqliteError(int err, sql::Statement *stmt) { } // The default handling is to assert on debug and to ignore on release. - if (!ShouldIgnore(err)) + if (!ShouldIgnoreSqliteError(err)) DLOG(FATAL) << GetErrorMessage(); return err; } diff --git a/sql/connection.h b/sql/connection.h index 7938606..5475a84 100644 --- a/sql/connection.h +++ b/sql/connection.h @@ -394,6 +394,12 @@ class SQL_EXPORT Connection { // SELECT type, name, tbl_name, sql FROM sqlite_master ORDER BY 1, 2, 3, 4; std::string GetSchema() const; + // Clients which provide an error_callback don't see the + // error-handling at the end of OnSqliteError(). Expose to allow + // those clients to work appropriately with ScopedErrorIgnorer in + // tests. + static bool ShouldIgnoreSqliteError(int error); + private: // For recovery module. friend class Recovery; @@ -436,7 +442,6 @@ class SQL_EXPORT Connection { // See test/scoped_error_ignorer.h. typedef base::Callback<bool(int)> ErrorIgnorerCallback; static ErrorIgnorerCallback* current_ignorer_cb_; - static bool ShouldIgnore(int error); static void SetErrorIgnorer(ErrorIgnorerCallback* ignorer); static void ResetErrorIgnorer(); diff --git a/sql/recovery.cc b/sql/recovery.cc index 9016f8a..fc67661 100644 --- a/sql/recovery.cc +++ b/sql/recovery.cc @@ -37,6 +37,13 @@ void Recovery::Unrecoverable(scoped_ptr<Recovery> r) { // ~Recovery() will RAZE_AND_POISON. } +// static +void Recovery::Rollback(scoped_ptr<Recovery> r) { + // TODO(shess): HISTOGRAM to track? Or just have people crash out? + // Crash and dump? + r->Shutdown(POISON); +} + Recovery::Recovery(Connection* connection) : db_(connection), recover_db_() { @@ -69,6 +76,18 @@ bool Recovery::Init(const base::FilePath& db_path) { // more complicated. db_->RollbackAllTransactions(); + // Disable exclusive locking mode so that the attached database can + // access things. The locking_mode change is not active until the + // next database access, so immediately force an access. Enabling + // writable_schema allows processing through certain kinds of + // corruption. + // TODO(shess): It would be better to just close the handle, but it + // is necessary for the final backup which rewrites things. It + // might be reasonable to close then re-open the handle. + ignore_result(db_->Execute("PRAGMA writable_schema=1")); + ignore_result(db_->Execute("PRAGMA locking_mode=NORMAL")); + ignore_result(db_->Execute("SELECT COUNT(*) FROM sqlite_master")); + if (!recover_db_.OpenTemporary()) return false; diff --git a/sql/recovery.h b/sql/recovery.h index c0bb6da..e832da6 100644 --- a/sql/recovery.h +++ b/sql/recovery.h @@ -64,7 +64,7 @@ class SQL_EXPORT Recovery { // If Recovered() is not called, the destructor will call // Unrecoverable(). // - // TODO(shess): At this time, this function an fail while leaving + // TODO(shess): At this time, this function can fail while leaving // the original database intact. Figure out which failure cases // should go to RazeAndClose() instead. static bool Recovered(scoped_ptr<Recovery> r) WARN_UNUSED_RESULT; @@ -73,6 +73,14 @@ class SQL_EXPORT Recovery { // database is razed, and the handle poisoned. static void Unrecoverable(scoped_ptr<Recovery> r); + // When initially developing recovery code, sometimes the possible + // database states are not well-understood without further + // diagnostics. Abandon recovery but do not raze the original + // database. + // NOTE(shess): Only call this when adding recovery support. In the + // steady state, all databases should progress to recovered or razed. + static void Rollback(scoped_ptr<Recovery> r); + // Handle to the temporary recovery database. sql::Connection* db() { return &recover_db_; } |