See testcase, I see this assertion appearing, after the testcase has reloaded a couple of times, in my current debug trunk build: ###!!! ASSERTION: ContentViewer exists outside gHistoryMaxViewer range: '!viewer ', file c:/mozilla-build/mozilla/docshell/shistory/src/nsSHistory.cpp, line 815

Bugzilla doesn't like my testcase, so you should download the testcase to your computer to see the assertion.

running reftests spits them out now very frequently seems like a regression

I spent some time analyzing this while looking for another problem. Here's what I found out. One way to make this bug appear is to: 1) Move around to a couple of pages to make sure that the history contains at least gHistoryMaxViewer viewers. A handful of pages should be enough. (The variable was 3 when I had a look in the debugger, and I got the impression it would never be above 8). 2) Change the current url by updating the part after # in the url in the address field. (Or just add #foo to the url.) 3) Go to yet another page. What happens is that the URL change of adding the #foo does make a new history item, but the code to evict ContentViewer objects is not run. It seems that the history is updated somewhere at the beginning of moving to the new URL, but that the eviction code is run in some callback after the page has loaded (I remember something like OnPageLoaded in the cass stack). So after the new URL is there, there is more than gHistoryMaxViewer before the current history item. This will trigger the assertion the next time that the evict code is run. BTW, this was on today's cvs HEAD.

Bug 396519 should fix the issue described in comment 3, if I can ever manage to land it...

Same on OS X with the following reftests: layout/reftests/bugs/234686-x.html

(gdb) frame 5 #5 0x03637119 in nsSHistory::EvictWindowContentViewers (this=0xded5af0, aFromIndex=33, aToIndex=34) at /Volumes/data/build/minefield/docshell/shistory/src/nsSHistory.cpp:839 839 NS_ASSERTION(!viewer, (gdb) p viewer $9 = { mRawPtr = 0xd53d7c0 } (gdb) p aFromIndex $11 = 33 (gdb) p aToIndex $13 = 34 (gdb) p mLength $10 = 35

Yes, yes. We know why the bug is happening, basically. It's because the bfcache tracking of where it has live entries is completely busted. See comment 4 and the bug linked therein.

With bz's permission, I changed this to an NS_WARN_IF_FALSE so we can make progress on making unexpected assertions be reftest failures. http://hg.mozilla.org/mozilla-central/rev/e12ccfe8c5a7 (Apparently the presence of this assertion is GC-timing-dependent, which makes this one particularly messy.)

So this is fixed then?

No, the underlying bug is still there.... I suppose we could track it via a new bug report, if we copy over the relevant info from this one.

( Ftr, I just confirm comment 2: http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1273840030.1273841301.21770.gz WINNT 5.2 comm-central-trunk debug test crashtest on 2010/05/14 05:27:10 WARNING: ContentViewer exists outside gHistoryMaxViewer range: '!viewer', file .../docshell/shistory/src/nsSHistory.cpp, line 846 )

The assertion was removed in Bug 646641 Part 2: https://hg.mozilla.org/mozilla-central/rev/051b3d8ed545