Bug 1891141 - Add configuration parameters for pages per step and step delay to mozIStorageAsyncConnection.backupToFileAsync. r=mak

This also fixes a bug in the test_connection_online_backup.js test which wasn't
properly evaluating that the page_size PRAGMA was being copied properly.

Differential Revision: https://phabricator.services.mozilla.com/D207471

dom/cache/Connection.cpp

@@ -288,7 +288,8 @@ uint32_t Connection::DecreaseTransactionNestingLevel(
 
 NS_IMETHODIMP
 Connection::BackupToFileAsync(nsIFile* aDestinationFile,
-                              mozIStorageCompletionCallback* aCallback) {
+                              mozIStorageCompletionCallback* aCallback,
+                              uint32_t aPagesPerStep, uint32_t aStepDelayMs) {
   // async methods are not supported
   return NS_ERROR_NOT_IMPLEMENTED;
 }

storage/mozIStorageAsyncConnection.idl

@@ -395,6 +395,12 @@ interface mozIStorageAsyncConnection : nsISupports {
    *        - status: the status of the operation, use this to check if making
    *                  the copy was successful.
    *        - value: unused.
+   * @param [aPagesPerStep=5]
+   *        The number of pages in the database to copy per step. Defaults to 5
+   *        if omitted or set to 0.
+   * @param [aStepDelayMs=250]
+   *        The number of milliseconds to wait between each step. Defaults to
+   *        250 if omitted or set to 0.
    * @throws NS_ERROR_ABORT
    *         If the application has begun the process of shutting down already.
    * @throws NS_ERROR_NOT_INITIALIZED
@@ -405,5 +411,7 @@ interface mozIStorageAsyncConnection : nsISupports {
    *         If the execution thread cannot be acquired.
    */
   void backupToFileAsync(in nsIFile aDestinationFile,
-                         in mozIStorageCompletionCallback aCallback);
+                         in mozIStorageCompletionCallback aCallback,
+                         [optional] in unsigned long aPagesPerStep,
+                         [optional] in unsigned long aStepDelayMs);
 };

storage/mozStorageConnection.cpp

@@ -606,12 +606,15 @@ class AsyncBackupDatabaseFile final : public Runnable, public nsITimerCallback {
    */
   AsyncBackupDatabaseFile(Connection* aConnection, sqlite3* aNativeConnection,
                           nsIFile* aDestinationFile,
-                          mozIStorageCompletionCallback* aCallback)
+                          mozIStorageCompletionCallback* aCallback,
+                          int32_t aPagesPerStep, uint32_t aStepDelayMs)
       : Runnable("storage::AsyncBackupDatabaseFile"),
         mConnection(aConnection),
         mNativeConnection(aNativeConnection),
         mDestinationFile(aDestinationFile),
         mCallback(aCallback),
+        mPagesPerStep(aPagesPerStep),
+        mStepDelayMs(aStepDelayMs),
         mBackupFile(nullptr),
         mBackupHandle(nullptr) {
     MOZ_ASSERT(NS_IsMainThread());
@@ -681,19 +684,14 @@ class AsyncBackupDatabaseFile final : public Runnable, public nsITimerCallback {
     rv = file->InitWithPath(tempPath);
     DISPATCH_AND_RETURN_IF_FAILED(rv);
 
-    // The number of milliseconds to wait between each batch of copies.
-    static constexpr uint32_t STEP_DELAY_MS = 250;
-    // The number of pages to copy per step
-    static constexpr int COPY_PAGES = 5;
-
-    int srv = ::sqlite3_backup_step(mBackupHandle, COPY_PAGES);
+    int srv = ::sqlite3_backup_step(mBackupHandle, mPagesPerStep);
     if (srv == SQLITE_OK || srv == SQLITE_BUSY || srv == SQLITE_LOCKED) {
       // We're continuing the backup later. Release the guard to avoid closing
       // the database.
       guard.release();
       // Queue up the next step
-      return NS_NewTimerWithCallback(getter_AddRefs(mTimer), this,
-                                     STEP_DELAY_MS, nsITimer::TYPE_ONE_SHOT,
+      return NS_NewTimerWithCallback(getter_AddRefs(mTimer), this, mStepDelayMs,
+                                     nsITimer::TYPE_ONE_SHOT,
                                      GetCurrentSerialEventTarget());
     }
 #ifdef DEBUG
@@ -771,6 +769,8 @@ class AsyncBackupDatabaseFile final : public Runnable, public nsITimerCallback {
   nsCOMPtr<nsITimer> mTimer;
   nsCOMPtr<nsIFile> mDestinationFile;
   nsCOMPtr<mozIStorageCompletionCallback> mCallback;
+  int32_t mPagesPerStep;
+  uint32_t mStepDelayMs;
   sqlite3* mBackupFile;
   sqlite3_backup* mBackupHandle;
 };
@@ -2962,7 +2962,8 @@ uint32_t Connection::DecreaseTransactionNestingLevel(
 
 NS_IMETHODIMP
 Connection::BackupToFileAsync(nsIFile* aDestinationFile,
-                              mozIStorageCompletionCallback* aCallback) {
+                              mozIStorageCompletionCallback* aCallback,
+                              uint32_t aPagesPerStep, uint32_t aStepDelayMs) {
   NS_ENSURE_ARG(aDestinationFile);
   NS_ENSURE_ARG(aCallback);
   NS_ENSURE_TRUE(NS_IsMainThread(), NS_ERROR_NOT_SAME_THREAD);
@@ -2984,9 +2985,28 @@ Connection::BackupToFileAsync(nsIFile* aDestinationFile,
     return NS_ERROR_NOT_INITIALIZED;
   }
 
+  // The number of pages of the database to copy per step
+  static constexpr int32_t DEFAULT_PAGES_PER_STEP = 5;
+  // The number of milliseconds to wait between each step.
+  static constexpr uint32_t DEFAULT_STEP_DELAY_MS = 250;
+
+  CheckedInt<int32_t> pagesPerStep(aPagesPerStep);
+  if (!pagesPerStep.isValid()) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
+  if (!pagesPerStep.value()) {
+    pagesPerStep = DEFAULT_PAGES_PER_STEP;
+  }
+
+  if (!aStepDelayMs) {
+    aStepDelayMs = DEFAULT_STEP_DELAY_MS;
+  }
+
   // Create and dispatch our backup event to the execution thread.
   nsCOMPtr<nsIRunnable> backupEvent =
-      new AsyncBackupDatabaseFile(this, mDBConn, aDestinationFile, aCallback);
+      new AsyncBackupDatabaseFile(this, mDBConn, aDestinationFile, aCallback,
+                                  pagesPerStep.value(), aStepDelayMs);
   rv = asyncThread->Dispatch(backupEvent, NS_DISPATCH_NORMAL);
   return rv;
 }

storage/test/unit/test_connection_online_backup.js

@@ -85,9 +85,15 @@ async function getPreparedAsyncDatabase() {
  *
  * @param {mozIStorageAsyncConnection} connection
  *   A connection to a database that should be copied.
+ * @param {number} [pagesPerStep]
+ *   The number of pages to copy per step. If not supplied or is 0, falls back
+ *   to the platform default which is currently 5.
+ * @param {number} [stepDelayMs]
+ *   The number of milliseconds to wait between copying step. If not supplied
+ *   or is 0, falls back to the platform default which is currently 250.
  * @returns {Promise<nsIFile>}
  */
-async function createCopy(connection) {
+async function createCopy(connection, pagesPerStep, stepDelayMs) {
   let destFilePath = PathUtils.join(PathUtils.profileDir, BACKUP_FILE_NAME);
   let destFile = await IOUtils.getFile(destFilePath);
   Assert.ok(
@@ -96,10 +102,15 @@ async function createCopy(connection) {
   );
 
   await new Promise(resolve => {
-    connection.backupToFileAsync(destFile, result => {
-      Assert.ok(Components.isSuccessCode(result));
-      resolve(result);
-    });
+    connection.backupToFileAsync(
+      destFile,
+      result => {
+        Assert.ok(Components.isSuccessCode(result));
+        resolve(result);
+      },
+      pagesPerStep,
+      stepDelayMs
+    );
   });
 
   return destFile;
@@ -121,7 +132,7 @@ async function assertSuccessfulCopy(file, expectedEntries = TEST_ROWS) {
 
   await executeSimpleSQLAsync(conn, "PRAGMA page_size", resultSet => {
     let result = resultSet.getNextRow();
-    Assert.equal(TEST_PAGE_SIZE, result.getResultByIndex(0).getAsUint32());
+    Assert.equal(TEST_PAGE_SIZE, result.getResultByIndex(0));
   });
 
   let stmt = conn.createAsyncStatement("SELECT COUNT(*) FROM test");
@@ -190,6 +201,36 @@ add_task(async function test_backupToFileAsync_during_insert() {
   await asyncClose(newConnection);
 });
 
+/**
+ * Tests that alternative pages-per-step and step delay values can be set when
+ * calling backupToFileAsync.
+ */
+add_task(async function test_backupToFileAsync_during_insert() {
+  let newConnection = await getPreparedAsyncDatabase();
+
+  // Let's try some higher values...
+  let copyFile = await createCopy(newConnection, 15, 500);
+  Assert.ok(
+    await IOUtils.exists(copyFile.path),
+    "A new file was created by backupToFileAsync"
+  );
+
+  await assertSuccessfulCopy(copyFile);
+  await IOUtils.remove(copyFile.path);
+
+  // And now we'll try some lower values...
+  copyFile = await createCopy(newConnection, 1, 25);
+  Assert.ok(
+    await IOUtils.exists(copyFile.path),
+    "A new file was created by backupToFileAsync"
+  );
+
+  await assertSuccessfulCopy(copyFile);
+  await IOUtils.remove(copyFile.path);
+
+  await asyncClose(newConnection);
+});
+
 /**
  * Tests the behaviour of backupToFileAsync as exposed through Sqlite.sys.mjs.
  */
@@ -206,6 +247,13 @@ add_task(async function test_backupToFileAsync_via_Sqlite_module() {
 
   await assertSuccessfulCopy(copyFile);
   await IOUtils.remove(copyFile.path);
+
+  // Also check that we can plumb through pagesPerStep and stepDelayMs.
+  await moduleConnection.backup(copyFilePath, 15, 500);
+  Assert.ok(await IOUtils.exists(copyFilePath), "A new file was created");
+  await assertSuccessfulCopy(copyFile);
+  await IOUtils.remove(copyFile.path);
+
   await moduleConnection.close();
   await asyncClose(xpcomConnection);
 });

toolkit/modules/Sqlite.sys.mjs

@@ -1184,9 +1184,15 @@ ConnectionData.prototype = Object.freeze({
    * @param {string} destFilePath
    *   The path on the local filesystem to write the database copy. Any existing
    *   file at this path will be overwritten.
+   * @param {number} [pagesPerStep=0]
+   *   The number of pages to copy per step. If not supplied or is 0, falls back
+   *   to the platform default which is currently 5.
+   * @param {number} [stepDelayMs=0]
+   *   The number of milliseconds to wait between copying step. If not supplied
+   *   or is 0, falls back to the platform default which is currently 250.
    * @return Promise<undefined, nsresult>
    */
-  async backupToFile(destFilePath) {
+  async backupToFile(destFilePath, pagesPerStep = 0, stepDelayMs = 0) {
     if (!this._dbConn) {
       return Promise.reject(
         new Error("No opened database connection to create a backup from.")
@@ -1194,13 +1200,18 @@ ConnectionData.prototype = Object.freeze({
     }
     let destFile = await IOUtils.getFile(destFilePath);
     return new Promise((resolve, reject) => {
-      this._dbConn.backupToFileAsync(destFile, result => {
-        if (Components.isSuccessCode(result)) {
-          resolve();
-        } else {
-          reject(result);
-        }
-      });
+      this._dbConn.backupToFileAsync(
+        destFile,
+        result => {
+          if (Components.isSuccessCode(result)) {
+            resolve();
+          } else {
+            reject(result);
+          }
+        },
+        pagesPerStep,
+        stepDelayMs
+      );
     });
   },
 });
@@ -2002,10 +2013,20 @@ OpenedConnection.prototype = Object.freeze({
    * @param {string} destFilePath
    *   The path on the local filesystem to write the database copy. Any existing
    *   file at this path will be overwritten.
+   * @param {number} [pagesPerStep=0]
+   *   The number of pages to copy per step. If not supplied or is 0, falls back
+   *   to the platform default which is currently 5.
+   * @param {number} [stepDelayMs=0]
+   *   The number of milliseconds to wait between copying step. If not supplied
+   *   or is 0, falls back to the platform default which is currently 250.
    * @return Promise<undefined, nsresult>
    */
-  backup(destFilePath) {
-    return this._connectionData.backupToFile(destFilePath);
+  backup(destFilePath, pagesPerStep = 0, stepDelayMs = 0) {
+    return this._connectionData.backupToFile(
+      destFilePath,
+      pagesPerStep,
+      stepDelayMs
+    );
   },
 });