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

@@ -457,6 +457,8 @@ nsresult CreateOrMigrateSchema(mozIStorageConnection& aConn) {
   mozStorageTransaction trans(&aConn, false,
                               mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+  CACHE_TRY(trans.Start());
+
   const bool migrating = schemaVersion != 0;
 
   if (migrating) {

dom/cache/Manager.cpp

@@ -98,6 +98,8 @@ class SetupAction final : public SyncDBAction {
       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));
@@ -540,6 +542,8 @@ class Manager::DeleteOrphanedCacheAction final : public SyncDBAction {
     mozStorageTransaction trans(aConn, false,
                                 mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    CACHE_TRY(trans.Start());
+
     CACHE_TRY_UNWRAP(mDeletionInfo, db::DeleteCacheId(*aConn, mCacheId));
 
     CACHE_TRY(MaybeUpdatePaddingFile(
@@ -847,6 +851,8 @@ class Manager::CachePutAllAction final : public DBAction {
     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) {
@@ -1105,6 +1111,8 @@ class Manager::CacheDeleteAction final : public Manager::BaseAction {
     mozStorageTransaction trans(aConn, false,
                                 mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    CACHE_TRY(trans.Start());
+
     CACHE_TRY_UNWRAP(
         auto maybeDeletionInfo,
         db::CacheDelete(*aConn, mCacheId, mArgs.request(), mArgs.params()));
@@ -1326,6 +1334,8 @@ class Manager::StorageOpenAction final : public Manager::BaseAction {
     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()));
@@ -1377,6 +1387,8 @@ class Manager::StorageDeleteAction final : public Manager::BaseAction {
     mozStorageTransaction trans(aConn, false,
                                 mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    CACHE_TRY(trans.Start());
+
     CACHE_TRY_INSPECT(const auto& maybeCacheId,
                       db::StorageGetCacheId(*aConn, mNamespace, mArgs.key()));
 

dom/indexedDB/ActorsParent.cpp

@@ -928,6 +928,8 @@ CreateStorageConnection(nsIFile& aDBFile, nsIFile& aFMDirectory,
     mozStorageTransaction transaction(
         connection.get(), false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    IDB_TRY(transaction.Start());
+
     if (newDatabase) {
       IDB_TRY(CreateTables(*connection));
 
@@ -12344,6 +12346,8 @@ nsresult FileManager::InitDirectory(nsIFile& aDirectory, nsIFile& aDatabaseFile,
 
       mozStorageTransaction transaction(connection.get(), false);
 
+      IDB_TRY(transaction.Start())
+
       IDB_TRY(connection->ExecuteSimpleSQL(
           "CREATE VIRTUAL TABLE fs USING filesystem;"_ns));
 
@@ -14093,6 +14097,8 @@ nsresult DatabaseMaintenance::DetermineMaintenanceAction(
   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(

dom/localstorage/ActorsParent.cpp

@@ -530,6 +530,8 @@ Result<nsCOMPtr<mozIStorageConnection>, nsresult> CreateStorageConnection(
     mozStorageTransaction transaction(
         connection, false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    LS_TRY(transaction.Start())
+
     if (newDatabase) {
       LS_TRY(CreateTables(connection));
 
@@ -6893,6 +6895,8 @@ nsresult PrepareDatastoreOp::DatabaseWork() {
     mozStorageTransaction transaction(
         connection, false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    LS_TRY(transaction.Start())
+
     {
       nsCOMPtr<mozIStorageFunction> function = new CompressFunction();
 

dom/quota/ActorsParent.cpp

@@ -466,6 +466,8 @@ Result<bool, nsresult> MaybeCreateOrUpgradeCache(
     mozStorageTransaction transaction(
         &aConnection, false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+    QM_TRY(transaction.Start());
+
     if (newCache) {
       QM_TRY(CreateCacheTables(&aConnection));
 
@@ -541,6 +543,7 @@ nsresult InvalidateCache(mozIStorageConnection& aConnection) {
     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());
@@ -4215,6 +4218,8 @@ void QuotaManager::UnloadQuota() {
   mozStorageTransaction transaction(
       mStorageConnection, false, mozIStorageConnection::TRANSACTION_IMMEDIATE);
 
+  QM_TRY(transaction.Start(), QM_VOID);
+
   QM_TRY(mStorageConnection->ExecuteSimpleSQL("DELETE FROM origin;"_ns),
          QM_VOID);
 
@@ -5706,6 +5711,8 @@ nsresult QuotaManager::MaybeCreateOrUpgradeStorage(
     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).

dom/storage/StorageDBThread.cpp

@@ -1414,7 +1414,10 @@ nsresult StorageDBThread::PendingOperations::Execute(StorageDBThread* aThread) {
 
   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];

dom/storage/StorageDBUpdater.cpp

@@ -281,6 +281,9 @@ 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;
@@ -299,7 +302,7 @@ nsresult CreateCurrentSchema(mozIStorageConnection* aConnection) {
   }
 #endif
 
-  nsresult rv = CreateCurrentSchemaOnEmptyTableInternal(aConnection);
+  rv = CreateCurrentSchemaOnEmptyTableInternal(aConnection);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = transaction.Commit();
@@ -309,10 +312,11 @@ nsresult CreateCurrentSchema(mozIStorageConnection* aConnection) {
 }
 
 nsresult Update(mozIStorageConnection* aWorkerConnection) {
-  nsresult rv;
-
   mozStorageTransaction transaction(aWorkerConnection, false);
 
+  nsresult rv = transaction.Start();
+  NS_ENSURE_SUCCESS(rv, rv);
+
   bool doVacuum = false;
 
   int32_t schemaVer;

netwerk/cache/nsDiskCacheDeviceSQL.cpp

@@ -760,6 +760,9 @@ nsApplicationCache::AddNamespaces(nsIArray* namespaces) {
 
   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);

netwerk/cookie/CookiePersistentStorage.cpp

@@ -454,6 +454,9 @@ 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();
@@ -465,6 +468,9 @@ void CookiePersistentStorage::RemoveCookiesFromExactHost(
     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();
@@ -832,6 +838,9 @@ CookiePersistentStorage::OpenDBResult CookiePersistentStorage::TryInitDB(
     // 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
@@ -1979,6 +1988,9 @@ nsresult CookiePersistentStorage::RunInTransaction(
 
   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;

storage/mozStorageHelper.h

@@ -9,6 +9,7 @@
 #include "nsCOMPtr.h"
 #include "nsString.h"
 #include "mozilla/DebugOnly.h"
+#include "mozilla/ScopeExit.h"
 
 #include "mozilla/storage/SQLiteMutex.h"
 #include "mozIStorageConnection.h"
@@ -65,51 +66,12 @@ class mozStorageTransaction {
       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) {
@@ -125,12 +87,77 @@ class mozStorageTransaction {
     }
   }
 
+  /**
+   * 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());
@@ -162,11 +189,11 @@ class mozStorageTransaction {
                                          IntToCString(mNestingLevel));
     }
 
-    if (NS_SUCCEEDED(rv)) {
-      TransactionFinished(lock);
-    }
+    NS_ENSURE_SUCCESS(rv, rv);
 
-    return rv;
+    TransactionFinished(lock);
+
+    return NS_OK;
   }
 
   /**
@@ -175,6 +202,8 @@ class mozStorageTransaction {
    * 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());
@@ -205,11 +234,11 @@ class mozStorageTransaction {
           nestingLevelCString);
     }
 
-    if (NS_SUCCEEDED(rv)) {
-      TransactionFinished(lock);
-    }
+    NS_ENSURE_SUCCESS(rv, rv);
 
-    return rv;
+    TransactionFinished(lock);
+
+    return NS_OK;
   }
 
  protected:
@@ -248,6 +277,7 @@ class mozStorageTransaction {
   }
 
   nsCOMPtr<mozIStorageConnection> mConnection;
+  int32_t mType;
   uint32_t mNestingLevel;
   bool mHasTransaction;
   bool mCommitOnComplete;

storage/test/gtest/test_transaction_helper.cpp

@@ -28,6 +28,7 @@ TEST(storage_transaction_helper, Commit)
   // 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();
@@ -47,6 +48,7 @@ TEST(storage_transaction_helper, Rollback)
   // 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();
@@ -66,6 +68,7 @@ TEST(storage_transaction_helper, AutoCommit)
   // 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);
   }
@@ -85,6 +88,7 @@ TEST(storage_transaction_helper, AutoRollback)
   // 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);
   }
@@ -100,6 +104,7 @@ 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()));
 }
@@ -118,6 +123,7 @@ TEST(storage_transaction_helper, async_Commit)
   {
     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();
@@ -146,12 +152,14 @@ TEST(storage_transaction_helper, Nesting)
 
   {
     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));

toolkit/components/places/Database.cpp

@@ -700,6 +700,8 @@ nsresult Database::EnsureFaviconsDatabaseAttached(
     // 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);
@@ -892,6 +894,9 @@ nsresult Database::TryToCloneTablesFromCorruptDatabase(
 
   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,
@@ -1083,6 +1088,9 @@ nsresult Database::InitSchema(bool* aDatabaseMigrated) {
   // 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:
     //

toolkit/components/places/FaviconHelpers.cpp

@@ -822,8 +822,13 @@ AsyncAssociateIconToPage::Run() {
 
   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);
@@ -1054,6 +1059,10 @@ AsyncReplaceFaviconData::Run() {
 
   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.

toolkit/components/places/History.cpp

@@ -819,6 +819,9 @@ class InsertVisitedURIs final : public Runnable {
     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;

toolkit/components/places/nsNavBookmarks.cpp

@@ -381,6 +381,9 @@ nsNavBookmarks::InsertBookmark(int64_t aFolder, nsIURI* aURI, int32_t aIndex,
 
   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;
@@ -485,6 +488,9 @@ nsNavBookmarks::RemoveItem(int64_t aItemId, uint16_t aSource) {
 
   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);
@@ -612,6 +618,9 @@ nsNavBookmarks::CreateFolder(int64_t aParent, const nsACString& aTitle,
 
   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;
@@ -772,6 +781,9 @@ nsresult nsNavBookmarks::RemoveFolderChildren(int64_t aFolderId,
   // 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") +
@@ -1077,6 +1089,9 @@ nsNavBookmarks::SetItemLastModified(int64_t aItemId, PRTime aLastModified,
     // 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);
@@ -1281,6 +1296,9 @@ nsNavBookmarks::SetItemTitle(int64_t aItemId, const nsACString& aTitle,
     // 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);
 

toolkit/components/places/nsNavHistory.cpp

@@ -273,6 +273,9 @@ class FixAndDecayFrecencyRunnable final : public Runnable {
     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;
     }