Martijn Wargers (dead) Reporter
	Description • 17 years ago

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

	Martijn Wargers (dead) Reporter
	Comment 1 • 17 years ago

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

	Bernd
	Comment 2 • 17 years ago

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

	Staffan Ulfberg
	Comment 3 • 16 years ago

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.

	Boris Zbarsky [:bzbarsky]
	Comment 4 • 16 years ago

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

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 5 • 16 years ago

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

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 7 • 15 years ago

(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

	Boris Zbarsky [:bzbarsky]
	Comment 8 • 15 years ago

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.

	David Baron :dbaron:
	Comment 9 • 15 years ago

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.)

	Martijn Wargers (dead) Reporter
	Comment 10 • 15 years ago

So this is fixed then?

	Boris Zbarsky [:bzbarsky]
	Comment 11 • 15 years ago

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.

	Serge Gautherie (:sgautherie)
	Comment 12 • 14 years ago

(
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
)

	:Cykesiopka
	Comment 13 • 11 years ago

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