Bug 1435446 - Add a default transaction type for storage connections. r=mak
authorKit Cambridge <kit@yakshaving.ninja>
Wed, 28 Feb 2018 22:44:40 -0800
changeset 461240 f8a3b9547dbce734899f16815edcc2b6b26e27d9
parent 461239 d45236740b636d635690fb69a8ec46c249a26b36
child 461241 400d6348abcaff1993820a95a946c99a7abc94c5
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1435446
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1435446 - Add a default transaction type for storage connections. r=mak This patch adds a `mozIStorageConnection::defaultTransactionType` attribute that controls the default transaction behavior for the connection. As before, `mozStorageTransaction` can override the default behavior for individual transactions. MozReview-Commit-ID: IRSlMesETWN
dom/cache/Connection.cpp
storage/mozIStorageAsyncConnection.idl
storage/mozIStorageConnection.idl
storage/mozStorageConnection.cpp
storage/mozStorageConnection.h
storage/mozStorageHelper.h
storage/test/unit/test_storage_connection.js
toolkit/modules/Sqlite.jsm
toolkit/modules/tests/xpcshell/test_sqlite.js
--- a/dom/cache/Connection.cpp
+++ b/dom/cache/Connection.cpp
@@ -241,28 +241,34 @@ Connection::IndexExists(const nsACString
 
 NS_IMETHODIMP
 Connection::GetTransactionInProgress(bool* aResultOut)
 {
   return mBase->GetTransactionInProgress(aResultOut);
 }
 
 NS_IMETHODIMP
+Connection::GetDefaultTransactionType(int32_t* aResultOut)
+{
+  return mBase->GetDefaultTransactionType(aResultOut);
+}
+
+NS_IMETHODIMP
+Connection::SetDefaultTransactionType(int32_t aType)
+{
+  return mBase->SetDefaultTransactionType(aType);
+}
+
+NS_IMETHODIMP
 Connection::BeginTransaction()
 {
   return mBase->BeginTransaction();
 }
 
 NS_IMETHODIMP
-Connection::BeginTransactionAs(int32_t aType)
-{
-  return mBase->BeginTransactionAs(aType);
-}
-
-NS_IMETHODIMP
 Connection::CommitTransaction()
 {
   return mBase->CommitTransaction();
 }
 
 NS_IMETHODIMP
 Connection::RollbackTransaction()
 {
--- a/storage/mozIStorageAsyncConnection.idl
+++ b/storage/mozIStorageAsyncConnection.idl
@@ -21,16 +21,31 @@ interface nsIFile;
  * connection attached to a specific file or to an in-memory data
  * storage.  It is the primary interface for interacting with a
  * database from the main thread, including creating prepared
  * statements, executing SQL, and examining database errors.
  */
 [scriptable, uuid(8bfd34d5-4ddf-4e4b-89dd-9b14f33534c6)]
 interface mozIStorageAsyncConnection : nsISupports {
   /**
+   * Transaction behavior constants.
+   */
+  const int32_t TRANSACTION_DEFAULT = -1;
+  const int32_t TRANSACTION_DEFERRED = 0;
+  const int32_t TRANSACTION_IMMEDIATE = 1;
+  const int32_t TRANSACTION_EXCLUSIVE = 2;
+
+  /**
+   * The default behavior for all transactions run on this connection. Defaults
+   * to `TRANSACTION_DEFERRED`, and can be overridden for individual
+   * transactions.
+   */
+  attribute int32_t defaultTransactionType;
+
+  /**
    * Close this database connection, allowing all pending statements
    * to complete first.
    *
    * @param aCallback [optional]
    *        A callback that will be notified when the close is completed,
    *        with the following arguments:
    *        - status: the status of the call
    *        - value: |null|
--- a/storage/mozIStorageConnection.idl
+++ b/storage/mozIStorageConnection.idl
@@ -175,30 +175,21 @@ interface mozIStorageConnection : mozISt
   //// Transactions
 
   /**
    * Returns true if a transaction is active on this connection.
    */
   readonly attribute boolean transactionInProgress;
 
   /**
-   * Begin a new transaction.  sqlite default transactions are deferred.
-   * If a transaction is active, throws an error.
+   * Begin a new transaction. If a transaction is active, throws an error.
    */
   void beginTransaction();
 
   /**
-   * Begins a new transaction with the given type.
-   */
-  const int32_t TRANSACTION_DEFERRED = 0;
-  const int32_t TRANSACTION_IMMEDIATE = 1;
-  const int32_t TRANSACTION_EXCLUSIVE = 2;
-  void beginTransactionAs(in int32_t transactionType);
-
-  /**
    * Commits the current transaction.  If no transaction is active,
    * @throws NS_ERROR_UNEXPECTED.
    * @throws NS_ERROR_NOT_INITIALIZED.
    */
   void commitTransaction();
 
   /**
    * Rolls back the current transaction.  If no transaction is active,
--- a/storage/mozStorageConnection.cpp
+++ b/storage/mozStorageConnection.cpp
@@ -529,16 +529,17 @@ Connection::Connection(Service *aService
                        bool aAsyncOnly,
                        bool aIgnoreLockingMode)
 : sharedAsyncExecutionMutex("Connection::sharedAsyncExecutionMutex")
 , sharedDBMutex("Connection::sharedDBMutex")
 , threadOpenedOn(do_GetCurrentThread())
 , mDBConn(nullptr)
 , mAsyncExecutionThreadShuttingDown(false)
 , mConnectionClosed(false)
+, mDefaultTransactionType(mozIStorageConnection::TRANSACTION_DEFERRED)
 , mTransactionInProgress(false)
 , mDestroying(false)
 , mProgressHandler(nullptr)
 , mFlags(aFlags)
 , mIgnoreLockingMode(aIgnoreLockingMode)
 , mStorageService(aService)
 , mAsyncOnly(aAsyncOnly)
 {
@@ -1527,16 +1528,19 @@ Connection::initializeClone(Connection* 
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   auto guard = MakeScopeExit([&]() {
     aClone->initializeFailed();
   });
 
+  rv = aClone->SetDefaultTransactionType(mDefaultTransactionType);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   // Re-attach on-disk databases that were attached to the original connection.
   {
     nsCOMPtr<mozIStorageStatement> stmt;
     rv = CreateStatement(NS_LITERAL_CSTRING("PRAGMA database_list"),
                          getter_AddRefs(stmt));
     MOZ_ASSERT(NS_SUCCEEDED(rv));
     bool hasResult = false;
     while (stmt && NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) {
@@ -1928,38 +1932,47 @@ Connection::GetTransactionInProgress(boo
   if (!mDBConn) return NS_ERROR_NOT_INITIALIZED;
 
   SQLiteMutexAutoLock lockedScope(sharedDBMutex);
   *_inProgress = mTransactionInProgress;
   return NS_OK;
 }
 
 NS_IMETHODIMP
-Connection::BeginTransaction()
+Connection::GetDefaultTransactionType(int32_t *_type)
 {
-  return BeginTransactionAs(mozIStorageConnection::TRANSACTION_DEFERRED);
+  *_type = mDefaultTransactionType;
+  return NS_OK;
 }
 
 NS_IMETHODIMP
-Connection::BeginTransactionAs(int32_t aTransactionType)
+Connection::SetDefaultTransactionType(int32_t aType)
+{
+  NS_ENSURE_ARG_RANGE(aType, TRANSACTION_DEFERRED, TRANSACTION_EXCLUSIVE);
+  mDefaultTransactionType = aType;
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+Connection::BeginTransaction()
 {
   if (!mDBConn) return NS_ERROR_NOT_INITIALIZED;
 
-  return beginTransactionInternal(mDBConn, aTransactionType);
+  return beginTransactionInternal(mDBConn, mDefaultTransactionType);
 }
 
 nsresult
 Connection::beginTransactionInternal(sqlite3 *aNativeConnection,
                                      int32_t aTransactionType)
 {
   SQLiteMutexAutoLock lockedScope(sharedDBMutex);
   if (mTransactionInProgress)
     return NS_ERROR_FAILURE;
   nsresult rv;
-  switch(aTransactionType) {
+  switch (aTransactionType) {
     case TRANSACTION_DEFERRED:
       rv = convertResultCode(executeSql(aNativeConnection, "BEGIN DEFERRED"));
       break;
     case TRANSACTION_IMMEDIATE:
       rv = convertResultCode(executeSql(aNativeConnection, "BEGIN IMMEDIATE"));
       break;
     case TRANSACTION_EXCLUSIVE:
       rv = convertResultCode(executeSql(aNativeConnection, "BEGIN EXCLUSIVE"));
--- a/storage/mozStorageConnection.h
+++ b/storage/mozStorageConnection.h
@@ -371,16 +371,21 @@ private:
    * connection.
    *
    * This variable should be accessed while holding the
    * sharedAsyncExecutionMutex.
    */
   bool mConnectionClosed;
 
   /**
+   * Stores the default behavior for all transactions run on this connection.
+   */
+  mozilla::Atomic<int32_t> mDefaultTransactionType;
+
+  /**
    * Tracks if we have a transaction in progress or not.  Access protected by
    * sharedDBMutex.
    */
   bool mTransactionInProgress;
 
   /**
    * Used to trigger cleanup logic only the first time our refcount hits 1.  We
    * may trigger a failsafe Close() that invokes SpinningSynchronousClose()
--- a/storage/mozStorageHelper.h
+++ b/storage/mozStorageHelper.h
@@ -34,18 +34,18 @@
  * to rollback.
  *
  * @param aConnection
  *        The connection to create the transaction on.
  * @param aCommitOnComplete
  *        Controls whether the transaction is committed or rolled back when
  *        this object goes out of scope.
  * @param aType [optional]
- *        The transaction type, as defined in mozIStorageConnection.  Defaults
- *        to TRANSACTION_DEFERRED.
+ *        The transaction type, as defined in mozIStorageConnection. Uses the
+ *        default transaction behavior for the connection if unspecified.
  * @param aAsyncCommit [optional]
  *        Whether commit should be executed asynchronously on the helper thread.
  *        This is a special option introduced as an interim solution to reduce
  *        main-thread fsyncs in Places.  Can only be used on main-thread.
  *
  *        WARNING: YOU SHOULD _NOT_ WRITE NEW MAIN-THREAD CODE USING THIS!
  *
  *        Notice that async commit might cause synchronous statements to fail
@@ -58,27 +58,31 @@
  *        For all of the above reasons, this should only be used as an interim
  *        solution and avoided completely if possible.
  */
 class mozStorageTransaction
 {
 public:
   mozStorageTransaction(mozIStorageConnection* aConnection,
                         bool aCommitOnComplete,
-                        int32_t aType = mozIStorageConnection::TRANSACTION_DEFERRED,
+                        int32_t aType = mozIStorageConnection::TRANSACTION_DEFAULT,
                         bool aAsyncCommit = false)
     : mConnection(aConnection),
       mHasTransaction(false),
       mCommitOnComplete(aCommitOnComplete),
       mCompleted(false),
       mAsyncCommit(aAsyncCommit)
   {
     if (mConnection) {
       nsAutoCString query("BEGIN");
-      switch(aType) {
+      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");
--- a/storage/test/unit/test_storage_connection.js
+++ b/storage/test/unit/test_storage_connection.js
@@ -909,16 +909,72 @@ add_task(async function test_sync_clone_
 
   Assert.equal(func.name, "kit");
 
   info("Clean up");
   db.close();
   clone.close();
 });
 
+add_task(async function test_defaultTransactionType() {
+  info("Open connection");
+  let db = Services.storage.openDatabase(getTestDB());
+  Assert.ok(db instanceof Ci.mozIStorageAsyncConnection);
+
+  info("Verify default transaction type");
+  Assert.equal(db.defaultTransactionType,
+    Ci.mozIStorageConnection.TRANSACTION_DEFERRED);
+
+  info("Test other transaction types");
+  for (let type of [Ci.mozIStorageConnection.TRANSACTION_IMMEDIATE,
+                    Ci.mozIStorageConnection.TRANSACTION_EXCLUSIVE]) {
+    db.defaultTransactionType = type;
+    Assert.equal(db.defaultTransactionType, type);
+  }
+
+  info("Should reject unknown transaction types");
+  Assert.throws(() => db.defaultTransactionType =
+                        Ci.mozIStorageConnection.TRANSACTION_DEFAULT,
+                /NS_ERROR_ILLEGAL_VALUE/);
+
+  db.defaultTransactionType =
+    Ci.mozIStorageConnection.TRANSACTION_IMMEDIATE;
+
+  info("Clone should inherit default transaction type");
+  let clone = await asyncClone(db, true);
+  Assert.ok(clone instanceof Ci.mozIStorageAsyncConnection);
+  Assert.equal(clone.defaultTransactionType,
+    Ci.mozIStorageConnection.TRANSACTION_IMMEDIATE);
+
+  info("Begin immediate transaction on main connection");
+  db.beginTransaction();
+
+  info("Queue immediate transaction on clone");
+  let stmts = [
+    clone.createAsyncStatement(`BEGIN IMMEDIATE TRANSACTION`),
+    clone.createAsyncStatement(`DELETE FROM test WHERE name = 'new'`),
+    clone.createAsyncStatement(`COMMIT`),
+  ];
+  let promiseStmtsRan = stmts.map(stmt => executeAsync(stmt));
+
+  info("Commit immediate transaction on main connection");
+  db.executeSimpleSQL(`INSERT INTO test(name) VALUES('new')`);
+  db.commitTransaction();
+
+  info("Wait for transaction to succeed on clone");
+  await Promise.all(promiseStmtsRan);
+
+  info("Clean up");
+  for (let stmt of stmts) {
+    stmt.finalize();
+  }
+  await asyncClose(clone);
+  await asyncClose(db);
+});
+
 add_task(async function test_getInterface() {
   let db = getOpenedDatabase();
   let target = db.QueryInterface(Ci.nsIInterfaceRequestor)
                  .getInterface(Ci.nsIEventTarget);
   // Just check that target is non-null.  Other tests will ensure that it has
   // the correct value.
   Assert.ok(target != null);
 
--- a/toolkit/modules/Sqlite.jsm
+++ b/toolkit/modules/Sqlite.jsm
@@ -220,16 +220,22 @@ function ConnectionData(connection, iden
   this._pendingStatements = new Map();
 
   // Increments for each executed statement for the life of the connection.
   this._statementCounter = 0;
 
   // Increments whenever we request a unique operation id.
   this._operationsCounter = 0;
 
+  if ("defaultTransactionType" in options) {
+    this.defaultTransactionType = options.defaultTransactionType;
+  } else {
+    this.defaultTransactionType = convertStorageTransactionType(
+      this._dbConn.defaultTransactionType);
+  }
   this._hasInProgressTransaction = false;
   // Manages a chain of transactions promises, so that new transactions
   // always happen in queue to the previous ones.  It never rejects.
   this._transactionQueue = Promise.resolve();
 
   this._idleShrinkMS = options.shrinkMemoryOnConnectionIdleMS;
   if (this._idleShrinkMS) {
     this._idleShrinkTimer = Cc["@mozilla.org/timer;1"]
@@ -534,18 +540,20 @@ ConnectionData.prototype = Object.freeze
     });
   },
 
   get transactionInProgress() {
     return this._open && this._hasInProgressTransaction;
   },
 
   executeTransaction(func, type) {
-    if (typeof type == "undefined") {
-      throw new Error("Internal error: expected a type");
+    if (type == OpenedConnection.prototype.TRANSACTION_DEFAULT) {
+      type = this.defaultTransactionType;
+    } else if (!OpenedConnection.TRANSACTION_TYPES.includes(type)) {
+      throw new Error("Unknown transaction type: " + type);
     }
     this.ensureOpen();
 
     this._log.debug("Beginning transaction");
 
     let promise = this._transactionQueue.then(() => {
       if (this._closeRequested) {
         throw new Error("Transaction canceled due to a closed connection.");
@@ -913,16 +921,26 @@ function openConnection(options) {
       throw new Error("shrinkMemoryOnConnectionIdleMS must be an integer. " +
                       "Got: " + options.shrinkMemoryOnConnectionIdleMS);
     }
 
     openedOptions.shrinkMemoryOnConnectionIdleMS =
       options.shrinkMemoryOnConnectionIdleMS;
   }
 
+  if ("defaultTransactionType" in options) {
+    let defaultTransactionType = options.defaultTransactionType;
+    if (!OpenedConnection.TRANSACTION_TYPES.includes(defaultTransactionType)) {
+      throw new Error("Unknown default transaction type: " +
+                      defaultTransactionType);
+    }
+
+    openedOptions.defaultTransactionType = defaultTransactionType;
+  }
+
   let file = FileUtils.File(path);
   let identifier = getIdentifierByFileName(OS.Path.basename(path));
 
   log.info("Opening database: " + path + " (" + identifier + ")");
 
   return new Promise((resolve, reject) => {
     let dbOptions = Cc["@mozilla.org/hash-property-bag;1"].
                     createInstance(Ci.nsIWritablePropertyBag);
@@ -1151,23 +1169,33 @@ function OpenedConnection(connection, id
   // before its `forget` method has been called, an event with topic
   // "sqlite-finalization-witness" is broadcasted along with the
   // connection identifier string of the database.
   this._witness = FinalizationWitnessService.make(
     "sqlite-finalization-witness",
     this._connectionData._identifier);
 }
 
+OpenedConnection.TRANSACTION_TYPES = ["DEFERRED", "IMMEDIATE", "EXCLUSIVE"];
+
+// Converts a `mozIStorageAsyncConnection::TRANSACTION_*` constant into the
+// corresponding `OpenedConnection.TRANSACTION_TYPES` constant.
+function convertStorageTransactionType(type) {
+  if (!(type in OpenedConnection.TRANSACTION_TYPES)) {
+    throw new Error("Unknown storage transaction type: " + type);
+  }
+  return OpenedConnection.TRANSACTION_TYPES[type];
+}
+
 OpenedConnection.prototype = Object.freeze({
+  TRANSACTION_DEFAULT: "DEFAULT",
   TRANSACTION_DEFERRED: "DEFERRED",
   TRANSACTION_IMMEDIATE: "IMMEDIATE",
   TRANSACTION_EXCLUSIVE: "EXCLUSIVE",
 
-  TRANSACTION_TYPES: ["DEFERRED", "IMMEDIATE", "EXCLUSIVE"],
-
   /**
    * The integer schema version of the database.
    *
    * This is 0 if not schema version has been set.
    *
    * @return Promise<int>
    */
   getSchemaVersion(schemaName = "main") {
@@ -1324,16 +1352,23 @@ OpenedConnection.prototype = Object.free
   execute(sql, params = null, onRow = null) {
     if (isInvalidBoundLikeQuery(sql)) {
       throw new Error("Please enter a LIKE clause with bindings");
     }
     return this._connectionData.execute(sql, params, onRow);
   },
 
   /**
+   * The default behavior for transactions run on this connection.
+   */
+  get defaultTransactionType() {
+    return this._connectionData.defaultTransactionType;
+  },
+
+  /**
    * Whether a transaction is currently in progress.
    */
   get transactionInProgress() {
     return this._connectionData.transactionInProgress;
   },
 
   /**
    * Perform a transaction.
@@ -1368,21 +1403,17 @@ OpenedConnection.prototype = Object.free
    * be resolved to whatever value the supplied function resolves to. If
    * the transaction is rolled back, the promise is rejected.
    *
    * @param func
    *        (function) What to perform as part of the transaction.
    * @param type optional
    *        One of the TRANSACTION_* constants attached to this type.
    */
-  executeTransaction(func, type = this.TRANSACTION_DEFERRED) {
-    if (!this.TRANSACTION_TYPES.includes(type)) {
-      throw new Error("Unknown transaction type: " + type);
-    }
-
+  executeTransaction(func, type = this.TRANSACTION_DEFAULT) {
     return this._connectionData.executeTransaction(() => func(this), type);
   },
 
   /**
    * Whether a table exists in the database (both persistent and temporary tables).
    *
    * @param name
    *        (string) Name of the table.
--- a/toolkit/modules/tests/xpcshell/test_sqlite.js
+++ b/toolkit/modules/tests/xpcshell/test_sqlite.js
@@ -79,16 +79,25 @@ async function getDummyTempDatabase(name
 
 add_task(async function test_setup() {
   ChromeUtils.import("resource://testing-common/services/common/logging.js");
   initTestLogging("Trace");
 });
 
 add_task(async function test_open_normal() {
   let c = await Sqlite.openConnection({path: "test_open_normal.sqlite"});
+  Assert.equal(c.defaultTransactionType, "DEFERRED");
+  await c.close();
+});
+
+add_task(async function test_open_with_defaultTransactionType() {
+  let c = await getConnection("execute_transaction_types", {
+    defaultTransactionType: "IMMEDIATE",
+  });
+  Assert.equal(c.defaultTransactionType, "IMMEDIATE");
   await c.close();
 });
 
 add_task(async function test_open_normal_error() {
   let currentDir = await OS.File.getCurrentDirectory();
 
   let src = OS.Path.join(currentDir, "corrupt.sqlite");
   Assert.ok((await OS.File.exists(src)), "Database file found");
@@ -910,20 +919,25 @@ add_task(async function test_cloneStorag
         resolve(db.QueryInterface(Ci.mozIStorageAsyncConnection));
       } else {
         reject(new Error(status));
       }
     });
   });
 
   let clone = await Sqlite.cloneStorageConnection({ connection: c, readOnly: true });
+  Assert.equal(clone.defaultTransactionType, "DEFERRED");
   // Just check that it works.
   await clone.execute("SELECT 1");
 
+  info("Set default transaction type on storage connection");
+  c.defaultTransactionType = Ci.mozIStorageConnection.TRANSACTION_IMMEDIATE;
+
   let clone2 = await Sqlite.cloneStorageConnection({ connection: c, readOnly: false });
+  Assert.equal(clone2.defaultTransactionType, "IMMEDIATE");
   // Just check that it works.
   await clone2.execute("CREATE TABLE test (id INTEGER PRIMARY KEY)");
 
   // Closing order should not matter.
   await c.asyncClose();
   await clone2.close();
   await clone.close();
 });
@@ -979,23 +993,35 @@ add_task(async function test_wrapStorage
         resolve(db.QueryInterface(Ci.mozIStorageAsyncConnection));
       } else {
         reject(new Error(status));
       }
     });
   });
 
   let wrapper = await Sqlite.wrapStorageConnection({ connection: c });
+  Assert.equal(wrapper.defaultTransactionType, "DEFERRED");
   // Just check that it works.
   await wrapper.execute("SELECT 1");
   await wrapper.executeCached("SELECT 1");
 
   // Closing the wrapper should just finalize statements but not close the
   // database.
   await wrapper.close();
+
+  info("Set default transaction type on storage connection");
+  c.defaultTransactionType = Ci.mozIStorageConnection.TRANSACTION_EXCLUSIVE;
+
+  let wrapper2 = await Sqlite.wrapStorageConnection({ connection: c });
+  Assert.equal(wrapper2.defaultTransactionType, "EXCLUSIVE");
+  // Just check that it works.
+  await wrapper2.execute("SELECT 1");
+
+  await wrapper2.close();
+
   await c.asyncClose();
 });
 
 // Test finalization
 add_task(async function test_closed_by_witness() {
   failTestsOnAutoClose(false);
   let c = await getDummyDatabase("closed_by_witness");