1109263 - Enabling event regions triggers invalidation changes, causes spurious reftest failures

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

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

Attachment: Exhibits

+++ This bug was initially created as a clone of Bug #1107280 +++

This is a spinoff from bug 1107280 to specifically cover the B2G failure in layout/reftests/backgrounds/background-repeat-large-area.html when event regions are enabled. I suspect (but have not verified) that the reftest failures in layout/reftests/css-gradients/ are also the same issue as the method of failure and test pages are very similar.

After investigating this I think what's happening is as follows:

When event regions are enabled, the display list and layer tree assignments change from "exhibit A" (see attachment) to "exhibit B" when the page is scrolled. This results in some items (the CanvasBackgroundColor and CanvasBackgroundImage) moving from one layer to another and triggers the code at [1] to add these items to the invalid region for the layer. This invalid region is 100000 pixels tall, because that's how tall the page is.

This invalid region then gets added included as part of the invalid region at [2] which is called from [3]. Basically, this area gets added to the rects in the MozAfterPaint event that is fired into the reftest harness.

The reftest harness then tries to call drawWindow using this area, which fails because of the check at [4]. The reftest snapshot of the page is never updated from the initial rendering, and that causes the reftest to fail.

Based on my understanding there are possibly two issues here. One is that turning on event regions changes the layerization of items. However that modified layerization may or may not be a bug, I'm not sure. It might just be expected behaviour. The other issue is that the reftest harness fails the test even though visually the test passes, because it tries to snapshot an area that is too large. This seems like a bug in the harness. Even in passing runs I intermittently see NS_ERROR_FAILURE returned from the drawWindow call and it wouldn't surprise me if this caused intermittent failures elsewhere in the tree.

I'll try to write a patch for the harness but I'll need somebody else to tell me whether or not the changed layerization is correct or not.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp?rev=923c99af6863#3105
[2] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/LayerTreeInvalidation.cpp?rev=7f9005cad6e0#158
[3] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDisplayList.cpp?rev=89858cf28204#1550
[4] http://mxr.mozilla.org/mozilla-central/source/dom/canvas/CanvasRenderingContext2D.cpp?rev=53fb431ea943#4533

Comment 1

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

Attachment: Patch for reftest harness

I verified that this fixes the problem for me locally. It also fixes the reftest failures in layout/reftests/css-gradients/

Try push including this change is at https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=689c97857594

Comment 2

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/1dc0c8dd17f5

Comment 3

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

Note to sheriffs: this may make the R-e10s perma-fail on Linux via TEST-UNEXPECTED-PASS on large-gradient-4.html (and large-gradient-3.html). In other words, this patch may "fix" the intermittent-ness of bug 1097289 and allow the test to be flagged as passing. In the few logs I looked at the large-gradient-3.html and large-gradient-4.html tests have the NS_ERROR_FAILURE which probably explains why they're failing, and should be fixed by this patch.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1dc0c8dd17f5