Handle cookie-file corruption found during open.

KillDatabaseErrorDelegate cannot Raze() if the corruption is found
while opening the databse, because by the time the callback happens
the database has been closed.  Add support for setting a flag for the
database-opening code to check.

Additionally, errors detected when executing or preparing statements
were not forwarded to the error delegate, which could let them sneak
by without notice.

BUG=159490


Review URL: https://chromiumcodereview.appspot.com/11358170

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

2 files changed, 33 insertions, 7 deletions

diff --git a/chrome/browser/net/sqlite_persistent_cookie_store.cc b/chrome/browser/net/sqlite_persistent_cookie_store.cc
index d85f1fc..1eb335e 100644
--- a/chrome/browser/net/sqlite_persistent_cookie_store.cc
+++ b/chrome/browser/net/sqlite_persistent_cookie_store.cc
@@ -71,6 +71,7 @@ class SQLitePersistentCookieStore::Backend
         num_pending_(0),
         force_keep_session_state_(false),
         initialized_(false),
+        corruption_detected_(false),
         restore_old_session_cookies_(restore_old_session_cookies),
         clear_on_exit_policy_(clear_on_exit_policy),
         num_cookies_read_(0),
@@ -220,6 +221,7 @@ class SQLitePersistentCookieStore::Backend
   void DeleteSessionCookiesOnShutdown();
 
   void KillDatabase();
+  void ScheduleKillDatabase();
 
   FilePath path_;
   scoped_ptr<sql::Connection> db_;
@@ -250,6 +252,9 @@ class SQLitePersistentCookieStore::Backend
   // Indicates if DB has been initialized.
   bool initialized_;
 
+  // Indicates if the kill-database callback has been scheduled.
+  bool corruption_detected_;
+
   // If false, we should filter out session cookies when reading the DB.
   bool restore_old_session_cookies_;
 
@@ -294,10 +299,7 @@ int SQLitePersistentCookieStore::Backend::KillDatabaseErrorDelegate::OnError(
   if (!attempted_to_kill_database_ && sql::IsErrorCatastrophic(error)) {
     attempted_to_kill_database_ = true;
 
-    // Don't just do the close/delete here, as we are being called by |db| and
-    // that seems dangerous.
-    MessageLoop::current()->PostTask(
-        FROM_HERE, base::Bind(&Backend::KillDatabase, backend_));
+    backend_->ScheduleKillDatabase();
   }
 
   return error;
@@ -530,9 +532,9 @@ void SQLitePersistentCookieStore::Backend::Notify(
 bool SQLitePersistentCookieStore::Backend::InitializeDatabase() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
 
-  if (initialized_) {
+  if (initialized_ || corruption_detected_) {
     // Return false if we were previously initialized but the DB has since been
-    // closed.
+    // closed, or if corruption caused a database reset during initialization.
     return db_ != NULL;
   }
 
@@ -554,6 +556,8 @@ bool SQLitePersistentCookieStore::Backend::InitializeDatabase() {
 
   if (!db_->Open(path_)) {
     NOTREACHED() << "Unable to open cookie DB.";
+    if (corruption_detected_)
+      db_->Raze();
     meta_table_.Reset();
     db_.reset();
     return false;
@@ -561,6 +565,8 @@ bool SQLitePersistentCookieStore::Backend::InitializeDatabase() {
 
   if (!EnsureDatabaseVersion() || !InitTable(db_.get())) {
     NOTREACHED() << "Unable to open cookie DB.";
+    if (corruption_detected_)
+      db_->Raze();
     meta_table_.Reset();
     db_.reset();
     return false;
@@ -579,6 +585,8 @@ bool SQLitePersistentCookieStore::Backend::InitializeDatabase() {
     "SELECT DISTINCT host_key FROM cookies"));
 
   if (!smt.is_valid()) {
+    if (corruption_detected_)
+      db_->Raze();
     meta_table_.Reset();
     db_.reset();
     return false;
@@ -1029,6 +1037,17 @@ void SQLitePersistentCookieStore::Backend::DeleteSessionCookiesOnShutdown() {
     LOG(WARNING) << "Unable to delete cookies on shutdown.";
 }
 
+void SQLitePersistentCookieStore::Backend::ScheduleKillDatabase() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+
+  corruption_detected_ = true;
+
+  // Don't just do the close/delete here, as we are being called by |db| and
+  // that seems dangerous.
+  MessageLoop::current()->PostTask(
+      FROM_HERE, base::Bind(&Backend::KillDatabase, this));
+}
+
 void SQLitePersistentCookieStore::Backend::KillDatabase() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
 
diff --git a/sql/connection.cc b/sql/connection.cc
index 93d0695a..1de9fc5 100644
--- a/sql/connection.cc
+++ b/sql/connection.cc
@@ -368,6 +368,9 @@ int Connection::ExecuteAndReturnErrorCode(const char* sql) {
 
 bool Connection::Execute(const char* sql) {
   int error = ExecuteAndReturnErrorCode(sql);
+  if (error != SQLITE_OK)
+    error = OnSqliteError(error, NULL);
+
   // This needs to be a FATAL log because the error case of arriving here is
   // that there's a malformed SQL statement. This can arise in development if
   // a change alters the schema but not all queries adjust.
@@ -417,9 +420,13 @@ scoped_refptr<Connection::StatementRef> Connection::GetUniqueStatement(
     return new StatementRef();  // Return inactive statement.
 
   sqlite3_stmt* stmt = NULL;
-  if (sqlite3_prepare_v2(db_, sql, -1, &stmt, NULL) != SQLITE_OK) {
+  int rc = sqlite3_prepare_v2(db_, sql, -1, &stmt, NULL);
+  if (rc != SQLITE_OK) {
     // This is evidence of a syntax error in the incoming SQL.
     DLOG(FATAL) << "SQL compile error " << GetErrorMessage();
+
+    // It could also be database corruption.
+    OnSqliteError(rc, NULL);
     return new StatementRef();
   }
   return new StatementRef(this, stmt);