1416291 - Disappearing images/flickering on Yelp carousel

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[Asif Youssuff]

Attachment: Screenshot at 2017-11-10 12-41-22.png

Steps to reproduce:

1. Create a new profile. Enable layout.display-list.retain. Restart
2. Navigate to a Yelp listing with photos, eg: https://www.yelp.com/biz/los-tacos-no-1-new-york
3. Hover over photos in photo carousel



Actual results:

Images disappear/reappear and flicker. 


Expected results:

This should work the way it does when retained display lists are not enabled.

Comment 1

[Matt Woodrow (:mattwoodrow)]

Attachment: Don't do partial building for preserve-3d

We already mark all preserve-3d frames for display (MarkPreserve3DFramesForDisplayList), but attempting to save and restore the preserve-3d was messing us up.

ComputeRebuildRegion computes the dirty rect for the middle transformed frame, and store that on its stacking context (the outer transformed frame), and sets MarkFrameForDisplayIfVisible on all ancestors.

When we build, we reach the outer frame with an empty dirty region (but we carry on since we have MarkFrameForDisplayIfVisible). We store that empty dirty region with SavePreserve3DRects.

We reach the middle transformed frame and load our stored dirty rect from ComputeRebuildRegion.

We reach the leaf transformed frame and load the cached preserve-3d rect (empty), and building stops there. The background color item never gets built, and the test fails.

This just makes sure we rebuild everything within preserve-3d if we touch it at all.

Comment 2

[Miko Mynttinen]

Comment on attachment 8928028 [details] [diff] [review]
Don't do partial building for preserve-3d

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

LGTM.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/609829a7c75e
Don't do partial display list building within preserve-3d contexts. r=miko

Comment 4

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/609829a7c75e

Comment 5

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8928028 [details] [diff] [review]
Don't do partial building for preserve-3d

Approval Request Comment
[Feature/Bug causing the regression]: bug 1352499. This is code that is preffed off, but we want to run a shield study enabling the pref.
[User impact if declined]: None, preffed off code.
[Is this code covered by automated tests?]: Yes, when the pref is enabled.
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Code is preffed off.
[String changes made/needed]: None

Comment 6

[Gerry Chang [:gchang]]

Comment on attachment 8928028 [details] [diff] [review]
Don't do partial building for preserve-3d

Take this to support the shield study in 58. Beta58+.

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/680b66df3875

Comment 8

[Cornel Ionce [:noni] [Hubs QA]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #5)
> [Is this code covered by automated tests?]: Yes, when the pref is enabled.
> [Needs manual test from QE? If yes, steps to reproduce]: No

Marking as qe-verify- since it does not require manual testing and it has automated coverage.