Raze old WebDatabases.

components/webdata/common/web_database.cc

Index: components/webdata/common/web_database.cc
diff --git a/components/webdata/common/web_database.cc b/components/webdata/common/web_database.cc
index 018663da75657c85126a5247476a69b9b3bcc3e4..f99f43713f82ebd670a4b3f073bcbff737a6167a 100644
--- a/components/webdata/common/web_database.cc
+++ b/components/webdata/common/web_database.cc
@@ -16,6 +16,8 @@
 // static
 const int WebDatabase::kCurrentVersionNumber = 62;
 
+const int WebDatabase::kDeprecatedVersionNumber = 51;
+
 namespace {
 
 const int kCompatibleVersionNumber = 61;
@@ -87,7 +89,12 @@ sql::InitStatus WebDatabase::Init(const base::FilePath& db_name) {
   if (!db_.Open(db_name))
     return sql::INIT_FAILURE;
 
-  // Initialize various tables
+  // Clobber really old databases.
+  DCHECK_LT(kDeprecatedVersionNumber, kCurrentVersionNumber);

[Peter Kasting, 2015/02/26 22:33:19]

I don't know that this DCHECK is really needed.

[Evan Stade, 2015/02/26 22:37:19]

This is copied from TopSitesDatabase. I am ambivalent about its necessity
overall but I would like to be consistent.

[Scott Hess - ex-Googler, 2015/02/27 06:49:38]

The rationale was that if someone accidentally got it wrong, all of the
databases would be reset with no way to recover.  I don't think there was any
reason not to use a compile-time assertion, though.

Of course given appropriate test coverage this is completely redundant.  I wish
there were a way to assert "You can only use this if you're testing this edge".

[Peter Kasting, 2015/02/27 09:15:25]

I didn't even think of that.  Yes, please use a static_assert if keeping this,
that's safer.

[Evan Stade, 2015/02/28 00:13:41]

Done.

+  sql::MetaTable::RazeIfDeprecated(&db_, kDeprecatedVersionNumber);
+
+  // Scope initialization in a transaction so we can't be partially
+  // initialized.
   sql::Transaction transaction(&db_);
   if (!transaction.Begin())
     return sql::INIT_FAILURE;
@@ -135,20 +142,14 @@ sql::InitStatus WebDatabase::MigrateOldVersionsAsNeeded() {
   if (current_version > meta_table_.GetVersionNumber())
     ChangeVersion(&meta_table_, current_version, false);
 
-  if (current_version < 20) {
-    // Versions 1 - 19 are unhandled.  Version numbers greater than
+  if (current_version < kDeprecatedVersionNumber) {

[Scott Hess - ex-Googler, 2015/02/27 06:49:38]

This case should not be possible anymore, right?

[Peter Kasting, 2015/02/27 09:15:25]

I think not; even if the user manually copies old DB files in after we raze
them, we'll just raze them again.  So this could probably become a DCHECK and
omit the rest of the block here.

[Evan Stade, 2015/02/28 00:13:41]

Done.

+    // Deprecated versions are unhandled.  Version numbers greater than
     // kCurrentVersionNumber should have already been weeded out by the caller.
     //
     // When the version is too old, we return failure error code.  The schema
     // is too out of date to migrate.
-    //
-    // There should not be a released product that makes a database too old to
-    // migrate. If we do encounter such a legacy database, we will need a
-    // better solution to handle it (i.e., pop up a dialog to tell the user,
-    // erase all their prefs and start over, etc.).
     LOG(WARNING) << "Web database version " << current_version
                  << " is too old to handle.";
-    NOTREACHED();
     return sql::INIT_FAILURE;
   }