Closed Bug 1915798 Opened 3 months ago Closed 2 months ago

Add support for "Action queues" to sequencially perform actions without races

Categories

(Remote Protocol :: Agent, task, P2)

Product:
Remote Protocol
Component:
Agent
Type:
task
Points:
3

Tracking

(firefox133 fixed)

Status:
RESOLVED FIXED
Milestone:
133 Branch
Tracking Flags:
Tracking Status
firefox133 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

(Blocks 2 open bugs,
URL
)

Details

(Whiteboard: [webdriver:m13], [wptsync upstream][webdriver:relnote])

Attachments

(2 files)

Bug 1915798 - [wdspec] Add WebDriver BiDi tests to check queuing of actions.
48 bytes, text/x-phabricator-request
Details | Review
Bug 1915798 - [remote] Add support for "Action queues" to sequencially perform actions without races.
48 bytes, text/x-phabricator-request
Details | Review

While working on bug 1904665, I noticed that we currently have a race condition in our action dispatching code. This issue causes actions submitted in parallel via WebDriver BiDi to execute simultaneously. However, according to the WebDriver classic specification, actions need to be run sequentially by ensuring that a token is created and placed in an action queue:

https://w3c.github.io/webdriver/#dfn-dispatch-actions

This ensures that only one set of actions can be run at a time, and therefore different actions commands using the same underlying state don't race. In a session that is only a HTTP session only one command can run at a time, so this will never block. But other session types can allow running multiple commands in parallel, in which case this is necessary to ensure sequential access.

With the patch from bug 1904665, the Puppeteer test "[mouse.spec] Mouse should not throw if clicking in parallel" is now failing because the action processing and dispatching in the parent process starts the individual dispatching of events earlier than before. This causes both actions to overlap and interfere with each other.

I propose that, for now, we mark the test as failing and immediately follow up with this bug to get it fixed in the same version. This is, of course, unless there are other issues in the web-platform tests. I'll submit a full try build soon to sort this out.

Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Points: --- → 5
Priority: -- → P2
Whiteboard: [webdriver:m12]

The implementation is easier than initially thought so we can lower the points.

Points: 5 → 3
Blocks: 1821460
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/27efecb55f94 [remote] Add support for "Action queues" to sequencially perform actions without races. r=webdriver-reviewers,jdescottes https://hg.mozilla.org/integration/autoland/rev/c163aae0b922 [wdspec] Add WebDriver BiDi tests to check queuing of actions. r=webdriver-reviewers,jdescottes
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/48016 for changes under testing/web-platform/tests
Whiteboard: [webdriver:m12] → [webdriver:m12], [wptsync upstream]

The underlying problem for the drag & drop failures come from bug 1904665.

Flags: needinfo?(hskupin)
Upstream PR merged by moz-wptsync-bot
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f971ce67915c [remote] Add support for "Action queues" to sequencially perform actions without races. r=webdriver-reviewers,jdescottes https://hg.mozilla.org/integration/autoland/rev/d0e02650c62f [wdspec] Add WebDriver BiDi tests to check queuing of actions. r=webdriver-reviewers,jdescottes
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/48073 for changes under testing/web-platform/tests

Backed out for causing wpt failures on input-events-get-target-ranges-joining-dl-elements.tentative.html.

[task 2024-09-10T16:13:53.766Z] 16:13:53     INFO - TEST-START | /input-events/input-events-arrow-key-on-number-input-delete-document.html
[task 2024-09-10T16:13:53.790Z] 16:13:53     INFO - Closing window d53441cd-c72e-4235-adf2-d16a95b48aa2
[task 2024-09-10T16:13:54.528Z] 16:13:54  WARNING - Action send_keys failed
[task 2024-09-10T16:13:54.551Z] 16:13:54  WARNING - Traceback (most recent call last):
[task 2024-09-10T16:13:54.551Z] 16:13:54  WARNING -   File "/builds/worker/workspace/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py", line 898, in run_func
[task 2024-09-10T16:13:54.551Z] 16:13:54  WARNING -     self.result = True, self.func(self.protocol, self.url, self.timeout)
[task 2024-09-10T16:13:54.551Z] 16:13:54  WARNING -   File "/builds/worker/workspace/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/executormarionette.py", line 1024, in do_testharness
[task 2024-09-10T16:13:54.551Z] 16:13:54  WARNING -     done, rv = handler(result)
[task 2024-09-10T16:13:54.551Z] 16:13:54  WARNING -   File "/builds/worker/workspace/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/base.py", line 755, in __call__
[task 2024-09-10T16:13:54.552Z] 16:13:54  WARNING -     return callback(url, payload)
[task 2024-09-10T16:13:54.552Z] 16:13:54  WARNING -   File "/builds/worker/workspace/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/base.py", line 772, in process_action
[task 2024-09-10T16:13:54.552Z] 16:13:54  WARNING -     result = action_handler(payload)
[task 2024-09-10T16:13:54.552Z] 16:13:54  WARNING -   File "/builds/worker/workspace/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/actions.py", line 92, in __call__
[task 2024-09-10T16:13:54.552Z] 16:13:54  WARNING -     element = self.protocol.select.element_by_selector(selector)
[task 2024-09-10T16:13:54.552Z] 16:13:54  WARNING -   File "/builds/worker/workspace/build/tests/web-platform/tests/tools/wptrunner/wptrunner/executors/protocol.py", line 289, in element_by_selector
[task 2024-09-10T16:13:54.552Z] 16:13:54  WARNING -     raise ValueError(f"Selector '{element_selector}' matches no elements")
[task 2024-09-10T16:13:54.552Z] 16:13:54  WARNING - ValueError: Selector ':root > *|body:nth-child(2) > *|input:nth-child(1)' matches no elements
[task 2024-09-10T16:13:54.552Z] 16:13:54  WARNING - 
[task 2024-09-10T16:13:54.625Z] 16:13:54     INFO - TEST-ERROR | /input-events/input-events-arrow-key-on-number-input-delete-document.html | took 857ms
[task 2024-09-10T16:13:54.996Z] 16:13:54     INFO - STDOUT: cleanup aborted: Unable to remount device
[task 2024-09-10T16:13:55.119Z] 16:13:55     INFO - STDOUT: cleanup aborted: Unable to remount device
[task 2024-09-10T16:13:55.121Z] 16:13:55     INFO - Closing logging queue
[task 2024-09-10T16:13:55.121Z] 16:13:55     INFO - queue closed
[task 2024-09-10T16:13:55.136Z] 16:13:55     INFO - Setting up ssl
[task 2024-09-10T16:13:55.288Z] 16:13:55     INFO - certutil | b''
[task 2024-09-10T16:13:55.338Z] 16:13:55     INFO - certutil | b''
[task 2024-09-10T16:13:55.347Z] 16:13:55     INFO - certutil | b'\nCertificate Nickname                                         Trust Attributes\n                                                             SSL,S/MIME,JAR/XPI\n\nweb-platform-tests                                           CT,, \n'
[task 2024-09-10T16:13:55.938Z] 16:13:55     INFO - adb Granting important runtime permissions to org.mozilla.geckoview.test_runner
[task 2024-09-10T16:13:57.109Z] 16:13:57     INFO - adb launch_application: am start -W -n org.mozilla.geckoview.test_runner/org.mozilla.geckoview.test_runner.TestRunnerActivity -a android.intent.action.MAIN --es env0 MOZ_CRASHREPORTER=1 --es env1 MOZ_CRASHREPORTER_NO_REPORT=1 --es env2 MOZ_CRASHREPORTER_SHUTDOWN=1 --es env3 MOZ_HIDE_RESULTS_TABLE=1 --es env4 MOZ_IN_AUTOMATION=1 --es env5 MOZ_LOG=signaling:3,mtransport:4,DataChannel:4,jsep:4 --es env6 R_LOG_LEVEL=6 --es env7 R_LOG_DESTINATION=stderr --es env8 R_LOG_VERBOSE=1 --es env9 MOZ_PROCESS_LOG=/tmp/tmp1810c69spidlog --es env10 MOZ_DISABLE_NONLOCAL_CONNECTIONS=1 --es arg0 -no-remote --es arg1 -profile --es arg2 /data/local/tmp/test_root/profile --es arg3 --marionette --es arg4 about:blank
[task 2024-09-10T16:13:58.980Z] 16:13:58     INFO - Starting runner
[task 2024-09-10T16:13:59.945Z] 16:13:59     INFO - TEST-START | /input-events/input-events-arrow-key-on-number-input-prevent-default.html
Flags: needinfo?(hskupin)
Upstream PR merged by moz-wptsync-bot
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/937789e1206d [remote] Add support for "Action queues" to sequencially perform actions without races. r=webdriver-reviewers,jdescottes https://hg.mozilla.org/integration/autoland/rev/99755cd9107c [wdspec] Add WebDriver BiDi tests to check queuing of actions. r=webdriver-reviewers,jdescottes
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/48148 for changes under testing/web-platform/tests

Details for the backout are on the other bug. Those changes did not cause the tests to fail.

Flags: needinfo?(hskupin)

Backed out for causing wpt failures in input-events-get-target-ranges-joining-dl-elements.tentative.html

  • Backout link
  • Push with failures
  • Failure Log
  • Failure line: TEST-UNEXPECTED-NOTRUN | /input-events/input-events-get-target-ranges-joining-dl-elements.tentative.html?Backspace | Backspace at "<dl><dd>list-item1[</dd></dl><dl><dd>list-item2]</dd><dd>list-item3</dd></dl>" - expected FAIL
    TEST-UNEXPECTED-NOTRUN | /input-events/input-events-get-target-ranges-joining-dl-elements.tentative.html?Backspace | Backspace at "<dl><dd>[list-item1</dd></dl><dl><dd>list-item2]</dd><dd>list-item3</dd></dl>" - expected FAIL
Flags: needinfo?(hskupin)
Upstream PR merged by moz-wptsync-bot

The failures are most likely coming from bug 1904665.

Flags: needinfo?(hskupin)
Whiteboard: [webdriver:m12], [wptsync upstream] → [webdriver:m13], [wptsync upstream]
Blocks: 1874559
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/efeb4fdb8b5e [remote] Add support for "Action queues" to sequencially perform actions without races. r=webdriver-reviewers,jdescottes https://hg.mozilla.org/integration/autoland/rev/2c80eec70b9b [wdspec] Add WebDriver BiDi tests to check queuing of actions. r=webdriver-reviewers,jdescottes
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/48391 for changes under testing/web-platform/tests

https://hg.mozilla.org/mozilla-central/rev/efeb4fdb8b5e
https://hg.mozilla.org/mozilla-central/rev/2c80eec70b9b

Status: ASSIGNED → RESOLVED
Closed: 2 months ago
status-firefox133: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 133 Branch
Upstream PR merged by moz-wptsync-bot
Regressions: 1930845
Whiteboard: [webdriver:m13], [wptsync upstream] → [webdriver:m13], [wptsync upstream][webdriver:relnote]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: