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

Status: RESOLVED FIXED

(Core :: Web Painting, enhancement)

Description

[Jet Villegas (inactive)]

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.

Comment 1

[Andrew Overholt [:overholt]]

Blocks what was a qf:p1 bug.

Comment 2

[Bas Schouten (:bas.schouten)]

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.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I though the way the reftest harness works tested at least some of this codepath...

Comment 4

[Jonathan Watt [:jwatt]]

Attachment: part 1 - Support running reftests with retained display lists on for the test files only

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.

Comment 5

[Jonathan Watt [:jwatt]]

Attachment: part 2 WIP - support `./mach reftest-rdl` to make it easier to run RDL-tests vs non-RDL-refs

Comment 6

[Jonathan Watt [:jwatt]]

Note these patches build on top of the patches from bug 1404181.

Comment 7

[Jonathan Watt [:jwatt]]

Attachment: part 3 - WIP fuzzing patch to the reftest harness

Comment 8

[Jonathan Watt [:jwatt]]

Try run with just part 1, running tests with RDL enabled, references with it off:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e902775b6126f9212aa73bb8cfcbabd665d73911

Try run with just part 2, fuzzing reftests without RDL enabled:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=53eb85b8c1adbc744241c1a917dafd1e437f0edb

Try run with both parts 1 and 2:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=898eb728fa6d88ab4801e6f10d3d47b0f88bde66

Comment 9

[Jonathan Watt [: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.

Comment 10

[Jonathan Watt [:jwatt]]

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.

Comment 11

[Jonathan Watt [:jwatt]]

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.

Comment 12

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 13

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8926424 [details] [diff] [review]
patch

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

Looks good!

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2bf7ca0839c
Support running reftests with retained display lists on for the test files only. r=mattwoodrow

Comment 15

[Noemi Erli[:noemi_erli]]

Backed out changeset c2bf7ca0839c (bug 1382427) for failing reftests r=backout on a CLOSED TREE

https://hg.mozilla.org/integration/mozilla-inbound/rev/907fb4683dfef81fbb746035939ac8154b66dfe5

https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=c2bf7ca0839c7be4964557d10eab2b198004c803&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception

Comment 16

[Pulsebot]

Pushed by [e-mail]:
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

Comment 17

[Jonathan Watt [:jwatt]]

(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.

Comment 18

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/4162a11c0a7e