1604270 - Make waitForApzFlushedRepaints() more robust (or audit its existing uses for problematic usage)

Status: NEW

(Core :: Panning and Zooming, task, P3)

Description

[Botond Ballo [:botond]]

From bug 1601568 comment 18:

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

I do feel like I've gotten a better understanding of waitForApzFlushedRepaints as a result of debugging this, though: it is not sufficient to simply synthesize a bunch of native events, call waitForApzFlushedRepaints, and then assert that scrolling has taken place. For waitForApzFlushedRepaints to have the desired effect, at the time it's called the paint that will carry the layers-dependent target confirmation needs to have been scheduled already (which generally means, the events that trigger said confirmation need to have arrived at content).

The pointer-events test in question does this correctly (it waits for the drag gesture's touchend before calling waitForApzFlushedRepaints), but this is useful to keep in mind for other tests.

Should we file a bug to improve this situation in the future? Either audit test usage to make sure it's okay and add a comment about how to use waitForApzFlushedRepaints or extend/modify waitForApzFlushedRepaints to avoid this problem?

Comment 1

[Botond Ballo [:botond]]

Expanding on this a bit:

What waitForApzFlushedRepaints does is:

• waits for any pending content paints (transactions) to be flushed
• waits for any repaint requests that the transactions and associated messages (notably, the SetTargetAPZC confirmation) may have triggered to be flushed
• wait for any additional content paints that those repaint requests may have triggered, to be flushed

When we synthesize native events, if they are targeting an inactive scroll frame, the following potentially needs to happen:

1. The native events arrive at APZ
2. APZ enqueues the events into its InputQueue and sends a copy to content
3. Content receives the events, layerizes the scroll frame, and schedules a paint and a layers-dependent SetTargetAPZC confirmation
4. The paint happens and the SetTargetAPZC confirmation is sent
5. APZ receives the SetTargetAPZC confirmation and processes the events in the InputQueue, triggering APZ scrolling
6. The APZ scrolling triggers a repaint request to content with the new scroll offset
7. Content receives the repaint request and updates its scroll offset

So waitForApzFlushedRepaints is useful for waiting until step 7 is done, if step 3 is done at the time it's called. However, if a test is just synthesizing native events, and calls waitForApzFlushedRepaints right after, step 3 may not have necessarily happened yet, and we don't wait long enough.

So, two possible solutions here are:

• Document this potential caveat, and audit call sites of waitForApzFlushedRepaints to make sure they're using it properly (i.e. in relevant situations, waiting for step 3 to happen by some other means, such as registering a listener for a touchend event); OR
• Modify waitForApzFlushedRepaints so that it waits for the entire sequence (steps 1-7 above)