1313170 - helper_long_tap.html | Got a mouselongtap - got "touchend", expected "mouselongtap" when GPU process enabled

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Try push with more logging: https://treeherder.mozilla.org/#/jobs?repo=try&revision=11ed136bc7ea1323261ba19156cf05fc1b9a1fb3

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Also, this is the failure in question: https://treeherder.mozilla.org/#/jobs?repo=try&author=[e-mail]&selectedJob=29898382

Seems to be win8 only, and win8 jobs are hidden on central branches, so this technically doesn't need to block gpu process enabling. It might be a legit bug though.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

From the log it looks like during the previous test (helper_bug1162771.html) touch injection fails for some reason. The log has this:
 INFO -  InjectTouchInput failure. GetLastError=87
87 appears to be the ERROR_INVALID_PARAMETER documented at [1]. I'm not sure what's going on there since the touch events seem to get through and that particular subtest passes.

Later after the helper_long_tap has occurred, there's another failure in the log:
 INFO -  InjectTouchInput failure. GetLastError=1460
Where 1460 is ERROR_TIMEOUT. Not sure what that's about either.

I think I'll try to repro this locally tomorrow, it might be easier to debug.

[1] https://msdn.microsoft.com/en-us/library/windows/desktop/hh802881(v=vs.85).aspx

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Parking with me for now.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

I can reproduce this locally on my Win8 device even without the GPU process patches. The ERROR_INVALID_PARAMETER is because helper_bug1162771.html does a thing where it makes the element display:none, which results in different coordinates being used for the touchend than were for the touchstart. The windows injection code doesn't like this, presumably because it expects to get touchmove events in between.

The ERROR_TIMEOUT is the more serious problem though, because according to the docs [1] the injection mechanism requires a new input frame at least every 100ms. In our tests we sometimes wait longer than 100ms because we're waiting for a repaint or (in this case) a long-tap timeout, so we can't really satisfy that requirement.

I think maybe the best option here is to not use the Windows touch injection code and fall back to the fallback code we have that runs for platforms that don't support touch injection. I'll do a try push with that and see if it works.

[1] https://msdn.microsoft.com/en-us/library/windows/desktop/hh802881(v=vs.85).aspx

Comment 6

[Kartikaya Gupta (email:[e-mail])]

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

Comment 7

[Kartikaya Gupta (email:[e-mail])]

^ That's green. I'll add a new pref to force us into the fallback touch injection path and make sure it's set on the relevant tests.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

New cleaned-up pushes with [1] and without [2] GPU process code

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=d94d98a54e6289038b352290f6dfba3c63c69b59
[2] https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f60ae748b81c8fc2a6b73a1ae1f817bd6774348

Comment 9

[Kartikaya Gupta (email:[e-mail])]

[1] above has failures, it looks like more tests need the magic pref.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=461c4ccbef1e3c803d233797374de3bb86f8a0db

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1313170 - Use the widget-level touch injection instead of the OS-level touch injection for some tests.

When using the InjectTouchInput API on Windows, the API requires that the caller
keep providing input frames (by calling the API function) at least every 100ms.
If the caller fails to do so, Windows can return an ERROR_TIMEOUT and throw
away the touch sequence. In some tests, it is hard for us to make this guarantee,
because we need to wait for other events between the touchdown and touchup. For
these tests, we can use the widget-level touch injection code that we have as
a fallback for the OS-level touch injection code. The widget-level touch
injection is less representative of real-world usage but allows us to bypass
the timeout problem.

Review commit: https://reviewboard.mozilla.org/r/89544/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89544/

Comment 11

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1313170 - Ensure that the touchend events in helper_bug1162771 are sent with the correct coordinates.

The test in helper_bug1162771 dispatches a touchstart on an element, and then
inside the touchstart handler makes that element display:none. It then sends a
touchend at what should be the same position. However, since the position of the
touch is calculated from the position of the element, the touchend position can
be wrong because the element is set to display:none in between. This patch
saves the position before sending the touchstart, and makes sure to use the
same position for the touchend.

Review commit: https://reviewboard.mozilla.org/r/89546/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89546/

Comment 12

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 8805945 [details]
Bug 1313170 - Use the widget-level touch injection instead of the OS-level touch injection for some tests.

https://reviewboard.mozilla.org/r/89544/#review89020

Comment 13

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 8805946 [details]
Bug 1313170 - Ensure that the touchend events in helper_bug1162771 are sent with the correct coordinates.

https://reviewboard.mozilla.org/r/89546/#review89022

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9b244c814861
Use the widget-level touch injection instead of the OS-level touch injection for some tests. r=dvander
https://hg.mozilla.org/integration/autoland/rev/c051b525c297
Ensure that the touchend events in helper_bug1162771 are sent with the correct coordinates. r=dvander

Comment 15

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/9b244c814861
https://hg.mozilla.org/mozilla-central/rev/c051b525c297