Reinitialize the cookie database if the meta table gets corrupted.

chrome/browser/net/sqlite_persistent_cookie_store.cc

Index: chrome/browser/net/sqlite_persistent_cookie_store.cc
diff --git a/chrome/browser/net/sqlite_persistent_cookie_store.cc b/chrome/browser/net/sqlite_persistent_cookie_store.cc
index e3007c6122b03ffeff17e67a729fe88620e2568e..d4243086a3bcd0056936c95474c67c275c7ba80f 100644
--- a/chrome/browser/net/sqlite_persistent_cookie_store.cc
+++ b/chrome/browser/net/sqlite_persistent_cookie_store.cc
@@ -768,10 +768,19 @@ bool SQLitePersistentCookieStore::Backend::EnsureDatabaseVersion() {
 
   // 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) <<
-      "Cookie database version " << cur_version << " is too old to handle.";
+  if (cur_version < kCurrentVersionNumber) {
+    meta_table_.Reset();
+    db_.reset(new sql::Connection);
+    if (!file_util::Delete(path_, false) ||

[Scott Hess - ex-Googler, 2012/03/01 22:15:15]

I am a small bit concerned about whether we'll see cases, especially on Windows,
where this fails.  It mostly shouldn't, but over time I've come to distrust
Delete() on Windows (because I have code which deletes a file and later the file
is still there).  I don't think you should attempt to resolve this here, because
it may be correlated with other oddness.  But I do think you should throw a
histogram in the overall version-check if() to track that we're making progress.

[I've considered adding a SQLite API to zero out an open database.  That would
address the problem of deleting with an open file handle (which I suspect is a
cause in many cases, think virus checkers or something), while also being better
than using DROP TABLE type statements to accomplish it.  But I haven't written
it, yet.]

[erikwright (departed), 2012/03/02 01:04:27]

Done.

+        !db_->Open(path_) ||
+        !meta_table_.Init(
+            db_.get(), kCurrentVersionNumber, kCompatibleVersionNumber)) {

[Scott Hess - ex-Googler, 2012/03/01 22:15:15]

I wish this wasn't replicating other code.  It also looks like you might need to
call InitTable() somewhere after meta_table_.Init().  The caller will call it in
case of FAILURE of this function.

Put another way, I hates it.  The EnsureDatabaseVersion() and InitTable() code
is confusing, at best.  Maybe the correct sequence should be:

If there is no meta table, assume a fresh db and create the meta table and the
cookie tables.

If there is a meta table, and there is no version info, nuke the database and go
to the above case.  Exactly once.

If there is a meta table, and there is version info, upgrade as necessary.

[erikwright (departed), 2012/03/02 01:04:27]

The caller actually calls InitTable only if this function succeeds. It's pretty
bad, and confusing, I agree, though the current form works (as demonstrated by
the unit test).
I would be happy to refactor this (actually, the entire cookie subsystem) into
something more sensible. I suppose, however, that if there is any possibility of
merging to 17 or 18 I should start with something that is minimal and correct.

I will send a status report to the relevant release managers and get their
feedback.

+      NOTREACHED() << "Unable to reset the cookie DB.";
+      meta_table_.Reset();
+      db_.reset();
+      return false;
+    }
+  }
 
   return true;
 }