1105836 - Flywheel scrolling breaks in some cases with event-regions enabled

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

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

Load latest B2G on flame and turn on layout.event-regions.enabled. Long-press on a homescreen icon to enter edit mode, and then do a double-fling to flywheel-scroll. ER: Flywheel AR: device stops responding to touches

This is a regression from bug 1085404, which made the APZ stop waiting for main-thread content responses while the APZ was in a "fast motion" state. However I stupidly forgot to also confirm the target APZ, so what ends up happening in the case of an initially unconfirmed target is that the input block never gets processed. It gets stuck in the queue, and since all those input events are never delivered to content, the target APZ is never confirmed.

Comment 1

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

Attachment: Part 1 - Logging

Lots of logging

Comment 2

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

Attachment: Part 2 - Fix

Comment 3

[Botond Ballo [:botond]]

Comment on attachment 8529862 [details] [diff] [review]
Part 1 - Logging

Review of attachment 8529862 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ +2335,5 @@
>  
>  bool AsyncPanZoomController::IsMovingFast() const {
>    ReentrantMonitorAutoEnter lock(mMonitor);
> +  if (GetVelocityVector().Length() > gfxPrefs::APZFlingStopOnTapThreshold()) {
> +    APZC_LOG("%p is moving fast; x-axis: %p, y-axis: %p\n", this, &mX, &mY);

Rather than looking at Axis addresses during debugging, what do you think about giving Axis a virtual method 'const char* Name()', with the implementations returning "x" and "y" respectively, and have debugging statements print Name()?

Comment 4

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

Name () will help with the biggest problem but there are still going to be multiple x- and y-axis objects, one for each APZC. Any ideas on how to disambiguate those without the address?

Comment 5

[Botond Ballo [:botond]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #4)
> Name () will help with the biggest problem but there are still going to be
> multiple x- and y-axis objects, one for each APZC. Any ideas on how to
> disambiguate those without the address?

We can print the address of the APZC along with Name(). That still reduces the number of addresses we have to track while debugging.

I actually prefer identifying APZCs by their ScrollableLayerGuid rather than their address (and that's what I print when I write logging statements of my own), but that's a larger change that we can do separately if we want.

Comment 6

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

Attachment: Part 1 - Logging (v2)

(In reply to Botond Ballo [:botond] from comment #5)
> We can print the address of the APZC along with Name(). That still reduces
> the number of addresses we have to track while debugging.

Good idea! Updated patch to do that.

> I actually prefer identifying APZCs by their ScrollableLayerGuid rather than
> their address (and that's what I print when I write logging statements of my
> own), but that's a larger change that we can do separately if we want.

Yeah we can do that separately. I like having the address there as well because it's easier to grep and match stuff from APZCTM output as well.

Comment 7

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

https://hg.mozilla.org/integration/b2g-inbound/rev/3910f8c77b51
https://hg.mozilla.org/integration/b2g-inbound/rev/5a68579c6fbf

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/3910f8c77b51
https://hg.mozilla.org/mozilla-central/rev/5a68579c6fbf