Closed Bug 1382427 Opened 2 years ago Closed 2 years ago

Support running reftest in a mode than runs the test with Retained Display lists on, and references with it off

Categories

(Core :: Web Painting, enhancement)

Other Branch
All
Unspecified
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: bugs, Assigned: jwatt)

References

Details

(Keywords: perf, Whiteboard: [qf:p2])

Attachments

(1 file, 3 obsolete files)

The Retained DisplayLists feature is a rendering optimization that caches the current display so that later Layout mutations are faster to paint. Our existing reftests are good at validating initial rendering, but not so good at testing Layout mutations with subsequent paints.

The current reftest harness does this:
1. start test
2. wait for stable paint
3. take snapshot for bitmap comparison

We need to modify the reftest harness so that it will test the cases that break when incorrect DisplayList calculations from Layout mutations end up painting the wrong pixels.
Assignee: nobody → tschneider
Blocks what was a qf:p1 bug.
Whiteboard: [qf:p1]
I talked to Matt, he says it's very unlikely for this to land in 57. In line with our triage rules that makes this P2.
Whiteboard: [qf:p1] → [qf:p2]
I though the way the reftest harness works tested at least some of this codepath...
Keywords: perf
Running reftests with the pref reftest.compareRetainedDisplayLists set to true will run them with RDL turned on for the tests and off for the reference files.
Note these patches build on top of the patches from bug 1404181.
Assignee: tschneider → jwatt
(In reply to Jonathan Watt [:jwatt] (needinfo? me) from comment #7)
> Created attachment 8920063 [details] [diff] [review]
> part 3 - WIP fuzzing patch to the reftest harness

This is based on the 10 year old add-on, refdyn, from:

https://bugzilla.mozilla.org/show_bug.cgi?id=373610

refdyn's code is no obsolete I didn't try and resurrect it in its previous add-on form. Instead I integrated the still useful parts into our current reftest harness and modified it with some of the current ideas from domfuzz.
Attachment #8920063 - Attachment is obsolete: true
I've pretty much rewritten the fuzzer code (e.g. to apply and then remove a stack of changes, rather than push and remove one change at a time) and moved it off to bug 1415558.
Let's make this bug just about supporting running reftest in a mode than runs the test with Retained Display lists on, and references with it off.
Summary: Reftest changes to validate retained DisplayLists → Support running reftest in a mode than runs the test with Retained Display lists on, and references with it off
Attached patch patchSplinter Review
Attachment #8917423 - Attachment is obsolete: true
Attachment #8917424 - Attachment is obsolete: true
Attachment #8926424 - Flags: review?(matt.woodrow)
Depends on: 1415485
Comment on attachment 8926424 [details] [diff] [review]
patch

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

Looks good!
Attachment #8926424 - Flags: review?(matt.woodrow) → review+
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2bf7ca0839c
Support running reftests with retained display lists on for the test files only. r=mattwoodrow
Pushed by jwatt@jwatt.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4162a11c0a7e
Support running reftest in a mode that runs the test with Retained Display lists on, and references with it off. r=mattwoodrow
(In reply to Noemi Erli[:noemi_erli] from comment #15)
> Backed out changeset c2bf7ca0839c (bug 1382427) for failing reftests
> r=backout on a CLOSED TREE

I pushed an old patch from the wrong tree with testing changes to modules/libpref/init/all.js to flip the comparison pref on.
Flags: needinfo?(jwatt)
https://hg.mozilla.org/mozilla-central/rev/4162a11c0a7e
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.