AppCache: Run a quick integrity check on the sqlite database when opening. 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

7 files changed, 117 insertions, 11 deletions

diff --git a/chrome/browser/history/thumbnail_database.cc b/chrome/browser/history/thumbnail_database.cc
index 1cb899e..ce9cc06 100644
--- a/chrome/browser/history/thumbnail_database.cc
+++ b/chrome/browser/history/thumbnail_database.cc
@@ -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());
diff --git a/sql/connection.cc b/sql/connection.cc
index 6e02cd7..0caa9ed 100644
--- a/sql/connection.cc
+++ b/sql/connection.cc
@@ -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
@@ -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,
diff --git a/sql/connection.h b/sql/connection.h
index ec76e3b..54c3f8b 100644
--- a/sql/connection.h
+++ b/sql/connection.h
@@ -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 ------------------------------------------------------------
 
@@ -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_;
diff --git a/sql/connection_unittest.cc b/sql/connection_unittest.cc
index 5aa7a9b..304ddcd 100644
--- a/sql/connection_unittest.cc
+++ b/sql/connection_unittest.cc
@@ -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
diff --git a/webkit/browser/appcache/appcache_database.cc b/webkit/browser/appcache/appcache_database.cc
index a3dadd9..0980973 100644
--- a/webkit/browser/appcache/appcache_database.cc
+++ b/webkit/browser/appcache/appcache_database.cc
@@ -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);
diff --git a/webkit/browser/appcache/appcache_database.h b/webkit/browser/appcache/appcache_database.h
index fedd068..d374cfd 100644
--- a/webkit/browser/appcache/appcache_database.h
+++ b/webkit/browser/appcache/appcache_database.h
@@ -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);
diff --git a/webkit/browser/appcache/appcache_database_unittest.cc b/webkit/browser/appcache/appcache_database_unittest.cc
index 71e4a08..b5f4e40 100644
--- a/webkit/browser/appcache/appcache_database_unittest.cc
+++ b/webkit/browser/appcache/appcache_database_unittest.cc
@@ -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"
@@ -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");