1921314 - Investigate retry logic for calling into child actors when performing and releasing actions

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, task, P2)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Once bug 1904665 is done we need to strengthen the action commands that call into child actors and may face an AbortError.

Via bug 1854942 I've made updates recently as well but this is for one call into the window global for BiDi. Given that for actions we have a call into content for deserialization, and then individual ones for dispatching the action we need to wrap all action logic into a try/catch so that we can restart from the beginning.

It would be good to also have a webdriver test for this which should be Mozilla-specific again.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

As per discussion today we maybe should not retry to dispatch actions if the underlying document got replaced.

I'll see which tests I can provide to handle different scenarios.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

It turns out that we do not have to abort the action when the browsing context changes. That is a result from a test with Chrome and some new wdspec tests that I'm going to add soon to this bug. This is mainly the case because actually dispatching actions is happening in the parent process for Chrome and soon as well for Firefox (with widget events enabled). In those cases we do not have to care about the content processes because there is no JSWindowActor communication anymore.

The only exception is actually getting the center point of the element origin, but that will be racy anyway. If there is a navigation that early during a input.performActions execution we would fail with no such element. Not sure if there is a way to reliably test that.

I'm going to add new wdspec tests so we can verify the behavior.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

While I was able to create tests for key and pointer input sources for the input.performActions command I do not see a possibility to do the same for input.releaseActions. Mainly because there is no chance to delay the execution of individual actions.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Bug 1921314 - [wdspec] Add tests for "input.performActions" to verify that there are no early aborts.

Tests for "input.releaseActions" cannot be created
because there is no way to delay processing certain
actions from the action chain.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Bug 1921314 - [wdspec] Tests for "input.performActions" to check that dispatching actions finishes even with an active navigation.

Tests for "input.releaseActions" cannot be created
because there is no way to delay processing certain
actions from the action chain.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6fb4c720d554
[wdspec] Tests for "input.performActions" to check that dispatching actions finishes even with an active navigation. r=webdriver-reviewers,jgraham,jdescottes

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/48880 for changes under testing/web-platform/tests

Comment 8

[amarc]

https://hg.mozilla.org/mozilla-central/rev/6fb4c720d554

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot