Closed Bug 378682 Opened 18 years ago Closed 18 years ago

Crash [@ nsPresContext::GetContainerInternal] when removing window on focus and reloading


(Core :: Layout, defect)

Windows XP
Not set





(Reporter: martijn.martijn, Assigned: MatsPalmgren_bugz)


(5 keywords, Whiteboard: [sg:critical?] using freed pres shell)

Crash Data


(4 files, 1 obsolete file)

Attached file testcase
See testcase, which crashes Mozilla within 200ms after load.
It also crashes branch builds, so I'm marking it security sensitive for now (please  open up, if not necessary).

Talkback ID: TB31509967G
nsPresContext::GetContainerInternal  [mozilla/layout/base/nsprescontext.cpp, line 1117]
PresShell::UnsuppressAndInvalidate  [mozilla/layout/base/nspresshell.cpp, line 4412]
PresShell::UnsuppressPainting  [mozilla/layout/base/nspresshell.cpp, line 4457]

The iframe consists of this code:
setTimeout(doe, 200);
function doe() {
function doe2() {
window.addEventListener('focus', function(e) {window.frameElement.parentNode.removeChild(window.frameElement) }, true);
setTimeout(doe2, 50);
I can't get trunk to crash, nor branch.
Assignee: nobody → mats.palmgren
Whiteboard: [sg:critical?] using freed pres shell
Attached file stack + tracing
DocumentViewerImpl::LoadComplete calls mPresShell->UnsuppressPainting()
which ends up destroying the pres shell, doc viewer, doc shell...,1013#1013

The pres shell code expects the caller to hold a strong ref, which
'mPresShell' is, but DocumentViewerImpl::Destroy nulls it...,1606#1564

This attachment shows two stack traces, the first is the stack
when DocumentViewerImpl::Destroy() is called while we are in
DocumentViewerImpl::LoadComplete().  I was expecting to see
LoadComplete() on the stack but the Visual debugger seems confused
by the JS related stack frames? The second stack is the crash. 
I have some trace printfs in there as well.
Attached patch wip1 (obsolete) — Splinter Review
Adding a strong ref on the stack fixes it for me.
Need to review all other pres shell calls from this file...
nsCOMPtr, please!
Attached patch Patch rev. 1Splinter Review
Changed to using nsCOMPtr.  I found one more call that I'm vaguely
suspicious about, mPresShell->HidePopups(), so I'm protecting that one
as well just in case.
Attachment #262837 - Attachment is obsolete: true
Attachment #262886 - Flags: superreview?(bzbarsky)
Attachment #262886 - Flags: review?(bzbarsky)
Comment on attachment 262886 [details] [diff] [review]
Patch rev. 1

Looks ok.  We probably want this on branch too, esp. once the HidePopups thing lands there.
Attachment #262886 - Flags: superreview?(bzbarsky)
Attachment #262886 - Flags: superreview+
Attachment #262886 - Flags: review?(bzbarsky)
Attachment #262886 - Flags: review+
Checked in to trunk at 2007-04-27 04:06	PDT.

Closed: 18 years ago
Flags: in-testsuite?
Flags: blocking1.8.1.5?
Flags: blocking1.8.0.13?
Resolution: --- → FIXED
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a4pre) Gecko/20070427 Minefield/3.0a4pre

I'm seeing a small issue, though, after the iframe has disappeared, the browser stays in the "loading" state. I suspect that is an already filed bug, but I haven't found one. Should I file a new bug on that?
Yeah I saw that too, the testcase in this bug is changing 'location' of
a nested iframe, which I think is handled by bug 143398.
> the browser stays in the "loading" state.

Worksforme in both Seamonkey and Firefox.  As does bug 143398...
Bug 143398 is indeed also worksforme, however the testcase in this bug is not worksforme, so I've filed it as bug 379206 (made it security sensitive, which sucks).
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: blocking1.8.1.5?
Flags: blocking1.8.1.5+
Flags: blocking1.8.0.13?
Flags: blocking1.8.0.13+
branch is still crashing on a deleted object. Can this patch land there or do we need a branch-specific patch?
Same thing, but one place less to fix because of bug 374570:
I have verified that this patch fixed the crash in a 1.8 Windows build,
although the throbber continues to spin as Martijn noted (bug 379206).
Attachment #271603 - Flags: approval1.8.1.5?
Attachment #271603 - Flags: approval1.8.0.13?
Comment on attachment 271603 [details] [diff] [review]
Patch rev. 1 -- branch version

approved for and, a=dveditz for release-drivers
Attachment #271603 - Flags: approval1.8.1.5?
Attachment #271603 - Flags: approval1.8.1.5+
Attachment #271603 - Flags: approval1.8.0.13?
Attachment #271603 - Flags: approval1.8.0.13+
mozilla/layout/base/nsDocumentViewer.cpp 	1.442.4.20 

mozilla/layout/base/nsDocumentViewer.cpp 	1.442. 
I was not able to reproduce the crash on Tbird 15012(pre fix) nor 15013. 
This does not reproduce in Thunderbird (2007080918) using
Thunderbrowse. It also doesn't reproduce in Thunderbird though.
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv: Gecko/20070822 BonEcho/ ID:2007082203

no crash on testcase - adding verified keyword
also verified using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv: Gecko/20070822 Firefox/ - no crash on testcase

Adding verified keyword 
Group: security
re-adding fixed1.8.1.5 to record when the actual fix happened, even though verification was later due to firedrills.
Keywords: fixed1.8.1.5
Crash Signature: [@ nsPresContext::GetContainerInternal]
You need to log in before you can comment on or make changes to this bug.