Revert 235492 "[sql] Recover Favicons v5 databases, with more re..."

trunk/src/chrome/browser/history/thumbnail_database.cc

Index: trunk/src/chrome/browser/history/thumbnail_database.cc
===================================================================
--- trunk/src/chrome/browser/history/thumbnail_database.cc	(revision 235594)
+++ trunk/src/chrome/browser/history/thumbnail_database.cc	(working copy)
@@ -275,60 +275,6 @@
   }
 }
 
-// Create v5 schema for recovery code.
-bool InitSchemaV5(sql::Connection* db) {
-  // This schema was derived from the strings used when v5 was in
-  // force.  The [favicons] index and the [icon_mapping] items were
-  // copied from the current strings, after verifying that the
-  // resulting schema exactly matches the schema created by the
-  // original versions of those strings.  This allows the linker to
-  // share the strings if they match, while preferring correctness of
-  // the current versions change.
-
-  const char kFaviconsV5[] =
-      "CREATE TABLE IF NOT EXISTS favicons("
-      "id INTEGER PRIMARY KEY,"
-      "url LONGVARCHAR NOT NULL,"
-      "last_updated INTEGER DEFAULT 0,"
-      "image_data BLOB,"
-      "icon_type INTEGER DEFAULT 1,"
-      "sizes LONGVARCHAR"
-      ")";
-  const char kFaviconsIndexV5[] =
-      "CREATE INDEX IF NOT EXISTS favicons_url ON favicons(url)";
-  if (!db->Execute(kFaviconsV5) || !db->Execute(kFaviconsIndexV5))
-    return false;
-
-  const char kIconMappingV5[] =
-      "CREATE TABLE IF NOT EXISTS icon_mapping"
-      "("
-      "id INTEGER PRIMARY KEY,"
-      "page_url LONGVARCHAR NOT NULL,"
-      "icon_id INTEGER"
-      ")";
-  const char kIconMappingUrlIndexV5[] =
-      "CREATE INDEX IF NOT EXISTS icon_mapping_page_url_idx"
-      " ON icon_mapping(page_url)";
-  const char kIconMappingIdIndexV5[] =
-      "CREATE INDEX IF NOT EXISTS icon_mapping_icon_id_idx"
-      " ON icon_mapping(icon_id)";
-  if (!db->Execute(kIconMappingV5) ||
-      !db->Execute(kIconMappingUrlIndexV5) ||
-      !db->Execute(kIconMappingIdIndexV5)) {
-    return false;
-  }
-
-  return true;
-}
-
-// TODO(shess): Consider InitSchemaV7().  InitSchemaV5() is worthwhile
-// because there appear to be 10s of thousands of marooned v5
-// databases in the wild.  Once recovery reaches stable, the number of
-// corrupt-but-recoverable databases should drop, possibly to the
-// point where it is not worthwhile to maintain previous-version
-// recovery code.
-// TODO(shess): Alternately, think on a way to more cleanly represent
-// versioned schema going forward.
 bool InitTables(sql::Connection* db) {
   const char kIconMappingSql[] =
       "CREATE TABLE IF NOT EXISTS icon_mapping"
@@ -396,31 +342,22 @@
 enum RecoveryEventType {
   RECOVERY_EVENT_RECOVERED = 0,
   RECOVERY_EVENT_FAILED_SCOPER,
-  RECOVERY_EVENT_FAILED_META_VERSION_ERROR,  // obsolete
-  RECOVERY_EVENT_FAILED_META_VERSION_NONE,  // obsolete
+  RECOVERY_EVENT_FAILED_META_VERSION_ERROR,
+  RECOVERY_EVENT_FAILED_META_VERSION_NONE,
   RECOVERY_EVENT_FAILED_META_WRONG_VERSION6,  // obsolete
-  RECOVERY_EVENT_FAILED_META_WRONG_VERSION5,  // obsolete
+  RECOVERY_EVENT_FAILED_META_WRONG_VERSION5,
   RECOVERY_EVENT_FAILED_META_WRONG_VERSION,
-  RECOVERY_EVENT_FAILED_RECOVER_META,  // obsolete
+  RECOVERY_EVENT_FAILED_RECOVER_META,
   RECOVERY_EVENT_FAILED_META_INSERT,  // obsolete
   RECOVERY_EVENT_FAILED_INIT,
-  RECOVERY_EVENT_FAILED_RECOVER_FAVICONS,  // obsolete
-  RECOVERY_EVENT_FAILED_FAVICONS_INSERT,  // obsolete
-  RECOVERY_EVENT_FAILED_RECOVER_FAVICON_BITMAPS,  // obsolete
-  RECOVERY_EVENT_FAILED_FAVICON_BITMAPS_INSERT,  // obsolete
-  RECOVERY_EVENT_FAILED_RECOVER_ICON_MAPPING,  // obsolete
-  RECOVERY_EVENT_FAILED_ICON_MAPPING_INSERT,  // obsolete
+  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,
   RECOVERY_EVENT_RECOVERED_VERSION6,
   RECOVERY_EVENT_FAILED_META_INIT,
-  RECOVERY_EVENT_FAILED_META_VERSION,
-  RECOVERY_EVENT_DEPRECATED,
-  RECOVERY_EVENT_FAILED_V5_INITSCHEMA,
-  RECOVERY_EVENT_FAILED_V5_AUTORECOVER_FAVICONS,
-  RECOVERY_EVENT_FAILED_V5_AUTORECOVER_ICON_MAPPING,
-  RECOVERY_EVENT_RECOVERED_VERSION5,
-  RECOVERY_EVENT_FAILED_AUTORECOVER_FAVICONS,
-  RECOVERY_EVENT_FAILED_AUTORECOVER_FAVICON_BITMAPS,
-  RECOVERY_EVENT_FAILED_AUTORECOVER_ICON_MAPPING,
 
   // Always keep this at the end.
   RECOVERY_EVENT_MAX,
@@ -468,106 +405,77 @@
     return;
   }
 
-  // Setup the meta recovery table and fetch the version number from
-  // the corrupt database.
-  int version = 0;
-  if (!recovery->SetupMeta() || !recovery->GetMetaVersionNumber(&version)) {
-    // TODO(shess): Prior histograms indicate all failures are in
-    // creating the recover virtual table for corrupt.meta.  The table
-    // may not exist, or the database may be too far gone.  Either
-    // way, unclear how to resolve.
-    sql::Recovery::Rollback(recovery.Pass());
-    RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_VERSION);
-    return;
-  }
-
-  // Recover v5 database to v5 schema.  Next pass through Init() will
-  // migrate to v7.
-  if (version == 5) {
-    sql::MetaTable recover_meta_table;
-    if (!recover_meta_table.Init(recovery->db(), version, version)) {
+  // 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.
+  int version = 0;  // For reporting which version was recovered.
+  {
+    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_META_INIT);
+      RecordRecoveryEvent(RECOVERY_EVENT_FAILED_RECOVER_META);
       return;
     }
 
-    // TODO(shess): These tests are separate for histogram purposes,
-    // but once things look stable it can be tightened up.
-    if (!InitSchemaV5(recovery->db())) {
-      sql::Recovery::Rollback(recovery.Pass());
-      RecordRecoveryEvent(RECOVERY_EVENT_FAILED_V5_INITSCHEMA);
-      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;
+      }
+      version = recovery_version.ColumnInt(0);
 
-    if (!recovery->AutoRecoverTable("favicons", 0, &favicons_rows_recovered)) {
-      sql::Recovery::Rollback(recovery.Pass());
-      RecordRecoveryEvent(RECOVERY_EVENT_FAILED_V5_AUTORECOVER_FAVICONS);
-      return;
+      // Recovery code is generally schema-dependent.  Version 7 and
+      // version 6 are very similar, so can be handled together.
+      // Track version 5, to see whether it's worth writing recovery
+      // code for.
+      if (version != 7 && version != 6) {
+        if (version == 5) {
+          RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_WRONG_VERSION5);
+        } else {
+          RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_WRONG_VERSION);
+        }
+        recovery_version.Clear();
+        sql::Recovery::Rollback(recovery.Pass());
+        return;
+      }
     }
 
-    if (!recovery->AutoRecoverTable("icon_mapping", 0,
-                                    &icon_mapping_rows_recovered)) {
+    // Either version 6 or version 7 recovers to current.
+    sql::MetaTable recover_meta_table;
+    if (!recover_meta_table.Init(recovery->db(), kCurrentVersionNumber,
+                                 kCompatibleVersionNumber)) {
       sql::Recovery::Rollback(recovery.Pass());
-      RecordRecoveryEvent(RECOVERY_EVENT_FAILED_V5_AUTORECOVER_ICON_MAPPING);
+      RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_INIT);
       return;
     }
-
-    ignore_result(sql::Recovery::Recovered(recovery.Pass()));
-
-    // TODO(shess): Could this code be shared with the v6/7 code
-    // without requiring too much state to be carried?
-
-    // Track the size of the recovered database relative to the size of
-    // the input database.  The size should almost always be smaller,
-    // unless the input database was empty to start with.  If the
-    // percentage results are very low, something is awry.
-    int64 final_size = 0;
-    if (original_size > 0 &&
-        file_util::GetFileSize(db_path, &final_size) &&
-        final_size > 0) {
-      int percentage = static_cast<int>(original_size * 100 / final_size);
-      UMA_HISTOGRAM_PERCENTAGE("History.FaviconsRecoveredPercentage",
-                               std::max(100, percentage));
-    }
-
-    // Using 10,000 because these cases mostly care about "none
-    // recovered" and "lots recovered".  More than 10,000 rows recovered
-    // probably means there's something wrong with the profile.
-    UMA_HISTOGRAM_COUNTS_10000("History.FaviconsRecoveredRowsFavicons",
-                               favicons_rows_recovered);
-    UMA_HISTOGRAM_COUNTS_10000("History.FaviconsRecoveredRowsIconMapping",
-                               icon_mapping_rows_recovered);
-
-    RecordRecoveryEvent(RECOVERY_EVENT_RECOVERED_VERSION5);
-    return;
   }
 
-  // This code may be able to fetch versions that the regular
-  // deprecation path cannot.
-  if (version <= kDeprecatedVersionNumber) {
-    sql::Recovery::Unrecoverable(recovery.Pass());
-    RecordRecoveryEvent(RECOVERY_EVENT_DEPRECATED);
-    return;
-  }
-
-  // TODO(shess): Earlier versions have been handled or deprecated,
-  // later versions should be impossible.  Unrecoverable() seems
-  // reasonable.
-  if (version != 6 && version != 7) {
-    RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_WRONG_VERSION);
-    sql::Recovery::Rollback(recovery.Pass());
-    return;
-  }
-
-  // Both v6 and v7 recover to current schema version.
-  sql::MetaTable recover_meta_table;
-  if (!recover_meta_table.Init(recovery->db(), kCurrentVersionNumber,
-                               kCompatibleVersionNumber)) {
-    sql::Recovery::Rollback(recovery.Pass());
-    RecordRecoveryEvent(RECOVERY_EVENT_FAILED_META_INIT);
-    return;
-  }
-
   // Create a fresh version of the database.  The recovery code uses
   // conflict-resolution to handle duplicates, so the indices are
   // necessary.
@@ -585,24 +493,114 @@
     return;
   }
 
-  // [favicons] differs because v6 had an unused [sizes] column which
-  // was removed in v7.
-  if (!recovery->AutoRecoverTable("favicons", 1, &favicons_rows_recovered)) {
-    sql::Recovery::Rollback(recovery.Pass());
-    RecordRecoveryEvent(RECOVERY_EVENT_FAILED_AUTORECOVER_FAVICONS);
-    return;
+  // Setup favicons table.
+  {
+    // Version 6 had the |sizes| column, version 7 removed it.  The
+    // recover virtual table treats more columns than expected as an
+    // error, but if _fewer_ columns are present, they can be treated
+    // as NULL.  SQLite requires this because ALTER TABLE adds columns
+    // to the schema, but not to the actual table storage.
+    const char kRecoverySql[] =
+        "CREATE VIRTUAL TABLE temp.recover_favicons USING recover"
+        "("
+        "corrupt.favicons,"
+        "id ROWID,"
+        "url TEXT NOT NULL,"
+        "icon_type INTEGER,"
+        "sizes TEXT"
+        ")";
+    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 main.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;
+    }
+    favicons_rows_recovered = recovery->db()->GetLastChangeCount();
   }
-  if (!recovery->AutoRecoverTable("favicon_bitmaps", 0,
-                                  &favicon_bitmaps_rows_recovered)) {
-    sql::Recovery::Rollback(recovery.Pass());
-    RecordRecoveryEvent(RECOVERY_EVENT_FAILED_AUTORECOVER_FAVICON_BITMAPS);
-    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 main.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;
+    }
+    favicon_bitmaps_rows_recovered = recovery->db()->GetLastChangeCount();
   }
-  if (!recovery->AutoRecoverTable("icon_mapping", 0,
-                                  &icon_mapping_rows_recovered)) {
-    sql::Recovery::Rollback(recovery.Pass());
-    RecordRecoveryEvent(RECOVERY_EVENT_FAILED_AUTORECOVER_ICON_MAPPING);
-    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 main.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;
+    }
+    icon_mapping_rows_recovered = recovery->db()->GetLastChangeCount();
   }
 
   // TODO(shess): Is it possible/likely to have broken foreign-key