1682757 - event.DoubleClickTracker not handled by MarionetteEventsActor

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect, P1)

Description

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

As noticed when landing my patch for bug 1669174, a wpt test checking for auxclick events is perma failing. Reason is that the event.DoubleClickTracker isn't properly updated by the MarionetteEventsActor. The framescript used the click, dblclick, unload events to update the tracker:

https://searchfox.org/mozilla-central/rev/8883276967d39918e2ce64e873afdd432fb406ca/testing/marionette/listener.js#66-70

I wonder why the wdspec action tests don't fail. Maybe we miss tests for double click? I will have a look.

Comment 1

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

The problem here is that the registration of the event handlers happen in MarionetteCommandsActor:

https://searchfox.org/mozilla-central/rev/8883276967d39918e2ce64e873afdd432fb406ca/testing/marionette/actors/MarionetteCommandsChild.jsm#62-77

That actor is only created when actually running a WebDriver command, but not when the site itself or the testharness in wpt triggers mouse events.

So moving these over to the MarionetteEventsActor should hopefully do it.

Comment 2

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

Actually the fix on bug 1669176 is wrong. We cannot store the inputStateIsDirty flag in the child actor given that for each frame we get a different actor, and the state of the flag is gone when the actor gets destroyed. Instead it would have to be moved to the parent actors module.

Comment 3

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

My last comment is partly true. While we should indeed move it to the parent, this involves a lot of work to do. As such I would propose that we move it to a separate bug. Just moving the event listeners to the events actor seems to fix it. Here a new try:

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

Comment 4

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

Attachment: Bug 1682757 - [marionette] Register MarionetteEventsActor globally.

To not miss certain events the event actor has to stay registered
throughout the whole lifetime of the session.

Comment 5

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

Attachment: Bug 1682757 - [marionette] Handle events for DoubleClickTracker in MarionetteEventsActor.

Depends on D99884

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/596508c4b86c
[marionette] Register MarionetteEventsActor globally. r=marionette-reviewers,jgraham
https://hg.mozilla.org/integration/autoland/rev/f6185b36024b
[marionette] Handle events for DoubleClickTracker in MarionetteEventsActor. r=marionette-reviewers,jgraham

Comment 7

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

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

Actually the fix on bug 1669176 is wrong. We cannot store the inputStateIsDirty flag in the child actor given that for each frame we get a different actor, and the state of the flag is gone when the actor gets destroyed. Instead it would have to be moved to the parent actors module.

It may indeed be better to store this state in the parent (that's not clear to me), but note that the scope of inputStateIsDirty right now is the whole MarionetteCommandsChild module, not the actor class, so that particular state is not gone when the actor gets destroyed.

Comment 8

[Natalia Csoregi [:nataliaCs]]

Backed out for pointerevents failures.

backout: https://hg.mozilla.org/integration/autoland/rev/786e0ad1a63a2c8e5968a61be88c32e414ced307

push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&classifiedState=unclassified&searchStr=windows%2C10%2Cx64%2Cwebrender%2Copt%2Cweb%2Cplatform%2Ctests%2Cwith%2Cfission%2Cenabled%2Ctest-windows10-64-qr%2Fopt-web-platform-tests-fis-e10s%2Cwpt2&revision=1a95e03f79f66743ed54fe359b326e8f5778401d

retrigger/backfill: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&classifiedState=unclassified&fromchange=4e07cf6a753615eb882847e1a57705c443cb2c5f&searchStr=windows%2C10%2Cx64%2Cwebrender%2Copt%2Cweb%2Cplatform%2Ctests%2Cwith%2Cfission%2Cenabled%2Ctest-windows10-64-qr%2Fopt-web-platform-tests-fis-e10s%2Cwpt2&selectedTaskRun=EZYjS0RCSX-4J-YsG6I0hg.0

failure log: https://treeherder.mozilla.org/logviewer?job_id=324726418&repo=autoland&lineNumber=2238

[task 2020-12-16T18:24:10.805Z] 18:24:10 INFO - TEST-START | /pointerevents/pointerevent_lostpointercapture_for_disconnected_node_in_shadow_dom.html
[task 2020-12-16T18:24:10.810Z] 18:24:10 INFO - Setting pref layout.reflow.synthMouseMove (true)
[task 2020-12-16T18:24:10.816Z] 18:24:10 INFO - Closing window 8589934593
[task 2020-12-16T18:24:10.906Z] 18:24:10 INFO - {'actions': [{'type': 'none', 'actions': [{'type': 'pause', 'duration': 16}, {'type': 'pause', 'duration': 16}, {'type': 'pause', 'duration': 16}], 'id': '0'}, {'type': 'pointer', 'actions': [{'type': 'pointerMove', 'x': 0, 'y': 0, 'origin': {'element-6066-11e4-a52e-4f735466cecf': '7e0b766d-d917-4268-a0df-43b8e91e90aa', 'chromeelement-9fc5-4b51-a3c8-01716eedeb04': '7e0b766d-d917-4268-a0df-43b8e91e90aa'}}, {'type': 'pointerDown', 'button': 0}, {'type': 'pointerUp', 'button': 0}], 'parameters': {'pointerType': 'mouse'}, 'id': '1'}]}
[task 2020-12-16T18:24:20.895Z] 18:24:20 INFO -
[task 2020-12-16T18:24:20.895Z] 18:24:20 INFO - TEST-UNEXPECTED-TIMEOUT | /pointerevents/pointerevent_lostpointercapture_for_disconnected_node_in_shadow_dom.html | lostpointercapture is dispatched on the document when shadow dom capturing element is removed - Test timed out
[task 2020-12-16T18:24:20.896Z] 18:24:20 INFO - TEST-UNEXPECTED-TIMEOUT | /pointerevents/pointerevent_lostpointercapture_for_disconnected_node_in_shadow_dom.html | expected OK
[task 2020-12-16T18:24:20.896Z] 18:24:20 INFO - TEST-INFO took 10091ms
[task 2020-12-16T18:24:20.904Z] 18:24:20 INFO - PID 4868 | 1608143060903 Marionette INFO Stopped listening on port 50640
[task 2020-12-16T18:24:21.251Z] 18:24:21 INFO - Browser exited with return code 0
[task 2020-12-16T18:24:21.252Z] 18:24:21 INFO - PROCESS LEAKS None

Comment 9

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

By moving the event registration code over to the MarionetteEventsActor I accidentally also deleted a call to clearActionInputState():
https://hg.mozilla.org/integration/autoland/rev/f6185b36024b#l1.28

That call should have been left in place. The try build looks fine so far:
https://treeherder.mozilla.org/jobs?repo=try&revision=db992aa0b7ad11af7631e14302b207e4d6b2dded

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/21fb944ca634
[marionette] Register MarionetteEventsActor globally. r=marionette-reviewers,jgraham
https://hg.mozilla.org/integration/autoland/rev/bd98eeb06846
[marionette] Handle events for DoubleClickTracker in MarionetteEventsActor. r=marionette-reviewers,jgraham

Comment 11

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/21fb944ca634
https://hg.mozilla.org/mozilla-central/rev/bd98eeb06846

Comment 12

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

Marking as regression from bug 1669176. Given that the problem actually only manifests when the framescript is not getting loaded, I would say we wontfix that for Firefox 85.