Add compatible version support since it was only halfway in place, and try and make our database versioning code and logging more similar across various consumers.

The compatible version support isn't really used yet.  It was going to be used for my cookie change until we decided that the old code was too busted to be forward-compatible.  It seems worthwhile to put this in but maybe I am wrong.

The logging similarity stuff is fairly useful.  In a couple consumers in the old code, we DLOGed instead of LOGing, which meant that most people would get nothing in the log at all.  I think it's a little weird that in a lot of these consumers, logging is all we do; for example, if you use a too-new cookie DB, you get output in the log, but no actual dialog box while the browser is running -- your cookies just silently don't get saved to disk.  Seems bad, but I'm not prepared to try and do major surgery to address that (and add translated strings, etc.).  At least now we'll actually get log messages in release builds instead of nothing at all.

Because my last-access change touches this code, I'm considering asking that this change be merged back to the branch.
Review URL: http://codereview.chromium.org/8712

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

8 files changed, 82 insertions, 50 deletions

diff --git a/chrome/browser/history/archived_database.cc b/chrome/browser/history/archived_database.cc
index 68821f6..468376c 100644
--- a/chrome/browser/history/archived_database.cc
+++ b/chrome/browser/history/archived_database.cc
@@ -10,6 +10,7 @@ namespace history {
 namespace {
 
 static const int kCurrentVersionNumber = 2;
+static const int kCompatibleVersionNumber = 2;
 
 }  // namespace
 
@@ -48,7 +49,8 @@ bool ArchivedDatabase::Init(const std::wstring& file_name) {
   BeginTransaction();
 
   // Version check.
-  if (!meta_table_.Init(std::string(), kCurrentVersionNumber, db_))
+  if (!meta_table_.Init(std::string(), kCurrentVersionNumber,
+                        kCompatibleVersionNumber, db_))
     return false;
 
   // Create the tables.
@@ -98,8 +100,10 @@ SqliteStatementCache& ArchivedDatabase::GetStatementCache() {
 
 InitStatus ArchivedDatabase::EnsureCurrentVersion() {
   // We can't read databases newer than we were designed for.
-  if (meta_table_.GetCompatibleVersionNumber() > kCurrentVersionNumber)
+  if (meta_table_.GetCompatibleVersionNumber() > kCurrentVersionNumber) {
+    LOG(WARNING) << "Archived database is too new.";
     return INIT_TOO_NEW;
+  }
 
   // NOTICE: If you are changing structures for things shared with the archived
   // history file like URLs, visits, or downloads, that will need migration as
@@ -107,20 +111,22 @@ InitStatus ArchivedDatabase::EnsureCurrentVersion() {
   // in the corresponding file (url_database.cc, etc.) and called from here and
   // from the archived_database.cc.
 
-  // When the version is too old, we just try to continue anyway, there should
-  // not be a released product that makes a database too old for us to handle.
   int cur_version = meta_table_.GetVersionNumber();
-
-  // Put migration code here
-
   if (cur_version == 1) {
-    if (!DropStarredIDFromURLs())
+    if (!DropStarredIDFromURLs()) {
+      LOG(WARNING) << "Unable to update archived database to version 2.";
       return INIT_FAILURE;
-    cur_version = 2;
+    }
+    ++cur_version;
     meta_table_.SetVersionNumber(cur_version);
-    meta_table_.SetCompatibleVersionNumber(cur_version);
+    meta_table_.SetCompatibleVersionNumber(
+        std::min(cur_version, kCompatibleVersionNumber));
   }
 
+  // Put future migration cases here.
+
+  // When the version is too old, we just try to continue anyway, there should
+  // not be a released product that makes a database too old for us to handle.
   LOG_IF(WARNING, cur_version < kCurrentVersionNumber) <<
       "Archived database version " << cur_version << " is too old to handle.";
 
diff --git a/chrome/browser/history/history_database.cc b/chrome/browser/history/history_database.cc
index cf49883..8a1bb59 100644
--- a/chrome/browser/history/history_database.cc
+++ b/chrome/browser/history/history_database.cc
@@ -18,7 +18,8 @@ namespace history {
 namespace {
 
 // Current version number.
-const int kCurrentVersionNumber = 16;
+static const int kCurrentVersionNumber = 16;
+static const int kCompatibleVersionNumber = 16;
 
 }  // namespace
 
@@ -66,7 +67,8 @@ InitStatus HistoryDatabase::Init(const std::wstring& history_name,
   // Create the tables and indices.
   // NOTE: If you add something here, also add it to
   //       RecreateAllButStarAndURLTables.
-  if (!meta_table_.Init(std::string(), kCurrentVersionNumber, db_))
+  if (!meta_table_.Init(std::string(), kCurrentVersionNumber,
+                        kCompatibleVersionNumber, db_))
     return INIT_FAILURE;
   if (!CreateURLTable(false) || !InitVisitTable() ||
       !InitKeywordSearchTermsTable() || !InitDownloadTable() ||
@@ -199,8 +201,10 @@ SqliteStatementCache& HistoryDatabase::GetStatementCache() {
 InitStatus HistoryDatabase::EnsureCurrentVersion(
     const std::wstring& tmp_bookmarks_path) {
   // We can't read databases newer than we were designed for.
-  if (meta_table_.GetCompatibleVersionNumber() > kCurrentVersionNumber)
+  if (meta_table_.GetCompatibleVersionNumber() > kCurrentVersionNumber) {
+    LOG(WARNING) << "History database is too new.";
     return INIT_TOO_NEW;
+  }
 
   // NOTICE: If you are changing structures for things shared with the archived
   // history file like URLs, visits, or downloads, that will need migration as
@@ -208,22 +212,24 @@ InitStatus HistoryDatabase::EnsureCurrentVersion(
   // in the corresponding file (url_database.cc, etc.) and called from here and
   // from the archived_database.cc.
 
-  // When the version is too old, we just try to continue anyway, there should
-  // not be a released product that makes a database too old for us to handle.
   int cur_version = meta_table_.GetVersionNumber();
 
   // Put migration code here
 
   if (cur_version == 15) {
-    if (!MigrateBookmarksToFile(tmp_bookmarks_path))
+    if (!MigrateBookmarksToFile(tmp_bookmarks_path) ||
+        !DropStarredIDFromURLs()) {
+      LOG(WARNING) << "Unable to update history database to version 16.";
       return INIT_FAILURE;
-    if (!DropStarredIDFromURLs())
-      return INIT_FAILURE;
-    cur_version = 16;
+    }
+    ++cur_version;
     meta_table_.SetVersionNumber(cur_version);
-    meta_table_.SetCompatibleVersionNumber(cur_version);
+    meta_table_.SetCompatibleVersionNumber(
+        std::min(cur_version, kCompatibleVersionNumber));
   }
 
+  // When the version is too old, we just try to continue anyway, there should
+  // not be a released product that makes a database too old for us to handle.
   LOG_IF(WARNING, cur_version < kCurrentVersionNumber) <<
       "History database version " << cur_version << " is too old to handle.";
 
diff --git a/chrome/browser/history/text_database.cc b/chrome/browser/history/text_database.cc
index 482db21..00d05ce 100644
--- a/chrome/browser/history/text_database.cc
+++ b/chrome/browser/history/text_database.cc
@@ -38,7 +38,8 @@ namespace history {
 
 namespace {
 
-const int kCurrentVersionNumber = 1;
+static const int kCurrentVersionNumber = 1;
+static const int kCompatibleVersionNumber = 1;
 
 // Snippet computation relies on the index of the columns in the original
 // create statement. These are the 0-based indices (as strings) of the
@@ -161,7 +162,8 @@ bool TextDatabase::Init() {
   sqlite3_exec(db_, "PRAGMA locking_mode=EXCLUSIVE", NULL, NULL, NULL);
 
   // Meta table tracking version information.
-  if (!meta_table_.Init(std::string(), kCurrentVersionNumber, db_))
+  if (!meta_table_.Init(std::string(), kCurrentVersionNumber,
+                        kCompatibleVersionNumber, db_))
     return false;
   if (meta_table_.GetCompatibleVersionNumber() > kCurrentVersionNumber) {
     // This version is too new. We don't bother notifying the user on this
@@ -170,6 +172,7 @@ bool TextDatabase::Init() {
     // here. If that's not the case, since this is only indexed data, it's
     // probably better to just not give FTS results than strange errors when
     // everything else is working OK.
+    LOG(WARNING) << "Text database is too new.";
     return false;
   }
 
diff --git a/chrome/browser/history/thumbnail_database.cc b/chrome/browser/history/thumbnail_database.cc
index 7cef980..106a45be 100644
--- a/chrome/browser/history/thumbnail_database.cc
+++ b/chrome/browser/history/thumbnail_database.cc
@@ -19,6 +19,7 @@ namespace history {
 
 // Version number of the database.
 static const int kCurrentVersionNumber = 3;
+static const int kCompatibleVersionNumber = 3;
 
 ThumbnailDatabase::ThumbnailDatabase()
     : db_(NULL),
@@ -64,7 +65,8 @@ InitStatus ThumbnailDatabase::Init(const std::wstring& db_name) {
   transaction.Begin();
 
   // Create the tables.
-  if (!meta_table_.Init(std::string(), kCurrentVersionNumber, db_) ||
+  if (!meta_table_.Init(std::string(), kCurrentVersionNumber,
+                        kCompatibleVersionNumber, db_) ||
       !InitThumbnailTable() ||
       !InitFavIconsTable(false))
     return INIT_FAILURE;
@@ -72,16 +74,22 @@ InitStatus ThumbnailDatabase::Init(const std::wstring& db_name) {
 
   // Version check. We should not encounter a database too old for us to handle
   // in the wild, so we try to continue in that case.
-  if (meta_table_.GetCompatibleVersionNumber() > kCurrentVersionNumber)
+  if (meta_table_.GetCompatibleVersionNumber() > kCurrentVersionNumber) {
+    LOG(WARNING) << "Thumbnail database is too new.";
     return INIT_TOO_NEW;
+  }
+
   int cur_version = meta_table_.GetVersionNumber();
   if (cur_version == 2) {
-    UpgradeToVersion3();
-    cur_version = meta_table_.GetVersionNumber();
+    if (!UpgradeToVersion3()) {
+      LOG(WARNING) << "Unable to update to thumbnail database to version 3.";
+      return INIT_FAILURE;
+    }
+    ++cur_version;
   }
 
-  DLOG_IF(WARNING, cur_version < kCurrentVersionNumber) <<
-    "Thumbnail database version " << cur_version << " is too old for us.";
+  LOG_IF(WARNING, cur_version < kCurrentVersionNumber) <<
+      "Thumbnail database version " << cur_version << " is too old to handle.";
 
   // Initialization is complete.
   if (transaction.Commit() != SQLITE_OK)
@@ -132,7 +140,7 @@ bool ThumbnailDatabase::InitThumbnailTable() {
   return true;
 }
 
-void ThumbnailDatabase::UpgradeToVersion3() {
+bool ThumbnailDatabase::UpgradeToVersion3() {
   // sqlite doesn't like the "ALTER TABLE xxx ADD (column_one, two,
   // three)" syntax, so list out the commands we need to execute:
   const char* alterations[] = {
@@ -146,12 +154,14 @@ void ThumbnailDatabase::UpgradeToVersion3() {
   for (int i = 0; alterations[i] != NULL; ++i) {
     if (sqlite3_exec(db_, alterations[i],
                      NULL, NULL, NULL) != SQLITE_OK) {
-      NOTREACHED() << "Failed to update to v3.";
-      return;
+      NOTREACHED();
+      return false;
     }
   }
 
-  meta_table_.SetVersionNumber(kCurrentVersionNumber);
+  meta_table_.SetVersionNumber(3);
+  meta_table_.SetCompatibleVersionNumber(std::min(3, kCompatibleVersionNumber));
+  return true;
 }
 
 bool ThumbnailDatabase::RecreateThumbnailTable() {
@@ -294,7 +304,7 @@ bool ThumbnailDatabase::ThumbnailScoreForId(
   // aren't replacing a good thumbnail with one that's worse.
   SQLITE_UNIQUE_STATEMENT(
       select_statement, *statement_cache_,
-      "SELECT boring_score,good_clipping,at_top,last_updated "
+      "SELECT boring_score, good_clipping, at_top, last_updated "
       "FROM thumbnails WHERE url_id=?");
   if (!select_statement.is_valid()) {
     NOTREACHED() << "Couldn't build select statement!";
@@ -321,7 +331,7 @@ bool ThumbnailDatabase::SetFavIcon(URLID icon_id,
   if (icon_data.size()) {
     SQLITE_UNIQUE_STATEMENT(
         statement, *statement_cache_,
-        "UPDATE favicons SET image_data=?,last_updated=? WHERE id=?");
+        "UPDATE favicons SET image_data=?, last_updated=? WHERE id=?");
     if (!statement.is_valid())
       return 0;
 
@@ -333,7 +343,7 @@ bool ThumbnailDatabase::SetFavIcon(URLID icon_id,
   } else {
     SQLITE_UNIQUE_STATEMENT(
         statement, *statement_cache_,
-        "UPDATE favicons SET image_data=NULL,last_updated=? WHERE id=?");
+        "UPDATE favicons SET image_data=NULL, last_updated=? WHERE id=?");
     if (!statement.is_valid())
       return 0;
 
@@ -377,7 +387,7 @@ bool ThumbnailDatabase::GetFavIcon(
   DCHECK(icon_id);
 
   SQLITE_UNIQUE_STATEMENT(statement, *statement_cache_,
-      "SELECT last_updated,image_data,url FROM favicons WHERE id=?");
+      "SELECT last_updated, image_data, url FROM favicons WHERE id=?");
   if (!statement.is_valid())
     return 0;
 
@@ -420,10 +430,9 @@ bool ThumbnailDatabase::DeleteFavIcon(FavIconID id) {
 
 FavIconID ThumbnailDatabase::CopyToTemporaryFavIconTable(FavIconID source) {
   SQLITE_UNIQUE_STATEMENT(statement, *statement_cache_,
-                          "INSERT INTO temp_favicons("
-                              "url, last_updated, image_data)"
-                          "SELECT url, last_updated, image_data "
-                          "FROM favicons WHERE id = ?");
+      "INSERT INTO temp_favicons (url, last_updated, image_data)"
+      "SELECT url, last_updated, image_data "
+      "FROM favicons WHERE id = ?");
   if (!statement.is_valid())
     return 0;
   statement->bind_int64(0, source);
diff --git a/chrome/browser/history/thumbnail_database.h b/chrome/browser/history/thumbnail_database.h
index a72c0ef..3b9e1ff 100644
--- a/chrome/browser/history/thumbnail_database.h
+++ b/chrome/browser/history/thumbnail_database.h
@@ -139,7 +139,7 @@ class ThumbnailDatabase {
   bool InitFavIconsTable(bool is_temporary);
 
   // Adds support for the new metadata on web page thumbnails.
-  void UpgradeToVersion3();
+  bool UpgradeToVersion3();
 
   // Creates the index over the favicon table. This will be called during
   // initialization after the table is created. This is a separate function
diff --git a/chrome/browser/meta_table_helper.cc b/chrome/browser/meta_table_helper.cc
index 0e17963..11fd5f5 100644
--- a/chrome/browser/meta_table_helper.cc
+++ b/chrome/browser/meta_table_helper.cc
@@ -20,6 +20,7 @@ MetaTableHelper::~MetaTableHelper() {
 
 bool MetaTableHelper::Init(const std::string& db_name,
                            int version,
+                           int compatible_version,
                            sqlite3* db) {
   DCHECK(!db_ && db);
   db_ = db;
@@ -42,7 +43,7 @@ bool MetaTableHelper::Init(const std::string& db_name,
     // couple of keys, so it doesn't matter. If we start storing more stuff in
     // there, we should create an index.
     SetVersionNumber(version);
-    SetCompatibleVersionNumber(version);
+    SetCompatibleVersionNumber(compatible_version);
   }
   return true;
 }
diff --git a/chrome/browser/meta_table_helper.h b/chrome/browser/meta_table_helper.h
index 8b072b8..05c1bc2 100644
--- a/chrome/browser/meta_table_helper.h
+++ b/chrome/browser/meta_table_helper.h
@@ -26,13 +26,16 @@ class MetaTableHelper {
   ~MetaTableHelper();
 
   // Initializes the MetaTableHelper, creating the meta table if necessary. For
-  // new tables, it will initialize the version number to |version| and will
-  // set the optional |is_new| out var to true.
+  // new tables, it will initialize the version number to |version| and the
+  // compatible version number to |compatible_version|.
   //
   // The name of the database in the sqlite connection (for tables named with
   // the "db_name.table_name" scheme is given in |db_name|. If empty, it is
   // assumed there is no database name.
-  bool Init(const std::string& db_name, int version, sqlite3* db);
+  bool Init(const std::string& db_name,
+            int version,
+            int compatible_version,
+            sqlite3* db);
 
   // Version number. This should be the version number of the creator of the
   // file. GetVersionNumber will return 0 if there is no version number.
diff --git a/chrome/browser/webdata/web_database.cc b/chrome/browser/webdata/web_database.cc
index 5e14723..dca2924 100644
--- a/chrome/browser/webdata/web_database.cc
+++ b/chrome/browser/webdata/web_database.cc
@@ -77,6 +77,7 @@ using base::Time;
 
 // Current version number.
 static const int kCurrentVersionNumber = 21;
+static const int kCompatibleVersionNumber = 21;
 
 // Keys used in the meta table.
 static const char* kDefaultSearchProviderKey = "Default Search Provider ID";
@@ -150,10 +151,11 @@ bool WebDatabase::Init(const std::wstring& db_name) {
   transaction.Begin();
 
   // Version check.
-  if (!meta_table_.Init(std::string(), kCurrentVersionNumber, db_))
+  if (!meta_table_.Init(std::string(), kCurrentVersionNumber,
+                        kCompatibleVersionNumber, db_))
     return false;
   if (meta_table_.GetCompatibleVersionNumber() > kCurrentVersionNumber) {
-    LOG(WARNING) << "Web database is too new";
+    LOG(WARNING) << "Web database is too new.";
     return false;
   }
 
@@ -870,11 +872,13 @@ void WebDatabase::MigrateOldVersionsAsNeeded() {
                        "ALTER TABLE keywords ADD COLUMN autogenerate_keyword "
                        "INTEGER DEFAULT 0", NULL, NULL, NULL) != SQLITE_OK) {
         NOTREACHED();
+        LOG(WARNING) << "Unable to update web database to version 21.";
         return;
       }
-      ++current_version;
-      meta_table_.SetVersionNumber(current_version);
-      meta_table_.SetCompatibleVersionNumber(current_version);
+      meta_table_.SetVersionNumber(21);
+      meta_table_.SetCompatibleVersionNumber(
+          std::min(21, kCompatibleVersionNumber));
+      // FALL THROUGH
 
     // Add successive versions here.  Each should set the version number and
     // compatible version number as appropriate, then fall through to the next