Running mochitest-chrome today, I noticed a couple of windows from the tests for bug 113934 hanging around until the end of the test suite. Handily, I had an older tree lying around to confirm this was new. Same when running the test in isolation. We're now only running 3 tests out of 13, before hitting an uncaught JS exception - an NS_ERROR_FAILURE bubbling up from the call to compareCanvases in compareSnapshots. hg bisect points to http://hg.mozilla.org/mozilla-central/rev/39bd0f6314c3 from bug 552605. This isn't just a local problem, the slaves running m-o on tinderbox are hitting this too (at least linux debug, not checked opt or other platforms). Build logs before/after the push: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1309594430.1309598240.28946.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1309600613.1309605277.30375.gz For the record, compareCanvases bales when checking the image sizes are equal, but it's presumably the change to WindowSnapshot in the cset above that's done the damage

And of course someone made exceptions non-fatal in chrome tests. :( Joe, can you take a look at this please?

Oh, I see, we used to never actually take the gWindowUtils codepath? Is there a reason that throws on a size mismatch instead of just returning false? Graeme, does making that change fix things here?

Oh, and I requested tracking because we're missing part of our test suite now, unless either this bug is fixed or the change to WindowSnapshot is backed out.

> Oh, I see, we used to never actually take the gWindowUtils codepath? Yes, using an older tree, for this test at least, the comparison was only on the data:// URLs. > Is there a reason that throws on a size mismatch instead of just returning > false? Looking at bug 387132, compareCanvases returns the number of differing pixels rather than true/false, so differing sizes would make that number meaningless.

OK. Does changing WindowSnapshot to check the imagedata sizes itself on that codepath and treat the result as "not equal" fix things? I can post a patch for that tomorrow if you'd prefer.

These are the actual values being compared: (gdb) p (img1.mRawPtr)->GetSize() $5 = { <mozilla::gfx::BaseSize<int,nsIntSize>> = { width = 1606389496, height = 32767 }, <No data fields>} (gdb) p (img2.mRawPtr)->GetSize() $6 = { <mozilla::gfx::BaseSize<int,nsIntSize>> = { width = 1606389496, height = 32767 }, <No data fields>} (gdb) p (img1.mRawPtr)->Stride() $7 = 1200 (gdb) p (img2.mRawPtr)->Stride() $8 = 120 Are they sensible?

The 1200 and 120 look correct. The GetSize results there look moderately bogus to me.

Is there anything I can do for this?

Sure, review the patch!

Comment on attachment 545370 [details] [diff] [review] Fix It feels weird to be calling ok() from this function, but as long as it works I'm happy.

Actually, we have no test to make sure this doesn't regress... that's what the "in-testsuite?" is about.

qa- as no QA fix verification necessary

Bug 652494 covers fixing the test suite. Removing this from the list of bugs that need tests created.