1178354 - layout/reftests/w3c-css/submitted/css21/pagination/moz-css21-table-page-break-inside-avoid-4.html fails on R-e10s with APZ enabled

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

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

When APZ is enabled a bunch of reftests fail on Linux R-e10s. tests/layout/reftests/w3c-css/submitted/css21/pagination/moz-css21-table-page-break-inside-avoid-4.html is one of them, and this bug is to track that. See https://treeherder.mozilla.org/#/jobs?repo=try&revision=67ec1b076ad0 for an example push that has this failure.

Comment 1

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

So, in a newer try push I didn't see this reftest fail but I saw a different one (layout/reftests/w3c-css/submitted/css21/pagination/moz-css21-row-page-break-inside-avoid-2.html) fail in the same way. I was able to reproduce it locally.

I ran it with layer texture dumping enabled and the textures didn't show the problem. I then ran it with gfx.xrender.enabled=false (thanks acomminos for that tip!) and stuck a watchpoint on the raw pixel data in the cairo_image_surface_t and reverse-executed in rr, tracing through the copies and so on. It turned out that the white rect was coming from the DrawQuad call at [1] because a layer was getting a scrollId but had an empty displayport and so was hitting the LayerHasCheckerboardingAPZC check.

Tracing backwards from there, the displayport margins are supposed to get set in the before-first-paint listener in TabChild.cpp, but that wasn't getting hit, because the before-first-paint event itself wasn't firing on that document. That in turn was because UnsuppressPainting was early-exiting as the painting was already unsuppressed. In fact, it was never suppressed to begin with, because of [2].

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/composite/ContainerLayerComposite.cpp?rev=91d6e262b662#358
[2] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp?rev=fee45cd9a4d9#1712

Comment 2

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

Attachment: Patch

This patch fixes it for me locally, although I'm not sure if there's any assumptions this violates. I'll do a try push both with and without APZ to make sure it doesn't break anything and fixes the failure on try.

Comment 3

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

Attachment: Patch

Slight update to also catch the case where the timer initialization fails, just in case.

Comment 4

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8638087 [details] [diff] [review]
Patch

This makes sense to me.

Comment 5

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

Try looks green as well: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b3974d7b5c8

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/59d0d20d6337

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/59d0d20d6337