Closed Bug 436990 Opened 12 years ago Closed 11 years ago

[FIX]"ASSERTION: RefreshURIList timer callbacks should only be RefreshTimer objects" with hash change and meta-refresh

Categories

(Core :: DOM: Navigation, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: bzbarsky)

References

Details

(Keywords: assertion, testcase)

Attachments

(4 files)

Steps to reproduce:
1. Load i.html#foo (the hash part is important)
2. Wait at least one second.
3. Leave the testcase (reload, navigate to another page, or close the tab).

Result:

###!!! ASSERTION: RefreshURIList timer callbacks should only be RefreshTimer objects: 'rt', file /Users/jruderman/central/docshell/base/nsDocShell.cpp, line 4790
Attached patch FixSplinter Review
This is kind of fun.  The issue is that when a timer fires it now nulls out its callback.  But nsRefreshTimer::Notify doesn't remove the timer from the refresh list, so when we go to stop all the timers later we have a timer with a null callback and assert.

The solution is to just remove the timer when it fires, since otherwise we're sort of leaking the timer object for the lifetime of the page.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #336385 - Flags: superreview?(jst)
Attachment #336385 - Flags: review?(jst)
Jesse, do you think you can make a crashtest out of this?
Summary: "ASSERTION: RefreshURIList timer callbacks should only be RefreshTimer objects" with hash change and meta-refresh → [FIX]"ASSERTION: RefreshURIList timer callbacks should only be RefreshTimer objects" with hash change and meta-refresh
Attachment #336385 - Flags: superreview?(jst)
Attachment #336385 - Flags: superreview+
Attachment #336385 - Flags: review?(jst)
Attachment #336385 - Flags: review+
Attached patch crashtestSplinter Review
I tested with both this directory's crashtest.list and the complete list; it triggers the assertion in both cases :)
Pushed changeset 684f3d78f65a.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Duplicate of this bug: 458283
The crashtest for this bug still seems to cause some issues. See bug 566159.
You need to log in before you can comment on or make changes to this bug.