Bug 1915798 - [remote] Add support for "Action queues" to sequencially perform actions without races. r=webdriver-reviewers,jdescottes

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

remote/jar.mn

@@ -14,6 +14,7 @@ remote.jar:
 
   # shared modules (all protocols)
   content/shared/AppInfo.sys.mjs (shared/AppInfo.sys.mjs)
+  content/shared/AsyncQueue.sys.mjs (shared/AsyncQueue.sys.mjs)
   content/shared/Browser.sys.mjs (shared/Browser.sys.mjs)
   content/shared/Capture.sys.mjs (shared/Capture.sys.mjs)
   content/shared/ChallengeHeaderParser.sys.mjs (shared/ChallengeHeaderParser.sys.mjs)

remote/marionette/actors/MarionetteCommandsParent.sys.mjs

@@ -389,7 +389,11 @@ export class MarionetteCommandsParent extends JSWindowActorParent {
       actions,
       this.#actionsOptions
     );
-    await actionChain.dispatch(this.#actionState, this.#actionsOptions);
+
+    // Enqueue to serialize access to input state.
+    await this.#actionState.enqueueAction(() =>
+      actionChain.dispatch(this.#actionState, this.#actionsOptions)
+    );
 
     // Process async follow-up tasks in content before the reply is sent.
     await this.#finalizeAction();
@@ -407,7 +411,12 @@ export class MarionetteCommandsParent extends JSWindowActorParent {
       return;
     }
 
-    await this.#actionState.release(this.#actionsOptions);
+    // Enqueue to serialize access to input state.
+    await this.#actionState.enqueueAction(() => {
+      const undoActions = this.#actionState.inputCancelList.reverse();
+      undoActions.dispatch(this.#actionState, this.#actionsOptions);
+    });
+
     this.#actionState = null;
 
     // Process async follow-up tasks in content before the reply is sent.

remote/shared/AsyncQueue.sys.mjs

@@ -0,0 +1,78 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/**
+ * Manages a queue of asynchronous tasks, ensuring they are processed sequentially.
+ */
+export class AsyncQueue {
+  #processing;
+  #queue;
+
+  constructor() {
+    this.#queue = [];
+    this.#processing = false;
+  }
+
+  /**
+   * Dequeue a task.
+   *
+   * @returns {Promise}
+   *     The wrapped task appearing as first item in the queue.
+   */
+  #dequeue() {
+    return this.#queue.shift();
+  }
+
+  /**
+   * Dequeue and try to process all the queued tasks.
+   *
+   * @returns {Promise<undefined>}
+   *     Promise that resolves when processing the queue is done.
+   */
+  async #processQueue() {
+    // The queue is already processed or no tasks queued up.
+    if (this.#processing || this.#queue.length === 0) {
+      return;
+    }
+
+    this.#processing = true;
+
+    while (this.#queue.length) {
+      const wrappedTask = this.#dequeue();
+      await wrappedTask();
+    }
+
+    this.#processing = false;
+  }
+
+  /**
+   * Enqueue a task.
+   *
+   * @param {Function} task
+   *     The task to queue.
+   *
+   * @returns {Promise<object>}
+   *     Promise that resolves when the task is completed, with the resolved
+   *     value being the result of the task.
+   */
+  enqueue(task) {
+    const onTaskExecuted = new Promise((resolve, reject) => {
+      // Wrap the task in a function that will resolve or reject the Promise.
+      const wrappedTask = async () => {
+        try {
+          const result = await task();
+          resolve(result);
+        } catch (error) {
+          reject(error);
+        }
+      };
+
+      // Add the wrapped task to the queue
+      this.#queue.push(wrappedTask);
+      this.#processQueue();
+    });
+
+    return onTaskExecuted;
+  }
+}

remote/shared/test/xpcshell/test_AsyncQueue.js

@@ -0,0 +1,102 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+const { setTimeout } = ChromeUtils.importESModule(
+  "resource://gre/modules/Timer.sys.mjs"
+);
+
+const { AsyncQueue } = ChromeUtils.importESModule(
+  "chrome://remote/content/shared/AsyncQueue.sys.mjs"
+);
+
+function sleep(delay = 100) {
+  // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
+  return new Promise(resolve => setTimeout(resolve, delay));
+}
+
+add_task(async function test_enqueueSyncTask() {
+  let value = "";
+
+  const queue = new AsyncQueue();
+  await Promise.all([
+    queue.enqueue(() => (value += "foo")),
+    queue.enqueue(() => (value += "bar")),
+  ]);
+
+  equal(value, "foobar", "Tasks run in the correct order");
+});
+
+add_task(async function test_enqueueAsyncTask() {
+  let value = "";
+
+  const queue = new AsyncQueue();
+  await Promise.all([
+    queue.enqueue(async () => {
+      await sleep(100);
+      value += "foo";
+    }),
+    queue.enqueue(async () => {
+      await sleep(10);
+      value += "bar";
+    }),
+  ]);
+
+  equal(value, "foobar", "Tasks run in the correct order");
+});
+
+add_task(async function test_enqueueAsyncTask() {
+  let value = "";
+
+  const queue = new AsyncQueue();
+  const promises = Promise.all([
+    queue.enqueue(async () => {
+      await sleep(100);
+      value += "foo";
+    }),
+    queue.enqueue(async () => {
+      await sleep(10);
+      value += "bar";
+    }),
+  ]);
+
+  const promise = queue.enqueue(async () => (value += "42"));
+
+  await promise;
+  await promises;
+
+  equal(value, "foobar42", "Tasks run in the correct order");
+});
+
+add_task(async function test_returnValue() {
+  const queue = new AsyncQueue();
+  const results = await Promise.all([
+    queue.enqueue(() => "foo"),
+    queue.enqueue(() => 42),
+  ]);
+
+  equal(results[0], "foo", "First task returned correct value");
+  equal(results[1], 42, "Second task returned correct value");
+});
+
+add_task(async function test_enqueueErroneousTasks() {
+  const queue = new AsyncQueue();
+
+  await Assert.rejects(
+    queue.enqueue(() => {
+      throw new Error("invalid");
+    }),
+    /Error: invalid/,
+    "Expected error was returned"
+  );
+
+  await Assert.rejects(
+    queue.enqueue(async () => {
+      throw new Error("invalid");
+    }),
+    /Error: invalid/,
+    "Expected error was returned"
+  );
+});

remote/shared/test/xpcshell/xpcshell.toml

@@ -3,6 +3,8 @@ head = "head.js"
 
 ["test_AppInfo.js"]
 
+["test_AsyncQueue.js"]
+
 ["test_ChallengeHeaderParser.js"]
 
 ["test_DOM.js"]

remote/shared/webdriver/Actions.sys.mjs

@@ -8,16 +8,18 @@
 const lazy = {};
 
 ChromeUtils.defineESModuleGetters(lazy, {
+  clearTimeout: "resource://gre/modules/Timer.sys.mjs",
+  setTimeout: "resource://gre/modules/Timer.sys.mjs",
+
   AppInfo: "chrome://remote/content/shared/AppInfo.sys.mjs",
   assert: "chrome://remote/content/shared/webdriver/Assert.sys.mjs",
-  clearTimeout: "resource://gre/modules/Timer.sys.mjs",
+  AsyncQueue: "chrome://remote/content/shared/AsyncQueue.sys.mjs",
   error: "chrome://remote/content/shared/webdriver/Errors.sys.mjs",
   event: "chrome://remote/content/shared/webdriver/Event.sys.mjs",
   keyData: "chrome://remote/content/shared/webdriver/KeyData.sys.mjs",
   Log: "chrome://remote/content/shared/Log.sys.mjs",
   pprint: "chrome://remote/content/shared/Format.sys.mjs",
   Sleep: "chrome://remote/content/marionette/sync.sys.mjs",
-  setTimeout: "resource://gre/modules/Timer.sys.mjs",
 });
 
 ChromeUtils.defineLazyGetter(lazy, "logger", () =>
@@ -77,11 +79,18 @@ const MODIFIER_NAME_LOOKUP = {
  * single State object.
  */
 action.State = class {
+  #actionsQueue;
+
   /**
    * Creates a new {@link State} instance.
    */
   constructor() {
+    // A queue that ensures that access to the input state is serialized.
+    this.#actionsQueue = new lazy.AsyncQueue();
+
+    // Tracker for mouse button clicks.
     this.clickTracker = new ClickTracker();
+
     /**
      * A map between input ID and the device state for that input
      * source, with one entry for each active input source.
@@ -97,31 +106,36 @@ action.State = class {
      */
     this.inputsToCancel = new TickActions();
 
-    /**
-     * Map between string input id and numeric pointer id
-     */
+    // Map between string input id and numeric pointer id.
     this.pointerIdMap = new Map();
   }
 
+  /**
+   * Returns the list of inputs to cancel when releasing the actions.
+   *
+   * @returns {TickActions}
+   *     The inputs to cancel.
+   */
+  get inputCancelList() {
+    return this.inputsToCancel;
+  }
+
   toString() {
     return `[object ${this.constructor.name} ${JSON.stringify(this)}]`;
   }
 
   /**
-   * Reset state stored in this object.
+   * Enqueue a new action task.
    *
-   * Note: It is an error to use the State object after calling release().
-   *
-   * @param {ActionsOptions} options
-   *     Configuration of actions dispatch.
+   * @param {Function} task
+   *     The task to queue.
    *
    * @returns {Promise}
-   *     Promise that is resolved once all inputs are released.
+   *     Promise that resolves when the task is completed, with the resolved
+   *     value being the result of the task.
    */
-  release(options) {
-    this.inputsToCancel.reverse();
-
-    return this.inputsToCancel.dispatch(this, options);
+  enqueueAction(task) {
+    return this.#actionsQueue.enqueue(task);
   }
 
   /**

remote/test/puppeteer/test/TestExpectations.json

@@ -453,13 +453,6 @@
     "expectations": ["FAIL"],
     "comment": "TODO: add a comment explaining why this expectation is required (include links to issues)"
   },
-  {
-    "testIdPattern": "[mouse.spec] Mouse should not throw if clicking in parallel",
-    "platforms": ["darwin", "linux", "win32"],
-    "parameters": ["firefox", "webDriverBiDi"],
-    "expectations": ["FAIL"],
-    "comment": "Needs support for action queue: https://bugzilla.mozilla.org/show_bug.cgi?id=1915798"
-  },
   {
     "testIdPattern": "[mouse.spec] Mouse should reset properly",
     "platforms": ["darwin", "linux", "win32"],

remote/webdriver-bidi/modules/root/input.sys.mjs

@@ -161,6 +161,27 @@ class InputModule extends RootBiDiModule {
     });
   }
 
+  /**
+   * Retrieves the action's input state.
+   *
+   * @param {BrowsingContext} context
+   *     The Browsing Context to retrieve the input state for.
+   *
+   * @returns {Actions.InputState}
+   *     The action's input state.
+   */
+  #getInputState(context) {
+    // Bug 1821460: Fetch top-level browsing context.
+    let inputState = this.#inputStates.get(context);
+
+    if (inputState === undefined) {
+      inputState = new lazy.action.State();
+      this.#inputStates.set(context, inputState);
+    }
+
+    return inputState;
+  }
+
   /**
    * Retrieve the in-view center point for the rect and visible viewport.
    *
@@ -200,6 +221,19 @@ class InputModule extends RootBiDiModule {
     );
   }
 
+  /**
+   * Resets the action's input state.
+   *
+   * @param {BrowsingContext} context
+   *     The Browsing Context to reset the input state for.
+   */
+  #resetInputState(context) {
+    // Bug 1821460: Fetch top-level browsing context.
+    if (this.#inputStates.has(context)) {
+      this.#inputStates.delete(context);
+    }
+  }
+
   async performActions(options = {}) {
     const { actions, context: contextId } = options;
 
@@ -215,20 +249,19 @@ class InputModule extends RootBiDiModule {
       );
     }
 
-    // Bug 1821460: Fetch top-level browsing context.
-    let inputState = this.#inputStates.get(context);
-    if (inputState === undefined) {
-      inputState = new lazy.action.State();
-      this.#inputStates.set(context, inputState);
-    }
-
+    const inputState = this.#getInputState(context);
     const actionsOptions = { ...this.#actionsOptions, context };
+
     const actionChain = await lazy.action.Chain.fromJSON(
       inputState,
       actions,
       actionsOptions
     );
-    await actionChain.dispatch(inputState, actionsOptions);
+
+    // Enqueue to serialize access to input state.
+    await inputState.enqueueAction(() =>
+      actionChain.dispatch(inputState, actionsOptions)
+    );
 
     // Process async follow-up tasks in content before the reply is sent.
     await this.#finalizeAction(context);
@@ -261,15 +294,16 @@ class InputModule extends RootBiDiModule {
       );
     }
 
-    // Bug 1821460: Fetch top-level browsing context.
-    let inputState = this.#inputStates.get(context);
-    if (inputState === undefined) {
-      return;
-    }
-
+    const inputState = this.#getInputState(context);
     const actionsOptions = { ...this.#actionsOptions, context };
-    await inputState.release(actionsOptions);
-    this.#inputStates.delete(context);
+
+    // Enqueue to serialize access to input state.
+    await inputState.enqueueAction(() => {
+      const undoActions = inputState.inputCancelList.reverse();
+      return undoActions.dispatch(inputState, actionsOptions);
+    });
+
+    this.#resetInputState(context);
 
     // Process async follow-up tasks in content before the reply is sent.
     this.#finalizeAction(context);

testing/web-platform/meta/webdriver/tests/classic/perform_actions/pointer_pen.py.ini

@@ -1,7 +1,6 @@
 [pointer_pen.py]
   [test_null_response_value]
-    expected:
-      ERROR
+    expected: FAIL
 
   [test_pen_pointer_in_shadow_tree[outer-open\]]
     expected: FAIL