It looks like bug 617152 changed the behavior of the reftest harness such that FlushRendering is not always called. Previously, OnDocumentLoad would invoke StartWaitingForTestEnd via setTimeout, and StartWaitingForTestEnd would call FlushRendering. In this revision, that changed: http://hg.mozilla.org/mozilla-central/rev/19f780daecd0 Now we don't always end up calling WaitForTestEnd, because AfterOnLoadScripts takes an initial snapshot and sometimes decides that that initial snapshot is good enough. See here for the full criteria: http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/reftest-content.js#606 This change looks like it could be causing intermittent oranges. In particular, it seems likely that it's responsible for bug 942004. ImageDocument calls SetWidth and SetHeight on an image element in a load event handler, and the reftest harness doesn't always wait for the consequent style flush and eventual layout flush before taking its snapshot. Ensuring that we always call FlushRendering should be enough to fix this problem. An alternate fix would be to always call WaitForTestEnd, but that seemed a bit more heavyweight so I didn't do things that way for the initial version of the patch.

This patches ensures that FlushRendering is always called before the reftest harness takes its initial snapshot. To make FlushRendering available at that point in the code, the patch also floats it to the top level.

Try job here: https://tbpl.mozilla.org/?tree=Try&rev=481350a2b021

Pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/4cb7f834871a

Comment on attachment 8438042 [details] [diff] [review] Always call FlushRendering in the reftest harness [Approval Request Comment] Bug caused by (feature/regressing bug #): 617152 User impact if declined: None. This is a testing issue. Our sheriffs will appreciate having fewer intermittent oranges, though. Testing completed (on m-c, etc.): Landed last week with no issues since. Risk to taking this patch (and alternatives if risky): Zero risk, since this patch only affects testing. String or IDL/UUID changes made by this patch: None.

Whoops, removing that request, since it looks like this patch itself caused a new test failure. We need a followup for that before backporting.

This patch is intended to fix bug 1025042. It seems like a fairly brute force way to avoid the problem, but it seems to me that: (1) It's OK if the documentElement doesn't exist (probably because the page is already being torn down, I imagine). There isn't anything better we could do in such a case. (2) In general (as far as I know), a child frame could have a missing document element, so it's not clear that we could just insert a check at the FlushRendering callsite. roc, if you think another approach is better let me know.

Try job: https://tbpl.mozilla.org/?tree=Try&rev=36ba19c0e1bc

FYI, patches that only touch tests or the test harness don't require approval for uplift :). I'll make sure this gets everywhere it can reasonably go once it sticks :)

(In reply to Seth Fowler [:seth] from comment #8) > https://tbpl.mozilla.org/?tree=Try&rev=36ba19c0e1bc Given that bug 1025042 hit on the B2G emulator, I would have thought this would have had such a run. I've gone ahead and triggered an emulator build for you :)

Everything looks good on Try, so I went ahead and pushed it. https://hg.mozilla.org/integration/mozilla-inbound/rev/8cf3aebcaeeb

https://hg.mozilla.org/releases/mozilla-aurora/rev/866eb4e421b2 https://hg.mozilla.org/releases/mozilla-beta/rev/423cd6386bd2 https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/4038383d5d6a https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/e1bb6e9a4853 https://hg.mozilla.org/releases/mozilla-esr24/rev/3d5d61210d6b

(In reply to Ryan VanderMeulen [:RyanVM UTC-4][PTO June 19-22] from comment #10) > Given that bug 1025042 hit on the B2G emulator, I would have thought this > would have had such a run. I've gone ahead and triggered an emulator build > for you :) Heh, yes, that would have been a good idea. That's what I get for pushing in a hurry. Thanks for triggering the emulator run, and for the uplift!