920881 - Assertion failure: aIndex < mLength, "aIndex is out of range" in nsSHistory::EvictOutOfRangeWindowContentViewers

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

There are a number of bugs filed on this issue. I can't tell if they're the same or not, so here's a new one. I made some changes that cause the nsSHistory object to be created earlier than it normally is (inside the <xul:browser> XBL binding constructor rather than the window's onLoad handler). Now I'm getting this assertion all over the place. It's very easy to reproduce.

Here's what seems to happen:

1. We create the nsSHistory with mLength == 0.
2. We add an entry so that mLength == 1 and mIndex == 0.
3. Then nsSHistory::PurgeHistory(1) is called. mLength goes back to 0.
   3.1. It calls nsDocShell::HistoryPurged(1), which sets mLoadedTransIndex to 0 here:
        http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#2615
        (It's taking the max of 0 and -1.)
4. We later do EvictOutOfRangeWindowContentViewers(mLoadedTransIndex).
   The callsite is here:
   http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDocumentViewer.cpp#1902
   EvictOutOfRangeWindowContentViewers asserts because mLength == 0 and aIndex == 0.

I don't know enough about this code to understand which step is wrong. Is the assertion bogus? The code seems capable of handling the situation perfectly fine.

Anyway, this is blocking what I'm working on, so I'd like to help sort out the problem if I can.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: rm-disablehistory

Here's the patch that causes the problem. It removes a sort of "optimization" we have and causes the session history to be created in a different place for the first tab in a window.

I can get the assertion to happen by running browser/components/sessionstore/test/browser_465223.js with this command (from my objdir):

./_virtualenv/bin/python ./_tests/testing/mochitest/runtests.py --browser-chrome --test-path=browser/components/sessionstore/test/browser_465223.js --console-level=INFO --autorun

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: rm-assertion

Olli requested the assertion be changed to a warning.

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cea94b22a6a6

Also had to change an annotated assertion that no longer fires.

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/cea94b22a6a6