1225571 - APZCCallbackHelper can make the SPCSPS glitch if it is changed by something else

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

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

With the C++ APZ enabled on Fennec, I see the SPCSPS glitch when the dynamic toolbar shows and hides. For example when hiding the toolbar on my nexus 4 I see values like this:

h=519
h=519
h=567
h=519 <-- glitchy value
h=567
h=567

For the most part the MobileViewportManager is correctly updating the SPCSPS but there is sometimes a repaint request inflight with a stale composition bounds. When the APZCCallbackHelper gets this repaint request it clobbers the SPCSPS with a stale value, before it gets restored on a new repaint request.

In concrete terms this manifests as fixed-pos items and scrollbars doing a glitchy 'jump' because they're using a bad SPCSPS for a frame. This is much more noticeable if the main thread has stalls in it, such as when scrolling up/down on http://people.mozilla.org/~kgupta/stall.html.

Comment 1

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

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9050c4b58593

Comment 2

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

Attachment: MozReview Request: Bug 1225571 - Move the code to update the SPCSPS from repaint request handling to when the resolution is updated in the presShell. r=

Bug 1225571 - Move the code to update the SPCSPS from repaint request handling to when the resolution is updated in the presShell. r=

Comment 3

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

Comment on attachment 8688574 [details]
MozReview Request: Bug 1225571 - Move the code to update the SPCSPS from repaint request handling to when the resolution is updated in the presShell. r=

Actually let me wait until the try push comes back, there's a high chance this will break something.

Comment 4

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

Just to clarify: the problem here is that the repaint request that comes in has a potentially updated resolution and a potentially stale composition bounds. So calling CalculateCompositedSizeInCssPixels might not return the right value. My patch moves the computation to after the resolution is updated on the presshell. This way layout now has the most up-to-date resolution and composition bounds values. It can then compute the correct new value of the SPCSPS.

Comment 5

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

Comment on attachment 8688574 [details]
MozReview Request: Bug 1225571 - Move the code to update the SPCSPS from repaint request handling to when the resolution is updated in the presShell. r=

Try seems to be green enough.

Comment 6

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

Comment on attachment 8688574 [details]
MozReview Request: Bug 1225571 - Move the code to update the SPCSPS from repaint request handling to when the resolution is updated in the presShell. r=

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/25349/diff/1-2/

Comment 7

[Botond Ballo [:botond]]

Comment on attachment 8688574 [details]
MozReview Request: Bug 1225571 - Move the code to update the SPCSPS from repaint request handling to when the resolution is updated in the presShell. r=

https://reviewboard.mozilla.org/r/25349/#review22981

The duplication of code between RefreshViewportSize and RefreshSPCSPS is a bit unfortunate, but I guess trying to share code between them would be a bit messy, so this is fine.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/52d6ee1e55c9

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/52d6ee1e55c9