Bug 1695883 - Add explicit fallible mozStorageTransaction::Start method for starting the transaction; r=dom-storage-reviewers,sg
authorJan Varga <jvarga@mozilla.com>
Thu, 04 Mar 2021 04:38:06 +0000
changeset 569609 aada018490c4fe60ebd8cc76e014c6dcd734de85
parent 569608 0f6f9ff1c44fc739b77fd7cdf93444a0de1d0e8f
child 569610 03dbda17119d8074f9dea0da772fed5264201612
push id137712
push userjvarga@mozilla.com
push dateThu, 04 Mar 2021 04:40:29 +0000
treeherderautoland@aada018490c4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdom-storage-reviewers, sg
bugs1695883, 1696129, 1696130, 1696133
milestone88.0a1
first release with
nightly linux32
aada018490c4 / 88.0a1 / 20210304092248 / files
nightly linux64
aada018490c4 / 88.0a1 / 20210304092248 / files
nightly mac
aada018490c4 / 88.0a1 / 20210304092248 / files
nightly win32
aada018490c4 / 88.0a1 / 20210304092248 / files
nightly win64
aada018490c4 / 88.0a1 / 20210304092248 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1695883 - Add explicit fallible mozStorageTransaction::Start method for starting the transaction; r=dom-storage-reviewers,sg The new method is mandatory because mozStorageTransaction constructor no longer starts the transaction. It must be started explicitely. All consumers have been adjusted, but only dom/quota, dom/indexedDB, dom/cache, dom/localstorage and dom/storage handle the error. Other components like netwerk/cache, netwerk/cookie and toolkit/components currently only warn on failure to start a transaction. Bug 1696129, 1696130 and 1696133 have been filed for proper handling of transaction start failures in those components. Differential Revision: https://phabricator.services.mozilla.com/D106893
dom/cache/DBSchema.cpp
dom/cache/Manager.cpp
dom/indexedDB/ActorsParent.cpp
dom/localstorage/ActorsParent.cpp
dom/quota/ActorsParent.cpp
dom/storage/StorageDBThread.cpp
dom/storage/StorageDBUpdater.cpp
netwerk/cache/nsDiskCacheDeviceSQL.cpp
netwerk/cookie/CookiePersistentStorage.cpp
storage/mozStorageHelper.h
storage/test/gtest/test_transaction_helper.cpp
toolkit/components/places/Database.cpp
toolkit/components/places/FaviconHelpers.cpp
toolkit/components/places/History.cpp
toolkit/components/places/nsNavBookmarks.cpp
toolkit/components/places/nsNavHistory.cpp
--- a/dom/cache/DBSchema.cpp
+++ b/dom/cache/DBSchema.cpp
@@ -452,16 +452,18 @@ nsresult CreateOrMigrateSchema(mozIStora
   }
 
   // Turn off checking foreign keys before starting a transaction, and restore
   // it once we're done.
   AutoDisableForeignKeyChecking restoreForeignKeyChecking(&aConn);
   mozStorageTransaction trans(&aConn, false,
                               mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+  CACHE_TRY(trans.Start());
+
   const bool migrating = schemaVersion != 0;
 
   if (migrating) {
     // A schema exists, but its not the current version.  Attempt to
     // migrate it to our new schema.
     CACHE_TRY(Migrate(aConn));
   } else {
     // There is no schema installed.  Create the database from scratch.
--- a/dom/cache/Manager.cpp
+++ b/dom/cache/Manager.cpp
@@ -93,16 +93,18 @@ class SetupAction final : public SyncDBA
     //
     // Note, this must be done after any schema version updates to
     // ensure our DBSchema methods work correctly.
     if (MarkerFileExists(aQuotaInfo)) {
       NS_WARNING("Cache not shutdown cleanly! Cleaning up stale data...");
       mozStorageTransaction trans(aConn, false,
                                   mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+      CACHE_TRY(trans.Start());
+
       // Clean up orphaned Cache objects
       CACHE_TRY_INSPECT(const auto& orphanedCacheIdList,
                         db::FindOrphanedCacheIds(*aConn));
 
       CACHE_TRY_INSPECT(
           const CheckedInt64& overallDeletedPaddingSize,
           Reduce(
               orphanedCacheIdList, CheckedInt64(0),
@@ -535,16 +537,18 @@ class Manager::DeleteOrphanedCacheAction
   virtual nsresult RunSyncWithDBOnTarget(
       const QuotaInfo& aQuotaInfo, nsIFile* aDBDir,
       mozIStorageConnection* aConn) override {
     mQuotaInfo.emplace(aQuotaInfo);
 
     mozStorageTransaction trans(aConn, false,
                                 mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    CACHE_TRY(trans.Start());
+
     CACHE_TRY_UNWRAP(mDeletionInfo, db::DeleteCacheId(*aConn, mCacheId));
 
     CACHE_TRY(MaybeUpdatePaddingFile(
         aDBDir, aConn, /* aIncreaceSize */ 0, mDeletionInfo.mDeletedPaddingSize,
         [&trans]() mutable { return trans.Commit(); }));
 
     return NS_OK;
   }
@@ -842,16 +846,18 @@ class Manager::CachePutAllAction final :
     if (NS_FAILED(mAsyncResult)) {
       DoResolve(mAsyncResult);
       return;
     }
 
     mozStorageTransaction trans(mConn, false,
                                 mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    CACHE_TRY(trans.Start(), QM_VOID);
+
     const nsresult rv = [this, &trans]() -> nsresult {
       CACHE_TRY(CollectEachInRange(mList, [this](Entry& e) -> nsresult {
         if (e.mRequestStream) {
           CACHE_TRY(BodyFinalizeWrite(*mDBDir, e.mRequestBodyId));
         }
         if (e.mResponseStream) {
           // Gerenate padding size for opaque response if needed.
           if (e.mResponse.type() == ResponseType::Opaque) {
@@ -1100,16 +1106,18 @@ class Manager::CacheDeleteAction final :
   virtual nsresult RunSyncWithDBOnTarget(
       const QuotaInfo& aQuotaInfo, nsIFile* aDBDir,
       mozIStorageConnection* aConn) override {
     mQuotaInfo.emplace(aQuotaInfo);
 
     mozStorageTransaction trans(aConn, false,
                                 mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    CACHE_TRY(trans.Start());
+
     CACHE_TRY_UNWRAP(
         auto maybeDeletionInfo,
         db::CacheDelete(*aConn, mCacheId, mArgs.request(), mArgs.params()));
 
     mSuccess = maybeDeletionInfo.isSome();
     if (mSuccess) {
       mDeletionInfo = std::move(maybeDeletionInfo.ref());
     }
@@ -1321,16 +1329,18 @@ class Manager::StorageOpenAction final :
 
   virtual nsresult RunSyncWithDBOnTarget(
       const QuotaInfo& aQuotaInfo, nsIFile* aDBDir,
       mozIStorageConnection* aConn) override {
     // Cache does not exist, create it instead
     mozStorageTransaction trans(aConn, false,
                                 mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    CACHE_TRY(trans.Start());
+
     // Look for existing cache
     CACHE_TRY_INSPECT(const auto& maybeCacheId,
                       db::StorageGetCacheId(*aConn, mNamespace, mArgs.key()));
 
     if (maybeCacheId.isSome()) {
       mCacheId = maybeCacheId.ref();
       MOZ_DIAGNOSTIC_ASSERT(mCacheId != INVALID_CACHE_ID);
       return NS_OK;
@@ -1372,16 +1382,18 @@ class Manager::StorageDeleteAction final
         mCacheId(INVALID_CACHE_ID) {}
 
   virtual nsresult RunSyncWithDBOnTarget(
       const QuotaInfo& aQuotaInfo, nsIFile* aDBDir,
       mozIStorageConnection* aConn) override {
     mozStorageTransaction trans(aConn, false,
                                 mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    CACHE_TRY(trans.Start());
+
     CACHE_TRY_INSPECT(const auto& maybeCacheId,
                       db::StorageGetCacheId(*aConn, mNamespace, mArgs.key()));
 
     if (maybeCacheId.isNothing()) {
       mCacheDeleted = false;
       return NS_OK;
     }
     mCacheId = maybeCacheId.ref();
--- a/dom/indexedDB/ActorsParent.cpp
+++ b/dom/indexedDB/ActorsParent.cpp
@@ -923,16 +923,18 @@ CreateStorageConnection(nsIFile& aDBFile
 #endif
     }
 
     bool vacuumNeeded = false;
 
     mozStorageTransaction transaction(
         connection.get(), false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    IDB_TRY(transaction.Start());
+
     if (newDatabase) {
       IDB_TRY(CreateTables(*connection));
 
 #ifdef DEBUG
       {
         IDB_TRY_INSPECT(const int32_t& schemaVersion,
                         MOZ_TO_RESULT_INVOKE(connection, GetSchemaVersion),
                         QM_ASSERT_UNREACHABLE);
@@ -12339,16 +12341,18 @@ nsresult FileManager::InitDirectory(nsIF
     if (hasJournals) {
       IDB_TRY_UNWRAP(const NotNull<nsCOMPtr<mozIStorageConnection>> connection,
                      CreateStorageConnection(
                          aDatabaseFile, aDirectory, VoidString(), aOrigin,
                          /* aDirectoryLockId */ -1, aTelemetryId, Nothing{}));
 
       mozStorageTransaction transaction(connection.get(), false);
 
+      IDB_TRY(transaction.Start())
+
       IDB_TRY(connection->ExecuteSimpleSQL(
           "CREATE VIRTUAL TABLE fs USING filesystem;"_ns));
 
       // The parameter names are not used, parameters are bound by index only
       // locally in the same function.
       IDB_TRY_INSPECT(
           const auto& stmt,
           MOZ_TO_RESULT_INVOKE_TYPED(
@@ -14088,16 +14092,18 @@ nsresult DatabaseMaintenance::DetermineM
     return NS_OK;
   }
 
   // This method shouldn't make any permanent changes to the database, so make
   // sure everything gets rolled back when we leave.
   mozStorageTransaction transaction(&aConnection,
                                     /* aCommitOnComplete */ false);
 
+  IDB_TRY(transaction.Start())
+
   // Check to see when we last vacuumed this database.
   IDB_TRY_INSPECT(const auto& stmt,
                   CreateAndExecuteSingleStepStatement(
                       aConnection,
                       "SELECT last_vacuum_time, last_vacuum_size "
                       "FROM database;"_ns));
 
   IDB_TRY_INSPECT(const PRTime& lastVacuumTime,
--- a/dom/localstorage/ActorsParent.cpp
+++ b/dom/localstorage/ActorsParent.cpp
@@ -525,16 +525,18 @@ Result<nsCOMPtr<mozIStorageConnection>, 
           "PRAGMA auto_vacuum = INCREMENTAL;"_ns
 #endif
           ));
     }
 
     mozStorageTransaction transaction(
         connection, false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    LS_TRY(transaction.Start())
+
     if (newDatabase) {
       LS_TRY(CreateTables(connection));
 
 #ifdef DEBUG
       {
         LS_TRY_INSPECT(const int32_t& schemaVersion,
                        MOZ_TO_RESULT_INVOKE(connection, GetSchemaVersion),
                        QM_ASSERT_UNREACHABLE);
@@ -6888,16 +6890,18 @@ nsresult PrepareDatastoreOp::DatabaseWor
 
     auto autoUpdateSize = MakeScopeExit([&quotaObject] {
       MOZ_ALWAYS_TRUE(quotaObject->MaybeUpdateSize(0, /* aTruncate */ true));
     });
 
     mozStorageTransaction transaction(
         connection, false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    LS_TRY(transaction.Start())
+
     {
       nsCOMPtr<mozIStorageFunction> function = new CompressFunction();
 
       LS_TRY(connection->CreateFunction("compress"_ns, 1, function));
 
       function = new CompressibleFunction();
 
       LS_TRY(connection->CreateFunction("compressible"_ns, 1, function));
--- a/dom/quota/ActorsParent.cpp
+++ b/dom/quota/ActorsParent.cpp
@@ -461,16 +461,18 @@ Result<bool, nsresult> MaybeCreateOrUpgr
   if (cacheVersion > kCacheVersion) {
     cacheUsable = false;
   } else if (cacheVersion != kCacheVersion) {
     const bool newCache = !cacheVersion;
 
     mozStorageTransaction transaction(
         &aConnection, false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    QM_TRY(transaction.Start());
+
     if (newCache) {
       QM_TRY(CreateCacheTables(&aConnection));
 
 #ifdef DEBUG
       {
         QM_TRY_INSPECT(const int32_t& cacheVersion,
                        LoadCacheVersion(aConnection));
         MOZ_ASSERT(cacheVersion == kCacheVersion);
@@ -536,16 +538,17 @@ nsresult InvalidateCache(mozIStorageConn
   static constexpr auto kDeleteCacheQuery = "DELETE FROM origin;"_ns;
   static constexpr auto kSetInvalidFlagQuery = "UPDATE cache SET valid = 0"_ns;
 
   // XXX Use QM_TRY_OR_WARN here in/after Bug 1686191.
   QM_TRY(([&]() -> Result<Ok, nsresult> {
     mozStorageTransaction transaction(
         &aConnection, false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    QM_TRY(transaction.Start());
     QM_TRY(aConnection.ExecuteSimpleSQL(kDeleteCacheQuery));
     QM_TRY(aConnection.ExecuteSimpleSQL(kSetInvalidFlagQuery));
     QM_TRY(transaction.Commit());
 
     return Ok{};
   }()
                        .orElse([&](const nsresult rv) -> Result<Ok, nsresult> {
                          QM_TRY(aConnection.ExecuteSimpleSQL(
@@ -4210,16 +4213,18 @@ void QuotaManager::UnloadQuota() {
   MOZ_ASSERT(mTemporaryStorageInitialized);
   MOZ_ASSERT(mCacheUsable);
 
   auto autoRemoveQuota = MakeScopeExit([&] { RemoveQuota(); });
 
   mozStorageTransaction transaction(
       mStorageConnection, false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+  QM_TRY(transaction.Start(), QM_VOID);
+
   QM_TRY(mStorageConnection->ExecuteSimpleSQL("DELETE FROM origin;"_ns),
          QM_VOID);
 
   nsCOMPtr<mozIStorageStatement> insertStmt;
 
   {
     MutexAutoLock lock(mQuotaMutex);
 
@@ -5701,16 +5706,18 @@ nsresult QuotaManager::MaybeCreateOrUpgr
         QM_TRY(aConnection.ExecuteSimpleSQL(nsPrintfCString(
             "PRAGMA page_size = %" PRIu32 ";", kSQLitePageSizeOverride)));
       }
     }
 
     mozStorageTransaction transaction(
         &aConnection, false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    QM_TRY(transaction.Start());
+
     // An upgrade method can upgrade the database, the storage or both.
     // The upgrade loop below can only be avoided when there's no database and
     // no storage yet (e.g. new profile).
     if (newDatabase && newDirectory) {
       QM_TRY(CreateTables(&aConnection));
 
 #ifdef DEBUG
       {
--- a/dom/storage/StorageDBThread.cpp
+++ b/dom/storage/StorageDBThread.cpp
@@ -1409,17 +1409,20 @@ bool StorageDBThread::PendingOperations:
   return !!mExecList.Length();
 }
 
 nsresult StorageDBThread::PendingOperations::Execute(StorageDBThread* aThread) {
   // Called outside the lock
 
   mozStorageTransaction transaction(aThread->mWorkerConnection, false);
 
-  nsresult rv;
+  nsresult rv = transaction.Start();
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
 
   for (uint32_t i = 0; i < mExecList.Length(); ++i) {
     const auto& task = mExecList[i];
     rv = task->Perform(aThread);
     if (NS_FAILED(rv)) {
       return rv;
     }
   }
--- a/dom/storage/StorageDBUpdater.cpp
+++ b/dom/storage/StorageDBUpdater.cpp
@@ -276,16 +276,19 @@ nsresult CreateCurrentSchemaOnEmptyTable
 
 }  // namespace
 
 namespace StorageDBUpdater {
 
 nsresult CreateCurrentSchema(mozIStorageConnection* aConnection) {
   mozStorageTransaction transaction(aConnection, false);
 
+  nsresult rv = transaction.Start();
+  NS_ENSURE_SUCCESS(rv, rv);
+
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
   {
     int32_t schemaVer;
     nsresult rv = aConnection->GetSchemaVersion(&schemaVer);
     NS_ENSURE_SUCCESS(rv, rv);
 
     MOZ_DIAGNOSTIC_ASSERT(0 == schemaVer);
 
@@ -294,29 +297,30 @@ nsresult CreateCurrentSchema(mozIStorage
                      &moz_webappsstoreExists);
     NS_ENSURE_SUCCESS(rv, rv);
 
     MOZ_DIAGNOSTIC_ASSERT(!webappsstore2Exists && !webappsstoreExists &&
                           !moz_webappsstoreExists);
   }
 #endif
 
-  nsresult rv = CreateCurrentSchemaOnEmptyTableInternal(aConnection);
+  rv = CreateCurrentSchemaOnEmptyTableInternal(aConnection);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = transaction.Commit();
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
 nsresult Update(mozIStorageConnection* aWorkerConnection) {
-  nsresult rv;
+  mozStorageTransaction transaction(aWorkerConnection, false);
 
-  mozStorageTransaction transaction(aWorkerConnection, false);
+  nsresult rv = transaction.Start();
+  NS_ENSURE_SUCCESS(rv, rv);
 
   bool doVacuum = false;
 
   int32_t schemaVer;
   rv = aWorkerConnection->GetSchemaVersion(&schemaVer);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // downgrade (v0) -> upgrade (v1+) specific code
--- a/netwerk/cache/nsDiskCacheDeviceSQL.cpp
+++ b/netwerk/cache/nsDiskCacheDeviceSQL.cpp
@@ -755,16 +755,19 @@ NS_IMETHODIMP
 nsApplicationCache::AddNamespaces(nsIArray* namespaces) {
   NS_ENSURE_TRUE(mValid, NS_ERROR_NOT_AVAILABLE);
   NS_ENSURE_TRUE(mDevice, NS_ERROR_NOT_AVAILABLE);
 
   if (!namespaces) return NS_OK;
 
   mozStorageTransaction transaction(mDevice->mDB, false);
 
+  // XXX Handle the error, bug 1696129.
+  Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
   uint32_t length;
   nsresult rv = namespaces->GetLength(&length);
   NS_ENSURE_SUCCESS(rv, rv);
 
   for (uint32_t i = 0; i < length; i++) {
     nsCOMPtr<nsIApplicationCacheNamespace> ns =
         do_QueryElementAt(namespaces, i);
     if (ns) {
--- a/netwerk/cookie/CookiePersistentStorage.cpp
+++ b/netwerk/cookie/CookiePersistentStorage.cpp
@@ -449,27 +449,33 @@ void CookiePersistentStorage::HandleCorr
     }
   }
 }
 
 void CookiePersistentStorage::RemoveCookiesWithOriginAttributes(
     const OriginAttributesPattern& aPattern, const nsACString& aBaseDomain) {
   mozStorageTransaction transaction(mDBConn, false);
 
+  // XXX Handle the error, bug 1696130.
+  Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
   CookieStorage::RemoveCookiesWithOriginAttributes(aPattern, aBaseDomain);
 
   DebugOnly<nsresult> rv = transaction.Commit();
   MOZ_ASSERT(NS_SUCCEEDED(rv));
 }
 
 void CookiePersistentStorage::RemoveCookiesFromExactHost(
     const nsACString& aHost, const nsACString& aBaseDomain,
     const OriginAttributesPattern& aPattern) {
   mozStorageTransaction transaction(mDBConn, false);
 
+  // XXX Handle the error, bug 1696130.
+  Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
   CookieStorage::RemoveCookiesFromExactHost(aHost, aBaseDomain, aPattern);
 
   DebugOnly<nsresult> rv = transaction.Commit();
   MOZ_ASSERT(NS_SUCCEEDED(rv));
 }
 
 void CookiePersistentStorage::RemoveCookieFromDB(const CookieListIter& aIter) {
   // if it's a non-session cookie, remove it from the db
@@ -827,16 +833,19 @@ CookiePersistentStorage::OpenDBResult Co
     // table already exists; check the schema version before reading
     int32_t dbSchemaVersion;
     rv = mSyncConn->GetSchemaVersion(&dbSchemaVersion);
     NS_ENSURE_SUCCESS(rv, RESULT_RETRY);
 
     // Start a transaction for the whole migration block.
     mozStorageTransaction transaction(mSyncConn, true);
 
+    // XXX Handle the error, bug 1696130.
+    Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
     switch (dbSchemaVersion) {
       // Upgrading.
       // Every time you increment the database schema, you need to implement
       // the upgrading code from the previous version to the new one. If
       // migration fails for any reason, it's a bug -- so we return RESULT_RETRY
       // such that the original database will be saved, in the hopes that we
       // might one day see it and fix it.
       case 1: {
@@ -1974,16 +1983,19 @@ nsresult CookiePersistentStorage::Create
 nsresult CookiePersistentStorage::RunInTransaction(
     nsICookieTransactionCallback* aCallback) {
   if (NS_WARN_IF(!mDBConn)) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   mozStorageTransaction transaction(mDBConn, true);
 
+  // XXX Handle the error, bug 1696130.
+  Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
   if (NS_FAILED(aCallback->Callback())) {
     Unused << transaction.Rollback();
     return NS_ERROR_FAILURE;
   }
 
   return NS_OK;
 }
 
--- a/storage/mozStorageHelper.h
+++ b/storage/mozStorageHelper.h
@@ -4,16 +4,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef MOZSTORAGEHELPER_H
 #define MOZSTORAGEHELPER_H
 
 #include "nsCOMPtr.h"
 #include "nsString.h"
 #include "mozilla/DebugOnly.h"
+#include "mozilla/ScopeExit.h"
 
 #include "mozilla/storage/SQLiteMutex.h"
 #include "mozIStorageConnection.h"
 #include "mozIStorageStatement.h"
 #include "mozIStoragePendingStatement.h"
 #include "nsError.h"
 
 /**
@@ -60,82 +61,108 @@ class mozStorageTransaction {
   using SQLiteMutexAutoLock = mozilla::storage::SQLiteMutexAutoLock;
 
  public:
   mozStorageTransaction(
       mozIStorageConnection* aConnection, bool aCommitOnComplete,
       int32_t aType = mozIStorageConnection::TRANSACTION_DEFAULT,
       bool aAsyncCommit = false)
       : mConnection(aConnection),
+        mType(aType),
         mNestingLevel(0),
         mHasTransaction(false),
         mCommitOnComplete(aCommitOnComplete),
         mCompleted(false),
-        mAsyncCommit(aAsyncCommit) {
-    if (mConnection) {
-      SQLiteMutexAutoLock lock(mConnection->GetSharedDBMutex());
-
-      // We nee to speculatively set the nesting level to be able to decide
-      // if this is a top level transaction and to be able to generate the
-      // savepoint name.
-      TransactionStarted(lock);
-
-      nsAutoCString query;
-
-      if (TopLevelTransaction(lock)) {
-        query.Assign("BEGIN");
-        int32_t type = aType;
-        if (type == mozIStorageConnection::TRANSACTION_DEFAULT) {
-          MOZ_ALWAYS_SUCCEEDS(mConnection->GetDefaultTransactionType(&type));
-        }
-        switch (type) {
-          case mozIStorageConnection::TRANSACTION_IMMEDIATE:
-            query.AppendLiteral(" IMMEDIATE");
-            break;
-          case mozIStorageConnection::TRANSACTION_EXCLUSIVE:
-            query.AppendLiteral(" EXCLUSIVE");
-            break;
-          case mozIStorageConnection::TRANSACTION_DEFERRED:
-            query.AppendLiteral(" DEFERRED");
-            break;
-          default:
-            MOZ_ASSERT(false, "Unknown transaction type");
-        }
-      } else {
-        query.Assign("SAVEPOINT sp"_ns + IntToCString(mNestingLevel));
-      }
-
-      // If the query fails to execute we need to revert the speculatively set
-      // nesting level on the connection.
-      if (NS_FAILED(mConnection->ExecuteSimpleSQL(query))) {
-        TransactionFinished(lock);
-      }
-    }
-  }
+        mAsyncCommit(aAsyncCommit) {}
 
   ~mozStorageTransaction() {
     if (mConnection && mHasTransaction && !mCompleted) {
       if (mCommitOnComplete) {
         mozilla::DebugOnly<nsresult> rv = Commit();
         NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
                              "A transaction didn't commit correctly");
       } else {
         mozilla::DebugOnly<nsresult> rv = Rollback();
         NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
                              "A transaction didn't rollback correctly");
       }
     }
   }
 
   /**
+   * Starts the transaction.
+   */
+  nsresult Start() {
+    // XXX We should probably get rid of mHasTransaction and use mConnection
+    // for checking if a transaction has been started. However, we need to
+    // first stop supporting null mConnection and also move aConnection from
+    // the constructor to Start.
+    MOZ_DIAGNOSTIC_ASSERT(!mHasTransaction);
+
+    // XXX We should probably stop supporting null mConnection.
+
+    // XXX We should probably get rid of mCompleted and allow to start the
+    // transaction again if it was already committed or rolled back.
+    if (!mConnection || mCompleted) {
+      return NS_OK;
+    }
+
+    SQLiteMutexAutoLock lock(mConnection->GetSharedDBMutex());
+
+    // We nee to speculatively set the nesting level to be able to decide
+    // if this is a top level transaction and to be able to generate the
+    // savepoint name.
+    TransactionStarted(lock);
+
+    // If there's a failure we need to revert the speculatively set nesting
+    // level on the connection.
+    auto autoFinishTransaction =
+        mozilla::MakeScopeExit([&] { TransactionFinished(lock); });
+
+    nsAutoCString query;
+
+    if (TopLevelTransaction(lock)) {
+      query.Assign("BEGIN");
+      int32_t type = mType;
+      if (type == mozIStorageConnection::TRANSACTION_DEFAULT) {
+        MOZ_ALWAYS_SUCCEEDS(mConnection->GetDefaultTransactionType(&type));
+      }
+      switch (type) {
+        case mozIStorageConnection::TRANSACTION_IMMEDIATE:
+          query.AppendLiteral(" IMMEDIATE");
+          break;
+        case mozIStorageConnection::TRANSACTION_EXCLUSIVE:
+          query.AppendLiteral(" EXCLUSIVE");
+          break;
+        case mozIStorageConnection::TRANSACTION_DEFERRED:
+          query.AppendLiteral(" DEFERRED");
+          break;
+        default:
+          MOZ_ASSERT(false, "Unknown transaction type");
+      }
+    } else {
+      query.Assign("SAVEPOINT sp"_ns + IntToCString(mNestingLevel));
+    }
+
+    nsresult rv = mConnection->ExecuteSimpleSQL(query);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    autoFinishTransaction.release();
+
+    return NS_OK;
+  }
+
+  /**
    * Commits the transaction if one is in progress. If one is not in progress,
    * this is a NOP since the actual owner of the transaction outside of our
    * scope is in charge of finally committing or rolling back the transaction.
    */
   nsresult Commit() {
+    // XXX Assert instead of returning NS_OK if the transaction hasn't been
+    // started.
     if (!mConnection || mCompleted || !mHasTransaction) return NS_OK;
 
     SQLiteMutexAutoLock lock(mConnection->GetSharedDBMutex());
 
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
     MOZ_DIAGNOSTIC_ASSERT(CurrentTransaction(lock));
 #else
     if (!CurrentTransaction(lock)) {
@@ -157,29 +184,31 @@ class mozStorageTransaction {
       } else {
         rv = mConnection->ExecuteSimpleSQL("COMMIT"_ns);
       }
     } else {
       rv = mConnection->ExecuteSimpleSQL("RELEASE sp"_ns +
                                          IntToCString(mNestingLevel));
     }
 
-    if (NS_SUCCEEDED(rv)) {
-      TransactionFinished(lock);
-    }
+    NS_ENSURE_SUCCESS(rv, rv);
 
-    return rv;
+    TransactionFinished(lock);
+
+    return NS_OK;
   }
 
   /**
    * Rolls back the transaction if one is in progress. If one is not in
    * progress, this is a NOP since the actual owner of the transaction outside
    * of our scope is in charge of finally rolling back the transaction.
    */
   nsresult Rollback() {
+    // XXX Assert instead of returning NS_OK if the transaction hasn't been
+    // started.
     if (!mConnection || mCompleted || !mHasTransaction) return NS_OK;
 
     SQLiteMutexAutoLock lock(mConnection->GetSharedDBMutex());
 
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
     MOZ_DIAGNOSTIC_ASSERT(CurrentTransaction(lock));
 #else
     if (!CurrentTransaction(lock)) {
@@ -200,21 +229,21 @@ class mozStorageTransaction {
       } while (rv == NS_ERROR_STORAGE_BUSY);
     } else {
       const auto nestingLevelCString = IntToCString(mNestingLevel);
       rv = mConnection->ExecuteSimpleSQL(
           "ROLLBACK TO sp"_ns + nestingLevelCString + "; RELEASE sp"_ns +
           nestingLevelCString);
     }
 
-    if (NS_SUCCEEDED(rv)) {
-      TransactionFinished(lock);
-    }
+    NS_ENSURE_SUCCESS(rv, rv);
 
-    return rv;
+    TransactionFinished(lock);
+
+    return NS_OK;
   }
 
  protected:
   void TransactionStarted(const SQLiteMutexAutoLock& aProofOfLock) {
     MOZ_ASSERT(mConnection);
     MOZ_ASSERT(!mHasTransaction);
     MOZ_ASSERT(mNestingLevel == 0);
     mHasTransaction = true;
@@ -243,16 +272,17 @@ class mozStorageTransaction {
     MOZ_ASSERT(mNestingLevel > 0);
     MOZ_ASSERT(CurrentTransaction(aProofOfLock));
     mConnection->DecreaseTransactionNestingLevel(aProofOfLock);
     mNestingLevel = 0;
     mHasTransaction = false;
   }
 
   nsCOMPtr<mozIStorageConnection> mConnection;
+  int32_t mType;
   uint32_t mNestingLevel;
   bool mHasTransaction;
   bool mCommitOnComplete;
   bool mCompleted;
   bool mAsyncCommit;
 };
 
 /**
--- a/storage/test/gtest/test_transaction_helper.cpp
+++ b/storage/test/gtest/test_transaction_helper.cpp
@@ -23,16 +23,17 @@ bool has_transaction(mozIStorageConnecti
 TEST(storage_transaction_helper, Commit)
 {
   nsCOMPtr<mozIStorageConnection> db(getMemoryDatabase());
 
   // Create a table in a transaction, call Commit, and make sure that it does
   // exists after the transaction falls out of scope.
   {
     mozStorageTransaction transaction(db, false);
+    do_check_success(transaction.Start());
     do_check_true(has_transaction(db));
     (void)db->ExecuteSimpleSQL("CREATE TABLE test (id INTEGER PRIMARY KEY)"_ns);
     (void)transaction.Commit();
   }
   do_check_false(has_transaction(db));
 
   bool exists = false;
   (void)db->TableExists("test"_ns, &exists);
@@ -42,16 +43,17 @@ TEST(storage_transaction_helper, Commit)
 TEST(storage_transaction_helper, Rollback)
 {
   nsCOMPtr<mozIStorageConnection> db(getMemoryDatabase());
 
   // Create a table in a transaction, call Rollback, and make sure that it does
   // not exists after the transaction falls out of scope.
   {
     mozStorageTransaction transaction(db, true);
+    do_check_success(transaction.Start());
     do_check_true(has_transaction(db));
     (void)db->ExecuteSimpleSQL("CREATE TABLE test (id INTEGER PRIMARY KEY)"_ns);
     (void)transaction.Rollback();
   }
   do_check_false(has_transaction(db));
 
   bool exists = true;
   (void)db->TableExists("test"_ns, &exists);
@@ -61,16 +63,17 @@ TEST(storage_transaction_helper, Rollbac
 TEST(storage_transaction_helper, AutoCommit)
 {
   nsCOMPtr<mozIStorageConnection> db(getMemoryDatabase());
 
   // Create a table in a transaction, and make sure that it exists after the
   // transaction falls out of scope.  This means the Commit was successful.
   {
     mozStorageTransaction transaction(db, true);
+    do_check_success(transaction.Start());
     do_check_true(has_transaction(db));
     (void)db->ExecuteSimpleSQL("CREATE TABLE test (id INTEGER PRIMARY KEY)"_ns);
   }
   do_check_false(has_transaction(db));
 
   bool exists = false;
   (void)db->TableExists("test"_ns, &exists);
   do_check_true(exists);
@@ -80,31 +83,33 @@ TEST(storage_transaction_helper, AutoRol
 {
   nsCOMPtr<mozIStorageConnection> db(getMemoryDatabase());
 
   // Create a table in a transaction, and make sure that it does not exists
   // after the transaction falls out of scope.  This means the Rollback was
   // successful.
   {
     mozStorageTransaction transaction(db, false);
+    do_check_success(transaction.Start());
     do_check_true(has_transaction(db));
     (void)db->ExecuteSimpleSQL("CREATE TABLE test (id INTEGER PRIMARY KEY)"_ns);
   }
   do_check_false(has_transaction(db));
 
   bool exists = true;
   (void)db->TableExists("test"_ns, &exists);
   do_check_false(exists);
 }
 
 TEST(storage_transaction_helper, null_database_connection)
 {
   // We permit the use of the Transaction helper when passing a null database
   // in, so we need to make sure this still works without crashing.
   mozStorageTransaction transaction(nullptr, false);
+  do_check_success(transaction.Start());
   do_check_true(NS_SUCCEEDED(transaction.Commit()));
   do_check_true(NS_SUCCEEDED(transaction.Rollback()));
 }
 
 TEST(storage_transaction_helper, async_Commit)
 {
   HookSqliteMutex hook;
 
@@ -113,16 +118,17 @@ TEST(storage_transaction_helper, async_C
   // -- wedge the thread
   nsCOMPtr<nsIThread> target(get_conn_async_thread(db));
   do_check_true(target);
   RefPtr<ThreadWedger> wedger(new ThreadWedger(target));
 
   {
     mozStorageTransaction transaction(
         db, false, mozIStorageConnection::TRANSACTION_DEFERRED, true);
+    do_check_success(transaction.Start());
     do_check_true(has_transaction(db));
     (void)db->ExecuteSimpleSQL("CREATE TABLE test (id INTEGER PRIMARY KEY)"_ns);
     (void)transaction.Commit();
   }
   do_check_true(has_transaction(db));
 
   // -- unwedge the async thread
   wedger->unwedge();
@@ -141,22 +147,24 @@ TEST(storage_transaction_helper, async_C
 }
 
 TEST(storage_transaction_helper, Nesting)
 {
   nsCOMPtr<mozIStorageConnection> db(getMemoryDatabase());
 
   {
     mozStorageTransaction transaction(db, false);
+    do_check_success(transaction.Start());
     do_check_true(has_transaction(db));
     do_check_success(
         db->ExecuteSimpleSQL("CREATE TABLE test (id INTEGER PRIMARY KEY)"_ns));
 
     {
       mozStorageTransaction nestedTransaction(db, false);
+      do_check_success(nestedTransaction.Start());
       do_check_true(has_transaction(db));
       do_check_success(db->ExecuteSimpleSQL(
           "CREATE TABLE nested_test (id INTEGER PRIMARY KEY)"_ns));
 
 #ifndef MOZ_DIAGNOSTIC_ASSERT_ENABLED
       do_check_true(transaction.Commit() == NS_ERROR_NOT_AVAILABLE);
       do_check_true(transaction.Rollback() == NS_ERROR_NOT_AVAILABLE);
 #endif
--- a/toolkit/components/places/Database.cpp
+++ b/toolkit/components/places/Database.cpp
@@ -695,16 +695,18 @@ nsresult Database::EnsureFaviconsDatabas
     rv = conn->GetDefaultPageSize(&defaultPageSize);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = SetupDurability(conn, defaultPageSize);
     NS_ENSURE_SUCCESS(rv, rv);
 
     // We are going to update the database, so everything from now on should be
     // in a transaction for performances.
     mozStorageTransaction transaction(conn, false);
+    // XXX Handle the error, bug 1696133.
+    Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
     rv = conn->ExecuteSimpleSQL(CREATE_MOZ_ICONS);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = conn->ExecuteSimpleSQL(CREATE_IDX_MOZ_ICONS_ICONURLHASH);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = conn->ExecuteSimpleSQL(CREATE_MOZ_PAGES_W_ICONS);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = conn->ExecuteSimpleSQL(CREATE_IDX_MOZ_PAGES_W_ICONS_ICONURLHASH);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -887,16 +889,19 @@ nsresult Database::TryToCloneTablesFromC
 
   rv = aStorage->OpenUnsharedDatabase(recoverFile, getter_AddRefs(conn));
   NS_ENSURE_SUCCESS(rv, rv);
   rv = AttachDatabase(conn, NS_ConvertUTF16toUTF8(path), "corrupt"_ns);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mozStorageTransaction transaction(conn, false);
 
+  // XXX Handle the error, bug 1696133.
+  Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
   // Copy the schema version.
   nsCOMPtr<mozIStorageStatement> stmt;
   (void)conn->CreateStatement("PRAGMA corrupt.user_version"_ns,
                               getter_AddRefs(stmt));
   NS_ENSURE_TRUE(stmt, NS_ERROR_OUT_OF_MEMORY);
   bool hasResult;
   rv = stmt->ExecuteStep(&hasResult);
   NS_ENSURE_SUCCESS(rv, rv);
@@ -1078,16 +1083,19 @@ nsresult Database::InitSchema(bool* aDat
     // regardless of its success.
     MigrateV52OriginFrecencies();
   });
 
   // We are going to update the database, so everything from now on should be in
   // a transaction for performances.
   mozStorageTransaction transaction(mMainConn, false);
 
+  // XXX Handle the error, bug 1696133.
+  Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
   if (databaseInitialized) {
     // Migration How-to:
     //
     // 1. increment PLACES_SCHEMA_VERSION.
     // 2. implement a method that performs upgrade to your version from the
     //    previous one.
     //
     // NOTE: The downgrade process is pretty much complicated by the fact old
--- a/toolkit/components/places/FaviconHelpers.cpp
+++ b/toolkit/components/places/FaviconHelpers.cpp
@@ -817,18 +817,23 @@ AsyncAssociateIconToPage::Run() {
         shouldUpdateIcon = true;
         break;
       }
     }
   }
 
   RefPtr<Database> DB = Database::GetDatabase();
   NS_ENSURE_STATE(DB);
+
   mozStorageTransaction transaction(
       DB->MainConn(), false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
+
+  // XXX Handle the error, bug 1696133.
+  Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
   nsresult rv;
   if (shouldUpdateIcon) {
     rv = SetIconInfo(DB, mIcon);
     if (NS_FAILED(rv)) {
       (void)transaction.Commit();
       return rv;
     }
 
@@ -1049,16 +1054,20 @@ NS_IMETHODIMP
 AsyncReplaceFaviconData::Run() {
   MOZ_ASSERT(!NS_IsMainThread());
 
   RefPtr<Database> DB = Database::GetDatabase();
   NS_ENSURE_STATE(DB);
 
   mozStorageTransaction transaction(
       DB->MainConn(), false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
+
+  // XXX Handle the error, bug 1696133.
+  Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
   nsresult rv = SetIconInfo(DB, mIcon, true);
   if (rv == NS_ERROR_NOT_AVAILABLE) {
     // There's no previous icon to replace, we don't need to do anything.
     (void)transaction.Commit();
     return NS_OK;
   }
   NS_ENSURE_SUCCESS(rv, rv);
   rv = transaction.Commit();
--- a/toolkit/components/places/History.cpp
+++ b/toolkit/components/places/History.cpp
@@ -814,16 +814,19 @@ class InsertVisitedURIs final : public R
     // Check if we were already shutting down.
     if (mHistory->IsShuttingDown()) {
       return NS_OK;
     }
 
     mozStorageTransaction transaction(
         mDBConn, false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    // XXX Handle the error, bug 1696133.
+    Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
     const VisitData* lastFetchedPlace = nullptr;
     uint32_t lastFetchedVisitCount = 0;
     bool shouldChunkNotifications = mPlaces.Length() > NOTIFY_VISITS_CHUNK_SIZE;
     nsTArray<VisitData> notificationChunk;
     if (shouldChunkNotifications) {
       notificationChunk.SetCapacity(NOTIFY_VISITS_CHUNK_SIZE);
     }
     for (nsTArray<VisitData>::size_type i = 0; i < mPlaces.Length(); i++) {
--- a/toolkit/components/places/nsNavBookmarks.cpp
+++ b/toolkit/components/places/nsNavBookmarks.cpp
@@ -376,16 +376,19 @@ nsNavBookmarks::InsertBookmark(int64_t a
   NS_ENSURE_ARG(aURI);
   NS_ENSURE_ARG_POINTER(aNewBookmarkId);
   NS_ENSURE_ARG_MIN(aIndex, nsINavBookmarksService::DEFAULT_INDEX);
 
   if (!aGUID.IsEmpty() && !IsValidGUID(aGUID)) return NS_ERROR_INVALID_ARG;
 
   mozStorageTransaction transaction(mDB->MainConn(), false);
 
+  // XXX Handle the error, bug 1696133.
+  Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
   nsNavHistory* history = nsNavHistory::GetHistoryService();
   NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY);
   int64_t placeId;
   nsAutoCString placeGuid;
   nsresult rv = history->GetOrCreateIdForPage(aURI, &placeId, placeGuid);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Get the correct index for insertion.  This also ensures the parent exists.
@@ -480,16 +483,19 @@ nsNavBookmarks::RemoveItem(int64_t aItem
   NS_ENSURE_ARG(!IsRoot(aItemId));
 
   BookmarkData bookmark;
   nsresult rv = FetchItemInfo(aItemId, bookmark);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mozStorageTransaction transaction(mDB->MainConn(), false);
 
+  // XXX Handle the error, bug 1696133.
+  Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
   if (bookmark.type == TYPE_FOLDER) {
     // Remove all of the folder's children.
     rv = RemoveFolderChildren(bookmark.id, aSource);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   nsCOMPtr<mozIStorageStatement> stmt =
       mDB->GetStatement("DELETE FROM moz_bookmarks WHERE id = :item_id");
@@ -607,16 +613,19 @@ nsNavBookmarks::CreateFolder(int64_t aPa
   int64_t grandParentId;
   nsAutoCString folderGuid;
   nsresult rv =
       FetchFolderInfo(aParent, &folderCount, folderGuid, &grandParentId);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mozStorageTransaction transaction(mDB->MainConn(), false);
 
+  // XXX Handle the error, bug 1696133.
+  Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
   if (aIndex == nsINavBookmarksService::DEFAULT_INDEX ||
       aIndex >= folderCount) {
     index = folderCount;
   } else {
     // Create space for the insertion.
     rv = AdjustIndices(aParent, index, INT32_MAX, 1);
     NS_ENSURE_SUCCESS(rv, rv);
   }
@@ -767,16 +776,19 @@ nsresult nsNavBookmarks::RemoveFolderChi
     }
   }
 
   int64_t syncChangeDelta = DetermineSyncChangeDelta(aSource);
 
   // Delete items from the database now.
   mozStorageTransaction transaction(mDB->MainConn(), false);
 
+  // XXX Handle the error, bug 1696133.
+  Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
   nsCOMPtr<mozIStorageStatement> deleteStatement =
       mDB->GetStatement(nsLiteralCString("DELETE FROM moz_bookmarks "
                                          "WHERE parent IN (:parent") +
                         foldersToRemove + ")"_ns);
   NS_ENSURE_STATE(deleteStatement);
   mozStorageStatementScoper deleteStatementScoper(deleteStatement);
 
   rv = deleteStatement->BindInt64ByName("parent"_ns, folder.id);
@@ -1072,16 +1084,19 @@ nsNavBookmarks::SetItemLastModified(int6
   bookmark.lastModified = RoundToMilliseconds(aLastModified);
 
   if (isTagging) {
     // If we're changing a tag, bump the change counter for all tagged
     // bookmarks. We use a separate code path to avoid a transaction for
     // non-tags.
     mozStorageTransaction transaction(mDB->MainConn(), false);
 
+    // XXX Handle the error, bug 1696133.
+    Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
     rv = SetItemDateInternal(LAST_MODIFIED, syncChangeDelta, bookmark.id,
                              bookmark.lastModified);
     NS_ENSURE_SUCCESS(rv, rv);
 
     rv = AddSyncChangesForBookmarksWithURL(bookmark.url, syncChangeDelta);
     NS_ENSURE_SUCCESS(rv, rv);
 
     rv = transaction.Commit();
@@ -1276,16 +1291,19 @@ nsNavBookmarks::SetItemTitle(int64_t aIt
   TruncateTitle(aTitle, title);
 
   if (isChangingTagFolder) {
     // If we're changing the title of a tag folder, bump the change counter
     // for all tagged bookmarks. We use a separate code path to avoid a
     // transaction for non-tags.
     mozStorageTransaction transaction(mDB->MainConn(), false);
 
+    // XXX Handle the error, bug 1696133.
+    Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
     rv = SetItemTitleInternal(bookmark, title, syncChangeDelta);
     NS_ENSURE_SUCCESS(rv, rv);
 
     rv = AddSyncChangesForBookmarksInFolder(bookmark.id, syncChangeDelta);
     NS_ENSURE_SUCCESS(rv, rv);
 
     rv = transaction.Commit();
     NS_ENSURE_SUCCESS(rv, rv);
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -268,16 +268,19 @@ class FixAndDecayFrecencyRunnable final 
       // There are more invalid frecencies to fix. Re-dispatch to the async
       // storage thread for the next chunk.
       return NS_DispatchToCurrentThread(this);
     }
 
     mozStorageTransaction transaction(
         mDB->MainConn(), false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    // XXX Handle the error, bug 1696133.
+    Unused << NS_WARN_IF(NS_FAILED(transaction.Start()));
+
     if (NS_WARN_IF(NS_FAILED(DecayFrecencies()))) {
       mDecayReason = mozIStorageStatementCallback::REASON_ERROR;
     }
 
     // We've finished fixing and decaying frecencies. Trigger frecency updates
     // for all affected origins.
     nsCOMPtr<mozIStorageStatement> updateOriginFrecenciesStmt =
         mDB->GetStatement("DELETE FROM moz_updateoriginsupdate_temp");