Skip to content

Commit

Permalink
AppCache: Run a quick integrity check on the sqlite database when ope…
Browse files Browse the repository at this point in the history
…ning. If the test fails, delete the appcache directory and start afresh.

NOTRY=true
TBR=sky
BUG=318544

Review URL: https://codereview.chromium.org/104593010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@240937 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
michaeln@chromium.org committed Dec 16, 2013
1 parent 3e0d61e commit 579446c
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 11 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/history/thumbnail_database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ void ReportCorrupt(sql::Connection* db, size_t startup_kb) {
std::vector<std::string> messages;

const base::TimeTicks before = base::TimeTicks::Now();
db->IntegrityCheck(&messages);
db->FullIntegrityCheck(&messages);
base::StringAppendF(&debug_info, "# %" PRIx64 " ms, %" PRIuS " records\n",
(base::TimeTicks::Now() - before).InMilliseconds(),
messages.size());
Expand Down
19 changes: 15 additions & 4 deletions sql/connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1050,9 +1050,21 @@ int Connection::OnSqliteError(int err, sql::Statement *stmt, const char* sql) {
return err;
}

// TODO(shess): Allow specifying integrity_check versus quick_check.
bool Connection::FullIntegrityCheck(std::vector<std::string>* messages) {
return IntegrityCheckHelper("PRAGMA integrity_check", messages);
}

bool Connection::QuickIntegrityCheck() {
std::vector<std::string> messages;
if (!IntegrityCheckHelper("PRAGMA quick_check", &messages))
return false;
return messages.size() == 1 && messages[0] == "ok";
}

// TODO(shess): Allow specifying maximum results (default 100 lines).
bool Connection::IntegrityCheck(std::vector<std::string>* messages) {
bool Connection::IntegrityCheckHelper(
const char* pragma_sql,
std::vector<std::string>* messages) {
messages->clear();

// This has the side effect of setting SQLITE_RecoveryMode, which
Expand All @@ -1065,8 +1077,7 @@ bool Connection::IntegrityCheck(std::vector<std::string>* messages) {

bool ret = false;
{
const char kSql[] = "PRAGMA integrity_check";
sql::Statement stmt(GetUniqueStatement(kSql));
sql::Statement stmt(GetUniqueStatement(pragma_sql));

// The pragma appears to return all results (up to 100 by default)
// as a single string. This doesn't appear to be an API contract,
Expand Down
19 changes: 14 additions & 5 deletions sql/connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,16 @@ class SQL_EXPORT Connection {
// histogram is recorded.
void AddTaggedHistogram(const std::string& name, size_t sample) const;

// Run "PRAGMA integrity_check" and post each line of results into
// |messages|. Returns the success of running the statement - per
// the SQLite documentation, if no errors are found the call should
// succeed, and a single value "ok" should be in messages.
bool IntegrityCheck(std::vector<std::string>* messages);
// Run "PRAGMA integrity_check" and post each line of
// results into |messages|. Returns the success of running the
// statement - per the SQLite documentation, if no errors are found the
// call should succeed, and a single value "ok" should be in messages.
bool FullIntegrityCheck(std::vector<std::string>* messages);

// Runs "PRAGMA quick_check" and, unlike the FullIntegrityCheck method,
// interprets the results returning true if the the statement executes
// without error and results in a single "ok" value.
bool QuickIntegrityCheck() WARN_UNUSED_RESULT;

// Initialization ------------------------------------------------------------

Expand Down Expand Up @@ -540,6 +545,10 @@ class SQL_EXPORT Connection {
// case for const functions).
scoped_refptr<StatementRef> GetUntrackedStatement(const char* sql) const;

bool IntegrityCheckHelper(
const char* pragma_sql,
std::vector<std::string>* messages) WARN_UNUSED_RESULT;

// The actual sqlite database. Will be NULL before Init has been called or if
// Init resulted in an error.
sqlite3* db_;
Expand Down
44 changes: 44 additions & 0 deletions sql/connection_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -822,4 +822,48 @@ TEST_F(SQLConnectionTest, Attach) {
EXPECT_FALSE(db().IsSQLValid("SELECT count(*) from other.bar"));
}

TEST_F(SQLConnectionTest, Basic_QuickIntegrityCheck) {
const char* kCreateSql = "CREATE TABLE foo (id INTEGER PRIMARY KEY, value)";
ASSERT_TRUE(db().Execute(kCreateSql));
EXPECT_TRUE(db().QuickIntegrityCheck());
db().Close();

ASSERT_TRUE(sql::test::CorruptSizeInHeader(db_path()));

{
sql::ScopedErrorIgnorer ignore_errors;
ignore_errors.IgnoreError(SQLITE_CORRUPT);
ASSERT_TRUE(db().Open(db_path()));
EXPECT_FALSE(db().QuickIntegrityCheck());
ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
}
}

TEST_F(SQLConnectionTest, Basic_FullIntegrityCheck) {
const std::string kOk("ok");
std::vector<std::string> messages;

const char* kCreateSql = "CREATE TABLE foo (id INTEGER PRIMARY KEY, value)";
ASSERT_TRUE(db().Execute(kCreateSql));
EXPECT_TRUE(db().FullIntegrityCheck(&messages));
EXPECT_EQ(1u, messages.size());
EXPECT_EQ(kOk, messages[0]);
db().Close();

ASSERT_TRUE(sql::test::CorruptSizeInHeader(db_path()));

{
sql::ScopedErrorIgnorer ignore_errors;
ignore_errors.IgnoreError(SQLITE_CORRUPT);
ASSERT_TRUE(db().Open(db_path()));
EXPECT_TRUE(db().FullIntegrityCheck(&messages));
EXPECT_LT(1u, messages.size());
EXPECT_NE(kOk, messages[0]);
ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
}

// TODO(shess): CorruptTableOrIndex could be used to produce a
// file that would pass the quick check and fail the full check.
}

} // namespace
2 changes: 1 addition & 1 deletion webkit/browser/appcache/appcache_database.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1019,7 +1019,7 @@ bool AppCacheDatabase::LazyOpen(bool create_if_needed) {
db_->Preload();
}

if (!opened || !EnsureDatabaseVersion()) {
if (!opened || !db_->QuickIntegrityCheck() || !EnsureDatabaseVersion()) {
LOG(ERROR) << "Failed to open the appcache database.";
AppCacheHistograms::CountInitResult(
AppCacheHistograms::SQL_DATABASE_ERROR);
Expand Down
1 change: 1 addition & 0 deletions webkit/browser/appcache/appcache_database.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ class WEBKIT_STORAGE_BROWSER_EXPORT AppCacheDatabase {

FRIEND_TEST_ALL_PREFIXES(AppCacheDatabaseTest, CacheRecords);
FRIEND_TEST_ALL_PREFIXES(AppCacheDatabaseTest, EntryRecords);
FRIEND_TEST_ALL_PREFIXES(AppCacheDatabaseTest, QuickIntegrityCheck);
FRIEND_TEST_ALL_PREFIXES(AppCacheDatabaseTest, NamespaceRecords);
FRIEND_TEST_ALL_PREFIXES(AppCacheDatabaseTest, GroupRecords);
FRIEND_TEST_ALL_PREFIXES(AppCacheDatabaseTest, LazyOpen);
Expand Down
41 changes: 41 additions & 0 deletions webkit/browser/appcache/appcache_database_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "sql/meta_table.h"
#include "sql/statement.h"
#include "sql/test/scoped_error_ignorer.h"
#include "sql/test/test_helpers.h"
#include "sql/transaction.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/sqlite/sqlite3.h"
Expand Down Expand Up @@ -73,6 +74,46 @@ TEST(AppCacheDatabaseTest, ReCreate) {
EXPECT_FALSE(base::PathExists(kOtherFile));
}

#ifdef NDEBUG
// Only run in release builds because sql::Connection and familiy
// crank up DLOG(FATAL)'ness and this test presents it with
// intentionally bad data which causes debug builds to exit instead
// of run to completion. In release builds, errors the are delivered
// to the consumer so we can test the error handling of the consumer.
// TODO: crbug/328576
TEST(AppCacheDatabaseTest, QuickIntegrityCheck) {
// Real files on disk for this test too, a corrupt database file.
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
base::FilePath mock_dir = temp_dir.path().AppendASCII("mock");
ASSERT_TRUE(base::CreateDirectory(mock_dir));

const base::FilePath kDbFile = mock_dir.AppendASCII("appcache.db");
const base::FilePath kOtherFile = mock_dir.AppendASCII("other_file");
EXPECT_EQ(3, file_util::WriteFile(kOtherFile, "foo", 3));

// First create a valid db file.
AppCacheDatabase db(kDbFile);
EXPECT_TRUE(db.LazyOpen(true));
EXPECT_TRUE(base::PathExists(kOtherFile));
EXPECT_TRUE(base::PathExists(kDbFile));
db.CloseConnection();

// Break it.
ASSERT_TRUE(sql::test::CorruptSizeInHeader(kDbFile));

// Reopening will notice the corruption and delete/recreate the directory.
{
sql::ScopedErrorIgnorer ignore_errors;
ignore_errors.IgnoreError(SQLITE_CORRUPT);
EXPECT_TRUE(db.LazyOpen(true));
EXPECT_FALSE(base::PathExists(kOtherFile));
EXPECT_TRUE(base::PathExists(kDbFile));
ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
}
}
#endif // NDEBUG

TEST(AppCacheDatabaseTest, ExperimentalFlags) {
const char kExperimentFlagsKey[] = "ExperimentFlags";
std::string kInjectedFlags("exp1,exp2");
Expand Down

0 comments on commit 579446c

Please sign in to comment.