472244 - Reftest layout/reftests/reftest-sanity/zoom-invalidation.html fails on Tinderbox

Status: RESOLVED FIXED

(Testing :: Reftest, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The test passed on my Mac, but it failed on the Tinderbox Mac test machine:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1231205144.1231208392.15342.gz#err0

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

... and now it fails on my machine too. Weird. Actually I think it's random, which is bad, since MozReftestInvalidate was supposed to make it not random.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

OK, I found and fixed the randomness. The problem is that after load has fired, even though we flush reflows and invalidates and repaints there could still be a pending MozAfterPaint event with the full window dimensions in it. That will cause reftest to repaint the entire canvas, so we have to delay firing MozReftestInvalidate until after that MozAfterPaint has been received.

Now to fix the actual zoom-invalidate.html test ...

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

OK, I had this code to round the repaint rectangle edges to "device pixels":

        var left = Math.floor(roundTo(r.left*scale, 0.001))/scale;
        var top = Math.floor(roundTo(r.top*scale, 0.001))/scale;
        var right = Math.ceil(roundTo(r.right*scale, 0.001))/scale;
        var bottom = Math.ceil(roundTo(r.bottom*scale, 0.001))/scale;

Problem is, drawWindow takes integer parameters (in CSS pixels) so this code goes all wrong as XPConnect rounds values. I need to think about what we can do here. Maybe drawWindow's parameters need to be floats...

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Okay, this patch has two parts. The main part is to delay attaching our MozAfterPaint listener and firing MozReftestInvalidate until after any MozAfterPaint triggered by load-event activity has been flushed. The other part is to make the parameters to drawWindow floats; this lets us draw at "device pixel" boundaries even when those don't align with CSS pixel boundaries in the zoomed content.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 355555 [details] [diff] [review]
fix

Need vlad's review on the drawWindow change.

Comment 6

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

Comment on attachment 355555 [details] [diff] [review]
fix

r=dbaron, although it might be clearer to name that function rather than having a 15 line anonymous function inside a function call

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 355555 [details] [diff] [review]
fix

Hey, this'll help mobile as well when doing its zoom stuff.  Though, why floats and not doubles?  Cairo and gfx math is going to be all in doubles.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Because all the other canvas APIs are float

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed 670a3b50dfe0

Comment 10

[Serge Gautherie (:sgautherie)]

This bug won't land on m-1.9.1, right?