1929757 - PANGESTURE_MAYSTART event can potentially stall the APZ input queue

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect, P2)

Description

[Botond Ballo [:botond]]

In bug 1926982, Markus reported the following regression from the (since backed out) bug 1926141:

(In reply to Markus Stange [:mstange] from comment #2)

I'm also seeing strange behavior when putting the fingers on the touchpad in a subframe. When I pause at the start of a scroll, the movement of the scroll is delayed. I see this in the timeline of the profiler when having more threads visible than fit. The profiler has overscroll-behavior: none set on the html and body elements, which may be related.

After looking at some APZ logs, I discovered that the problem is the following:

• When doing a two-finger scroll on the touchpad, Mac first sends a PANGESTURE_MAYSTART (when the fingers go down), followed by a PANGESTURE_START (when scrolling starts)
• Both events start a new input block, such that there will be an input block containing only the MAYSTART event
• The MAYSTART event does not get sent to web content because it has a zero delta
• As a result, the MAYSTART event's input block never receives a content response
• If the page has an active wheel event listener, the input block is therefore never confirmed and stalls the input queue until it times out

I'm not sure yet if this scenario affects Linux. (On Linux, we may get a "hold-end" (PANGESTURE_CANCELLED) in between the MAYSTART and the START, such that the above can never happen.) I will double check this when I'm at my Wayland machine.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

(In reply to Botond Ballo [:botond] from comment #0)

In bug 1926982, Markus reported the following regression from the (since backed out) bug 1926141:

(In reply to Markus Stange [:mstange] from comment #2)

I'm also seeing strange behavior when putting the fingers on the touchpad in a subframe. When I pause at the start of a scroll, the movement of the scroll is delayed. I see this in the timeline of the profiler when having more threads visible than fit. The profiler has overscroll-behavior: none set on the html and body elements, which may be related.

After looking at some APZ logs, I discovered that the problem is the following:

• When doing a two-finger scroll on the touchpad, Mac first sends a PANGESTURE_MAYSTART (when the fingers go down), followed by a PANGESTURE_START (when scrolling starts)
• Both events start a new input block, such that there will be an input block containing only the MAYSTART event
• The MAYSTART event does not get sent to web content because it has a zero delta
• As a result, the MAYSTART event's input block never receives a content response
• If the page has an active wheel event listener, the input block is therefore never confirmed and stalls the input queue until it times out

This scenario is basically same as the scenario I found in bug 1919566. In the bug 1919566 case, the event is PANGESTURE_END, and the bug behavior happens after the timeout though.

Comment 2

[Botond Ballo [:botond]]

Attachment: Bug 1929757 - Do not wait for a content response for a hold gesture. r=dlrobertson,hiro

The MAYSTART never gets to content (it has a zero delta), so if it's
the only input event in its input block, that input block will never
be confirmed by a content response and will stall the input queue
until it times out.

Comment 3

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #0)

I'm not sure yet if this scenario affects Linux. (On Linux, we may get a "hold-end" (PANGESTURE_CANCELLED) in between the MAYSTART and the START, such that the above can never happen.) I will double check this when I'm at my Wayland machine.

I tested this. It turns out that Linux does send a CANCELLED event in between the MAYSTART and the START, but it's nonetheless affected by this bug (the CANCELLED event doesn't do anything to remove the stalled event from the input queue). Moreover, the fix I currently posted does not apply to this scenario.

This, together with the fact that Hiro has found another issue with my fix, and Markus has found yet another one (the MAYSTART event not having a delta means that we can pick a different target APZC for it than the one we would pick for the START event which does have a delta) is making me gravitate towards a different fix approach: let the MAYSTART event (along with a possible subsequent CANCELLED) have its own input block, but consider that input block to be IsReadyForHandling() == true from the start, since it will not receive a content response.

Comment 4

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #3)

I tested this. It turns out that Linux does send a CANCELLED event in between the MAYSTART and the START, but it's nonetheless affected by this bug (the CANCELLED event doesn't do anything to remove the stalled event from the input queue).

I guess that means this is a regression from bug 1568722 affecting Linux.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1568722

Comment 6

[Donal Meehan [:dmeehan]]

:botond, I know the patch is still in review, but reaching out now since we only have one beta build left in the Fx133 cycle.

• What is the severity of this?
• Do you plan on adding a beta uplift request on it?
  • If yes, any concerns with landing and adding a beta uplift request in time for the final beta build? (i.e. request by eod Thursday 2024-11-14)

Comment 7

[Botond Ballo [:botond]]

I'm going to mark this as an S3: this only affects scroll frames with active wheel listeners, which are the minority, and scrolling does resume after a pause.

I'd still like to avoid shipping this regression in 133, as the pauses are a potential annoyance. We'll consider two options in our team meeting today: (1) uplifting this patch to beta, and (2) prefing off hold gestures in 133 (using the pref added and uplifted in bug 1929746), and get back to you with a recommendation. Leaving needinfo until then.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/99adc86b3ee1
Do not wait for a content response for a hold gesture. r=hiro

Comment 9

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #7)

I'd still like to avoid shipping this regression in 133, as the pauses are a potential annoyance. We'll consider two options in our team meeting today: (1) uplifting this patch to beta, and (2) prefing off hold gestures in 133 (using the pref added and uplifted in bug 1929746), and get back to you with a recommendation.

We discussed this and would like to go to option (2), prefing off hold gestures in 133. I'll file a bug with a patch that does that shortly, for uplift to 133. Once that's in place, this bug should become firefox133: disabled.

Comment 10

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #9)

We discussed this and would like to go to option (2), prefing off hold gestures in 133. I'll file a bug with a patch that does that shortly, for uplift to 133. Once that's in place, this bug should become firefox133: disabled.

Tracking this is bug 1931142.

Comment 11

[Donal Meehan [:dmeehan]]

The feature was disabled in beta via Bug 1931142

Comment 12

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/99adc86b3ee1