[sql] QuotaDatabase schema upgrade doesn't use transactions right.

storage/browser/quota/quota_database.cc

 // Copyright 2013 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 "storage/browser/quota/quota_database.h"
 
 #include <vector>
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
@@ skipping 169 matching lines @@
 
   *quota = statement.ColumnInt64(0);
   return true;
 }
 
 bool QuotaDatabase::SetHostQuota(
     const std::string& host, StorageType type, int64 quota) {
   DCHECK_GE(quota, 0);
   if (!LazyOpen(true))
     return false;
-
-  const char* kSql =
-      "INSERT OR REPLACE INTO HostQuotaTable"
-      " (quota, host, type)"
-      " VALUES (?, ?, ?)";
-  sql::Statement statement(db_->GetCachedStatement(SQL_FROM_HERE, kSql));
-  statement.BindInt64(0, quota);
-  statement.BindString(1, host);
-  statement.BindInt(2, static_cast<int>(type));
-
-  if (!statement.Run())
+  if (!InsertOrReplaceHostQuota(host, type, quota))
     return false;
-
   ScheduleCommit();
   return true;
 }
 
 bool QuotaDatabase::SetOriginLastAccessTime(
     const GURL& origin, StorageType type, base::Time last_access_time) {
   if (!LazyOpen(true))
     return false;
 
   sql::Statement statement;
@@ skipping 277 matching lines @@
   return meta_table_->SetValue(kIsOriginTableBootstrapped, bootstrap_flag);
 }
 
 void QuotaDatabase::Commit() {
   if (!db_)
     return;
 
   if (timer_.IsRunning())
     timer_.Stop();
 
+  DCHECK_EQ(1, db_->transaction_nesting());
   db_->CommitTransaction();
+  DCHECK_EQ(0, db_->transaction_nesting());
   db_->BeginTransaction();
+  DCHECK_EQ(1, db_->transaction_nesting());
 }
 
 void QuotaDatabase::ScheduleCommit() {
   if (timer_.IsRunning())
     return;
   timer_.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(kCommitIntervalMs),
                this, &QuotaDatabase::Commit);
 }
 
 bool QuotaDatabase::FindOriginUsedCount(
@@ skipping 137 matching lines @@
       return false;
     }
   }
 
   return transaction.Commit();
 }
 
 bool QuotaDatabase::ResetSchema() {
   DCHECK(!db_file_path_.empty());
   DCHECK(base::PathExists(db_file_path_));
+  DCHECK(!db_ || !db_->transaction_nesting());
   VLOG(1) << "Deleting existing quota data and starting over.";
 
   db_.reset();
   meta_table_.reset();
 
   if (!sql::Connection::Delete(db_file_path_))
     return false;
 
   // So we can't go recursive.
   if (is_recreating_)
     return false;
 
   base::AutoReset<bool> auto_reset(&is_recreating_, true);
   return LazyOpen(true);
 }
 
 bool QuotaDatabase::UpgradeSchema(int current_version) {
+  DCHECK_EQ(0, db_->transaction_nesting());
+
   if (current_version == 2) {
     QuotaTableImporter importer;
     typedef std::vector<QuotaTableEntry> QuotaTableEntries;
     if (!DumpQuotaTable(base::Bind(&QuotaTableImporter::Append,
                                    base::Unretained(&importer)))) {
       return false;
     }
     ResetSchema();
+
+    sql::Transaction transaction(db_.get());
+    if (!transaction.Begin())
+      return false;
     for (QuotaTableEntries::const_iterator iter = importer.entries.begin();
          iter != importer.entries.end(); ++iter) {
-      if (!SetHostQuota(iter->host, iter->type, iter->quota))
+      if (!InsertOrReplaceHostQuota(iter->host, iter->type, iter->quota))
         return false;
     }
-    Commit();
+    return transaction.Commit();
+  } else if (current_version < 5) {
+    sql::Transaction transaction(db_.get());
+    if (!transaction.Begin())
+      return false;
 
-    return true;
-  } else if (current_version < 5) {
     const QuotaDatabase::TableSchema& eviction_table_schema = kTables[2];
     DCHECK_EQ(strcmp(kEvictionInfoTable, eviction_table_schema.table_name), 0);
 
     std::string sql("CREATE TABLE ");
     sql += eviction_table_schema.table_name;
     sql += eviction_table_schema.columns;
     if (!db_->Execute(sql.c_str())) {
       VLOG(1) << "Failed to execute " << sql;
       return false;
     }
 
     meta_table_->SetVersionNumber(5);
-    Commit();
-
-    return true;
+    return transaction.Commit();
   }
   return false;
 }
 
+bool QuotaDatabase::InsertOrReplaceHostQuota(
+    const std::string& host, StorageType type, int64 quota) {
+  DCHECK(db_.get());
+  const char* kSql =
+      "INSERT OR REPLACE INTO HostQuotaTable"
+      " (quota, host, type)"
+      " VALUES (?, ?, ?)";
+  sql::Statement statement(db_->GetCachedStatement(SQL_FROM_HERE, kSql));
+  statement.BindInt64(0, quota);
+  statement.BindString(1, host);
+  statement.BindInt(2, static_cast<int>(type));
+  return statement.Run();
+}
+
 bool QuotaDatabase::DumpQuotaTable(const QuotaTableCallback& callback) {
   if (!LazyOpen(true))
     return false;
 
   const char* kSql = "SELECT * FROM HostQuotaTable";
   sql::Statement statement(db_->GetCachedStatement(SQL_FROM_HERE, kSql));
 
   while (statement.Step()) {
     QuotaTableEntry entry = QuotaTableEntry(
       statement.ColumnString(0),
@@ skipping 45 matching lines @@
   if (lhs.origin < rhs.origin) return true;
   if (rhs.origin < lhs.origin) return false;
   if (lhs.type < rhs.type) return true;
   if (rhs.type < lhs.type) return false;
   if (lhs.used_count < rhs.used_count) return true;
   if (rhs.used_count < lhs.used_count) return false;
   return lhs.last_access_time < rhs.last_access_time;
 }
 
 }  // namespace storage