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

RESOLVED FIXED in mozilla27

Status

()

Core
Document Navigation
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: billm, Assigned: billm)

Tracking

unspecified
mozilla27
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

4 years ago
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.
(Assignee)

Comment 1

4 years ago
Created attachment 811345 [details] [diff] [review]
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
(Assignee)

Comment 2

4 years ago
Created attachment 811348 [details] [diff] [review]
rm-assertion

Olli requested the assertion be changed to a warning.
Assignee: nobody → wmccloskey
Status: NEW → ASSIGNED
Attachment #811348 - Flags: review?(bugs)
Attachment #811348 - Flags: review?(bugs) → review+
(Assignee)

Comment 3

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

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

Comment 4

4 years ago
https://hg.mozilla.org/mozilla-central/rev/cea94b22a6a6
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.