http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1282397897.1282398760.29607.gz s: talos-r3-fed64-030 3440 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/src/threads/test/test_suspend.html | Wrong url! - got http://mochi.test:8888/tests/dom/src/threads/test/suspend_iframe.html, expected "about:blank" 3441 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/src/threads/test/test_suspend.html | Already suspended? - got true, expected false 3442 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/src/threads/test/test_suspend.html | Already resumed? - got true, expected false repeated a few hundred times.

Hit this on the 4.0b5 release tests today: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Release/1283266349.1283267231.18092.gz

We also hit this several times a day on various non-release trees, but I hadn't noticed that tbpl was failing to actually comment when it's starred.

With my patch from bug 598482 I hit this failure on all platforms consistently. The problem was that calling iframe.history.back() didn't restore test_suspend.html from bfcache, but reloaded it instead. (badOnloadCallback didn't catch the reload because it wasn't attached as a load handler correctly.) Why didn't test_suspend.html come from the bfcache? Because an attribute change had kicked it out. This attribute change was caused by a delayed reflow flush. Without the patch from bug 598482, anything that causes invalidations causes reflows to be flushed pretty much instantly (one trip through the event loop). But with that patch, they are flushed from the refresh driver timer, and that can be too late. When is "too late"? When the new document is shown, because that's when the content viewer of the previous document is stored on the session history entry. Any change to the stored document after that point will cause the entry to drop the cached content viewer. (How can a reflow change attributes? For example through nsGfxScrollFrameInner::FinishReflowForScrollbar...)

Maybe nsDocumentViewer should do the flush before it adds itself to the session history entry. Boris, do you think that's a good idea?

Comment on attachment 493291 [details] [diff] [review] add a flush Neat! Thanks for digging in to this!

> Maybe nsDocumentViewer should do the flush before it adds itself No, that seems like a terrible idea; it introduces another place script can run during the unload sequence, and does work we don't really need to do in the common case of not coming back to the page. Are we really hitting a reflow from the refresh driver? The refresh driver should be suspended while the page is in bfcache... See nsRefreshDriver::Freeze. Is that not working? Also, perhaps we should special-case scrollbar attr changes in the shentry mutation observers and not drop bfcache for those.