LoginDatabase::Init: rollback before closing a DB

components/password_manager/core/browser/login_database.cc

 // Copyright 2014 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 "components/password_manager/core/browser/login_database.h"
 
 #include <stddef.h>
 #include <stdint.h>
 #include <algorithm>
 #include <limits>
@@ skipping 513 matching lines @@
   if (!db_.Open(db_path_)) {
     LogDatabaseInitError(OPEN_FILE_ERROR);
     LOG(ERROR) << "Unable to open the password store database.";
     return false;
   }
 
   sql::Transaction transaction(&db_);
   if (!transaction.Begin()) {
     LogDatabaseInitError(START_TRANSACTION_ERROR);
     LOG(ERROR) << "Unable to start a transaction.";
+    transaction.Rollback();

[vasilii, 2016/08/22 14:31:54]

If it didn't begin there is nothing to rollback. The call would DCHECK.

[vabr (Chromium), 2016/08/22 14:39:48]

Done.

     db_.Close();
     return false;
   }
 
   // Check the database version.
   if (!meta_table_.Init(&db_, kCurrentVersionNumber,
                         kCompatibleVersionNumber)) {
     LogDatabaseInitError(META_TABLE_INIT_ERROR);
     LOG(ERROR) << "Unable to create the meta table.";
+    transaction.Rollback();
     db_.Close();
     return false;
   }
   if (meta_table_.GetCompatibleVersionNumber() > kCurrentVersionNumber) {
     LogDatabaseInitError(INCOMPATIBLE_VERSION);
     LOG(ERROR) << "Password store database is too new, kCurrentVersionNumber="
                << kCurrentVersionNumber << ", GetCompatibleVersionNumber="
                << meta_table_.GetCompatibleVersionNumber();
+    transaction.Rollback();
     db_.Close();
     return false;
   }
 
   SQLTableBuilder builder;
   InitializeBuilder(&builder);
   InitializeStatementStrings(builder);
 
   if (!db_.DoesTableExist("logins")) {
     if (!builder.CreateTable(&db_)) {
       VLOG(0) << "Failed to create the 'logins' table";
+      transaction.Rollback();
       db_.Close();
       return false;
     }
   }
 
   stats_table_.Init(&db_);
 
   int current_version = meta_table_.GetVersionNumber();
   bool migration_success = FixVersionIfNeeded(&db_, &current_version);
   DCHECK_LE(current_version, kCurrentVersionNumber);
 
   // If the file on disk is an older database version, bring it up to date.
   if (migration_success && current_version < kCurrentVersionNumber) {
     migration_success = MigrateLogins(
         base::checked_cast<unsigned>(current_version), &builder, &db_);
   }
   if (migration_success && current_version <= 15) {
     migration_success = stats_table_.MigrateToVersion(16);
   }
   if (migration_success) {
     meta_table_.SetCompatibleVersionNumber(kCompatibleVersionNumber);
     meta_table_.SetVersionNumber(kCurrentVersionNumber);
   } else {
     LogDatabaseInitError(MIGRATION_ERROR);
     UMA_HISTOGRAM_SPARSE_SLOWLY("PasswordManager.LoginDatabaseFailedVersion",
                                 meta_table_.GetVersionNumber());
     LOG(ERROR) << "Unable to migrate database from "
                << meta_table_.GetVersionNumber() << " to "
                << kCurrentVersionNumber;
+    transaction.Rollback();
     db_.Close();
     return false;
   }
 
   if (!stats_table_.CreateTableIfNecessary()) {
     LogDatabaseInitError(INIT_STATS_ERROR);
     LOG(ERROR) << "Unable to create the stats table.";
+    transaction.Rollback();
     db_.Close();
     return false;
   }
 
   if (!transaction.Commit()) {
     LogDatabaseInitError(COMMIT_TRANSACTION_ERROR);
     LOG(ERROR) << "Unable to commit a transaction.";
+    transaction.Rollback();

[vasilii, 2016/08/22 14:31:54]

That's a bug to call Rollback() after Commit().

[vabr (Chromium), 2016/08/22 14:39:48]

Done.

     db_.Close();
     return false;
   }
 
   LogDatabaseInitError(INIT_OK);
   return true;
 }
 
 void LoginDatabase::ReportMetrics(const std::string& sync_username,
                                   bool custom_passphrase_sync_enabled) {
@@ skipping 683 matching lines @@
   DCHECK(blacklisted_statement_.empty());
   blacklisted_statement_ =
       "SELECT " + all_column_names +
       " FROM logins WHERE blacklisted_by_user == ? ORDER BY origin_url";
   DCHECK(encrypted_statement_.empty());
   encrypted_statement_ =
       "SELECT password_value FROM logins WHERE " + all_unique_key_column_names;
 }
 
 }  // namespace password_manager