1922077 - Enable async event dispatching (remote.events.async.enabled) by default

Status: RESOLVED FIXED

(Remote Protocol :: Agent, task, P2)

Description

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

At a certain point when all events can be dispatched as widget (async) events we should consider turning the preference remote.events.async.enabled on by default. Maybe this should happen for Nightly builds first.

Comment 1

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

(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #0)

At a certain point when all events can be dispatched as widget (async) events we should consider turning the preference remote.events.async.enabled on by default. Maybe this should happen for Nightly builds first.

We actually do not need the widget events to enable the first batch of changes for processing actions in the parent process. For widget events I'm going to add additional preferences for each input type, which will depend on the above preference's value.

Turning on the feature by default requires all known issues to be fixed. I'm certain that we can enable it for the next Firefox 135 train. As such lets move this bug into the M14 milestone.

Comment 2

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

Before enabling the feature we should do some quick performance checks in how much the extra IPC communication delays the processing of the action chain, but not in that detail as initially assumed via bug 1904859.

Comment 3

[Julian Descottes [:jdescottes]]

Need to decide what to do for the job.

Comment 4

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

(In reply to Julian Descottes [:jdescottes] from comment #3)

Need to decide what to do for the job.

Maybe we can leave the async Wd jobs untouched for a couple of days until we have the first widget event for wheel scrolling implemented. Then we could use this job to have these extra preferences (like remote.events.async.wheel.enabled) enabled.

Comment 5

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

Over the last days since Saturday last week I was not able to find any test failure with the async code enabled that is related to the list of failures in the dependency list. If it stays that way we may consider to push a patch for this bug already next week when the version bump to 135 is done.

Comment 6

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

Attachment: Bug 1922077 - [wdspec] Fix expectation for several tests when remote async events are enabled.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6eda5763eee9
[wdspec] Fix expectation for several tests when remote async events are enabled. r=webdriver-reviewers,Sasha

Comment 8

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

I'm going to land the update for the test expectations but the rest will need to be followed-up by next week. I'm still seeing at least one issue for Marionette integration tests.

Comment 9

[Norisz Fay [:noriszfay]]

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

Comment 10

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

We discussed this today and agreed on that for now we will keep the async tests active. Soon I will start on the first wheel scroll events that will be send as widget events and we need special test scenarios in CI again. We are going to re-use that test job for it by maybe specifying a specific tag or by passing specific folders only for testing.

Comment 11

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

(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #10)

We discussed this today and agreed on that for now we will keep the async tests active. Soon I will start on the first wheel scroll events that will be send as widget events and we need special test scenarios in CI again. We are going to re-use that test job for it by maybe specifying a specific tag or by passing specific folders only for testing.

It would make sense to not run those tests by default for the time being but have them available just for try builds. As discussed with Julian I'll make that change and then we can figure out how to continue using these jobs when the first widget event is about to land.

Comment 12

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

Attachment: Bug 1922077 - [remote] Enable processing and dispatching actions via the parent process by default.

Comment 13

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

Attachment: Bug 1922077 - [wdspec] Fix navigation tests for the perform action commands.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d718674957f8
[wdspec] Fix navigation tests for the perform action commands. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/ad7280231844
[remote] Enable processing and dispatching actions via the parent process by default. r=webdriver-reviewers,taskgraph-reviewers,bhearsum,jdescottes

Comment 15

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

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

Comment 16

[agoloman]

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

• Backout link
• Push with failures
• Failure Log
  ————————————————————————————————————
• Push with failures tier 2 wpt failures @input-events-spin-button-click-on-number-input-delete-document.html.
• Failure Log

Comment 17

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

Upstream PR was closed without merging

Comment 18

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

The problem with the test input-events-spin-button-click-on-number-input-delete-document.html is that an action actually closes the frame we want to dispatch another action to. I filed bug 1935324 to get this corrected.

For the failure in input-events-get-target-ranges-joining-dl-elements.tentative.html it's still unclear to me what is causing it. As it looks like only the GeckoView Lite package is affected. I've pushed a try build with trace logs enabled to see more details of what's going on given that I cannot run this kind of build locally on my M1 MacBook:

https://treeherder.mozilla.org/jobs?repo=try&revision=653f1f2c9c456fe709167d812e34138395b9d0a4

Comment 19

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

The failure in input-events-get-target-ranges-joining-dl-elements.tentative.html doesn't happen anymore in my try build after I rebased on latest mozilla-central. Maybe it was a side-effect. I would try landing it again with the other test marked as expected fail.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a0a6da1b41c7
[wdspec] Fix navigation tests for the perform action commands. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/8e40eaab9ad5
[remote] Enable processing and dispatching actions via the parent process by default. r=webdriver-reviewers,taskgraph-reviewers,bhearsum,jdescottes

Comment 21

[Sandor Molnar[:smolnar]]

Backed out for causing wpt failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/a47beb271989b0d3835f38d0392a2a678467adbe

Push with failures

Failure log -> https://treeherder.mozilla.org/logviewer?job_id=485643644&repo=autoland&lineNumber=4936

Comment 22

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

Upstream PR was closed without merging

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6e7923664afa
[wdspec] Fix navigation tests for the perform action commands. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/2f4ac27d288f
[remote] Enable processing and dispatching actions via the parent process by default. r=webdriver-reviewers,taskgraph-reviewers,bhearsum,jdescottes,jgraham

Comment 24

[agoloman]

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

• Backout link
• Push with failures
• Failure Log

Comment 25

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

Upstream PR was closed without merging

Comment 26

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

I pushed a new try build with all the tests in that file set to multiple statuses (fail, timeout, notrun) on Android:
https://treeherder.mozilla.org/jobs?repo=try&revision=655736ade6161b3db34cde3865c05b3b535901eb

Comment 27

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

Attachment: Bug 1922077 - [remote] Enable processing and dispatching actions via the parent process by default.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52ba62ee5d9a
[wdspec] Fix navigation tests for the perform action commands. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/746421efcf13
[remote] Enable processing and dispatching actions via the parent process by default. r=webdriver-reviewers,taskgraph-reviewers,bhearsum,jdescottes,jgraham

Comment 29

[amarc]

https://hg.mozilla.org/mozilla-central/rev/52ba62ee5d9a
https://hg.mozilla.org/mozilla-central/rev/746421efcf13

Comment 30

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

Upstream PR merged by moz-wptsync-bot