[sql] Recovery code for "Top Sites" database.

Port recovery code from thumbnail_database.cc to
top_sites_database.cc.  Also retry Init() in case the recovery code
fixes things (a quarter to a third of databases have problems detected
during open).

Extend sql::Recovery::AutoRecoverTable() to handle DOUBLE columns.

Abstract out sql::test::CorruptTableOrIndex() to ease writing
corruption tests.

IWYU pass on various files.

BUG=321852

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239377 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 147 insertions, 86 deletions

diff --git a/sql/recovery.cc b/sql/recovery.cc
index 89abbc9..42f1a9a 100644
--- a/sql/recovery.cc
+++ b/sql/recovery.cc
@@ -382,6 +382,12 @@ bool Recovery::AutoRecoverTable(const char* table_name,
     // SQLite's affinity detection is documented at:
     // http://www.sqlite.org/datatype3.html#affname
     // The gist of it is that CHAR, TEXT, and INT use substring matches.
+    // TODO(shess): It would be nice to unit test the type handling,
+    // but it is not obvious to me how to write a test which would
+    // fail appropriately when something was broken.  It would have to
+    // somehow use data which would allow detecting the various type
+    // coercions which happen.  If STRICT could be enabled, type
+    // mismatches could be detected by which rows are filtered.
     if (column_type.find("INT") != std::string::npos) {
       if (pk_column == 1) {
         rowid_ofs = create_column_decls.size();
@@ -393,12 +399,13 @@ bool Recovery::AutoRecoverTable(const char* table_name,
       column_decl += " TEXT";
     } else if (column_type == "BLOB") {
       column_decl += " BLOB";
+    } else if (column_type.find("DOUB") != std::string::npos) {
+      column_decl += " FLOAT";
     } else {
       // TODO(shess): AFAICT, there remain:
       // - contains("CLOB") -> TEXT
-      // - contains("REAL") -> REAL
-      // - contains("FLOA") -> REAL
-      // - contains("DOUB") -> REAL
+      // - contains("REAL") -> FLOAT
+      // - contains("FLOA") -> FLOAT
       // - other -> "NUMERIC"
       // Just code those in as they come up.
       NOTREACHED() << " Unsupported type " << column_type;
diff --git a/sql/recovery.h b/sql/recovery.h
index 2475b0f..60fb674 100644
--- a/sql/recovery.h
+++ b/sql/recovery.h
@@ -27,10 +27,28 @@ namespace sql {
 //   scoped_ptr<sql::Recovery> r =
 //       sql::Recovery::Begin(orig_db, orig_db_path);
 //   if (r) {
-//     if (r.db()->Execute(kCreateSchemaSql) &&
-//         r.db()->Execute(kCopyDataFromOrigSql)) {
-//       sql::Recovery::Recovered(r.Pass());
+//     // Create the schema to recover to.  On failure, clear the
+//     // database.
+//     if (!r.db()->Execute(kCreateSchemaSql)) {
+//       sql::Recovery::Unrecoverable(r.Pass());
+//       return;
 //     }
+//
+//     // Recover data in "mytable".
+//     size_t rows_recovered = 0;
+//     if (!r.AutoRecoverTable("mytable", 0, &rows_recovered)) {
+//       sql::Recovery::Unrecoverable(r.Pass());
+//       return;
+//     }
+//
+//     // Manually cleanup additional constraints.
+//     if (!r.db()->Execute(kCleanupSql)) {
+//       sql::Recovery::Unrecoverable(r.Pass());
+//       return;
+//     }
+//
+//     // Commit the recovered data to the original database file.
+//     sql::Recovery::Recovered(r.Pass());
 //   }
 // }
 //
diff --git a/sql/recovery_unittest.cc b/sql/recovery_unittest.cc
index e3c9738..ce3884b 100644
--- a/sql/recovery_unittest.cc
+++ b/sql/recovery_unittest.cc
@@ -2,17 +2,18 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <string>
+
 #include "base/bind.h"
-#include "base/file_util.h"
+#include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
-#include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
-#include "base/strings/stringprintf.h"
 #include "sql/connection.h"
 #include "sql/meta_table.h"
 #include "sql/recovery.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"
 
@@ -57,51 +58,6 @@ std::string GetSchema(sql::Connection* db) {
   return ExecuteWithResults(db, kSql, "|", "\n");
 }
 
-#if !defined(USE_SYSTEM_SQLITE)
-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(base::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(base::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;
-}
-#endif  // !defined(USE_SYSTEM_SQLITE)
-
 class SQLRecoveryTest : public testing::Test {
  public:
   SQLRecoveryTest() {}
@@ -296,24 +252,12 @@ TEST_F(SQLRecoveryTest, RecoverCorruptIndex) {
 
     ASSERT_TRUE(db().CommitTransaction());
   }
-
-
-  // Capture the index's root page into |buf|.
-  int index_page = GetRootPage(&db(), "x_id");
-  int page_size = GetPageSize(&db());
-  scoped_ptr<char[]> buf(new char[page_size]);
-  ASSERT_TRUE(ReadPage(db_path(), index_page, buf.get(), page_size));
-
-  // Delete the row from the table and index.
-  ASSERT_TRUE(db().Execute("DELETE FROM x WHERE id = 0"));
-
-  // Close to clear any cached data.
   db().Close();
 
-  // Put the stale index page back.
-  ASSERT_TRUE(WritePage(db_path(), index_page, buf.get(), page_size));
-
-  // At this point, the index references a value not in the table.
+  // Delete a row from the table, while leaving the index entry which
+  // references it.
+  const char kDeleteSql[] = "DELETE FROM x WHERE id = 0";
+  ASSERT_TRUE(sql::test::CorruptTableOrIndex(db_path(), "x_id", kDeleteSql));
 
   ASSERT_TRUE(Reopen());
 
@@ -368,25 +312,12 @@ TEST_F(SQLRecoveryTest, RecoverCorruptTable) {
 
     ASSERT_TRUE(db().CommitTransaction());
   }
-
-  // Capture the table's root page into |buf|.
-  // Find the page the table is stored on.
-  const int table_page = GetRootPage(&db(), "x");
-  const int page_size = GetPageSize(&db());
-  scoped_ptr<char[]> buf(new char[page_size]);
-  ASSERT_TRUE(ReadPage(db_path(), table_page, buf.get(), page_size));
-
-  // Delete the row from the table and index.
-  ASSERT_TRUE(db().Execute("DELETE FROM x WHERE id = 0"));
-
-  // Close to clear any cached data.
   db().Close();
 
-  // Put the stale table page back.
-  ASSERT_TRUE(WritePage(db_path(), table_page, buf.get(), page_size));
+  // Delete a row from the index while leaving a table entry.
+  const char kDeleteSql[] = "DELETE FROM x WHERE id = 0";
+  ASSERT_TRUE(sql::test::CorruptTableOrIndex(db_path(), "x", kDeleteSql));
 
-  // At this point, the table contains a value not referenced by the
-  // index.
   // TODO(shess): Figure out a query which causes SQLite to notice
   // this organically.  Meanwhile, just handle it manually.
 
diff --git a/sql/test/test_helpers.cc b/sql/test/test_helpers.cc
index 6e12b04..8d56140 100644
--- a/sql/test/test_helpers.cc
+++ b/sql/test/test_helpers.cc
@@ -21,6 +21,26 @@ size_t CountSQLItemsOfType(sql::Connection* db, const char* type) {
   return s.ColumnInt(0);
 }
 
+// Get page size for the database.
+bool GetPageSize(sql::Connection* db, int* page_size) {
+  sql::Statement s(db->GetUniqueStatement("PRAGMA page_size"));
+  if (!s.Step())
+    return false;
+  *page_size = s.ColumnInt(0);
+  return true;
+}
+
+// Get |name|'s root page number in the database.
+bool GetRootPage(sql::Connection* db, const char* name, int* page_number) {
+  const char kPageSql[] = "SELECT rootpage FROM sqlite_master WHERE name = ?";
+  sql::Statement s(db->GetUniqueStatement(kPageSql));
+  s.BindString(0, name);
+  if (!s.Step())
+    return false;
+  *page_number = s.ColumnInt(0);
+  return true;
+}
+
 // Helper for reading a number from the SQLite header.
 // See net/base/big_endian.h.
 unsigned ReadBigEndian(unsigned char* buf, size_t bytes) {
@@ -88,6 +108,70 @@ bool CorruptSizeInHeader(const base::FilePath& db_path) {
   return true;
 }
 
+bool CorruptTableOrIndex(const base::FilePath& db_path,
+                         const char* tree_name,
+                         const char* update_sql) {
+  sql::Connection db;
+  if (!db.Open(db_path))
+    return false;
+
+  int page_size = 0;
+  if (!GetPageSize(&db, &page_size))
+    return false;
+
+  int page_number = 0;
+  if (!GetRootPage(&db, tree_name, &page_number))
+    return false;
+
+  // SQLite uses 1-based page numbering.
+  const long int page_ofs = (page_number - 1) * page_size;
+  scoped_ptr<char[]> page_buf(new char[page_size]);
+
+  // Get the page into page_buf.
+  file_util::ScopedFILE file(base::OpenFile(db_path, "rb+"));
+  if (!file.get())
+    return false;
+  if (0 != fseek(file.get(), page_ofs, SEEK_SET))
+    return false;
+  if (1u != fread(page_buf.get(), page_size, 1, file.get()))
+    return false;
+
+  // Require the page to be a leaf node.  A multilevel tree would be
+  // very hard to restore correctly.
+  if (page_buf[0] != 0xD && page_buf[0] != 0xA)
+    return false;
+
+  // The update has to work, and make changes.
+  if (!db.Execute(update_sql))
+    return false;
+  if (db.GetLastChangeCount() == 0)
+    return false;
+
+  // Ensure that the database is fully flushed.
+  db.Close();
+
+  // Check that the stored page actually changed.  This catches usage
+  // errors where |update_sql| is not related to |tree_name|.
+  scoped_ptr<char[]> check_page_buf(new char[page_size]);
+  // The on-disk data should have changed.
+  if (0 != fflush(file.get()))
+    return false;
+  if (0 != fseek(file.get(), page_ofs, SEEK_SET))
+    return false;
+  if (1u != fread(check_page_buf.get(), page_size, 1, file.get()))
+    return false;
+  if (!memcmp(check_page_buf.get(), page_buf.get(), page_size))
+    return false;
+
+  // Put the original page back.
+  if (0 != fseek(file.get(), page_ofs, SEEK_SET))
+    return false;
+  if (1u != fwrite(page_buf.get(), page_size, 1, file.get()))
+    return false;
+
+  return true;
+}
+
 size_t CountSQLTables(sql::Connection* db) {
   return CountSQLItemsOfType(db, "table");
 }
diff --git a/sql/test/test_helpers.h b/sql/test/test_helpers.h
index b9d5e9b..d9dda22 100644
--- a/sql/test/test_helpers.h
+++ b/sql/test/test_helpers.h
@@ -9,6 +9,7 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
+#include "base/files/file_path.h"
 
 // Collection of test-only convenience functions.
 
@@ -33,6 +34,26 @@ namespace test {
 // Returns false if any error occurs accessing the file.
 bool CorruptSizeInHeader(const base::FilePath& db_path) WARN_UNUSED_RESULT;
 
+// Frequently corruption is a result of failure to atomically update
+// pages in different structures.  For instance, if an index update
+// takes effect but the corresponding table update does not.  This
+// helper restores the prior version of a b-tree root after running an
+// update which changed that b-tree.  The named b-tree must exist and
+// must be a leaf node (either index or table).  Returns true if the
+// on-disk file is successfully modified, and the restored page
+// differs from the updated page.
+//
+// The resulting database should be possible to open, and many
+// statements should work.  SQLITE_CORRUPT will be thrown if a query
+// through the index finds the row missing in the table.
+//
+// TODO(shess): It would be very helpful to allow a parameter to the
+// sql statement.  Perhaps a version with a string parameter would be
+// sufficient, given affinity rules?
+bool CorruptTableOrIndex(const base::FilePath& db_path,
+                         const char* tree_name,
+                         const char* update_sql) WARN_UNUSED_RESULT;
+
 // Return the number of tables in sqlite_master.
 size_t CountSQLTables(sql::Connection* db) WARN_UNUSED_RESULT;