Closed Bug 404693 Opened 12 years ago Closed 6 years ago

ASSERTION: ContentViewer exists outside gHistoryMaxViewer range, with onbeforeunload, document.write and removing window

Categories

(Core :: DOM: Navigation, defect)

defect
Not set

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: martijn.martijn, Unassigned)

References

(Depends on 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(2 files)

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.
Summary: ContentViewer exists outside gHistoryMaxViewer range, with onbeforeunload, document.write and removing window → ASSERTION: ContentViewer exists outside gHistoryMaxViewer range, with onbeforeunload, document.write and removing window
running reftests spits them out now very frequently seems like a regression
Component: History: Session → Document Navigation
QA Contact: history.session → docshell
Blocks: 385276
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...
Depends on: 396519
Same on OS X with the following reftests: layout/reftests/bugs/234686-x.html
OS: Windows XP → All
Hardware: x86 → All
(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.
Attachment #289602 - Attachment description: testcase → testcase (save and test locally)
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
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.