Reinitialize the cookie database if the meta table gets corrupted.

chrome/browser/net/sqlite_persistent_cookie_store.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/net/sqlite_persistent_cookie_store.h"
 
 #include <list>
 #include <map>
 #include <set>
 #include <utility>
@@ skipping 750 matching lines @@
     meta_table_.SetVersionNumber(cur_version);
     meta_table_.SetCompatibleVersionNumber(
         std::min(cur_version, kCompatibleVersionNumber));
     transaction.Commit();
     UMA_HISTOGRAM_TIMES("Cookie.TimeDatabaseMigrationToV5",
                         base::TimeTicks::Now() - start_time);
   }
 
   // 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;
 }
 
 void SQLitePersistentCookieStore::Backend::AddCookie(
     const net::CookieMonster::CanonicalCookie& cc) {
   BatchOperation(PendingOperation::COOKIE_ADD, cc);
 }
 
 void SQLitePersistentCookieStore::Backend::UpdateCookieAccessTime(
@@ skipping 221 matching lines @@
   if (backend_.get())
     backend_->SetClearLocalStateOnExit(clear_local_state);
 }
 
 void SQLitePersistentCookieStore::Flush(const base::Closure& callback) {
   if (backend_.get())
     backend_->Flush(callback);
   else if (!callback.is_null())
     MessageLoop::current()->PostTask(FROM_HERE, callback);
 }