Closed Bug 420415 Opened 16 years ago Closed 16 years ago

Crash while quitting with dialog from <xul:iframe src="nosuchprotocol:">


(Core :: DOM: Navigation, defect, P1)






(Reporter: jruderman, Assigned: smaug)



(Keywords: assertion, crash, testcase, Whiteboard: [sg:critical?])


(4 files)

About 10% of the time, this assertion failure occurs just before the dialog appears:

###!!! ASSERTION: painting in the middle of frame construction: 'mPresContext->mLayoutPhaseCount[eLayoutPhase_FrameC] == 0', file ../../dist/include/layout/nsPresContext.h, line 931

If I click "OK" in the dialog, nothing bad happens.  But if I press Cmd+Q while the dialog is up, there is a 60% chance of crashing, regardless of whether the assertion failure occurred.  The crash dereferences 0xdddddde9.
Flags: blocking1.9?
Attached file testcase
Attached file stack traces
Uh, this is really bad.  We're putting up a modal dialog in the middle of frame construction.

Luckily, smaug's patch should fix that part.  But then we'd still be putting up a modal dialog during BindToTree.

Can we make that modal dialog async or something?  Or do the load or the DisplayLoadError call async?
Component: Layout → Embedding: Docshell
QA Contact: layout → docshell
Smaug's patch in which bug?
Whiteboard: [sg:critical?]
Works for me, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008022906 Minefield/3.0b4pre
We need to do the same thing for loadFrame as what was done for Destroy.
on the 1.8 branch I see the assertion

###!!! ASSERTION: failed to load URL: 'NS_SUCCEEDED(rv)', file c:/dev/ff2/mozilla/content/base/src/nsFrameLoader.cpp, line 183

and then crash dereferencing 0xdddddde9 in nsCachedStyleData::GetStyleData()
Assignee: nobody → dbaron
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.14?
Depends on: 395609
Have to still test some more. I can't reproduce the crash, but without the patches I did see the assertion.
At least with the patches I can't see the assertion, which should mean
that frame loading starts at a safe time.
I did run also mochitest/browser/chrome, everything looks ok.
Who's reviewing the patch?

+'ing w/P2.
Have to solve the window-event-loop problem in bug 395609 first.
But I think jonas and jst should review, since they reviewed Bug 402982.
And btw, if anyone on mac could test the patch and the latest patch for bug 395609 to see if the crash is fixed ;)

(I *must* get a mac, unfortunately.)
Attachment #307451 - Flags: review?(jst)
Attachment #307451 - Flags: review?(jst) → review+
I'll test this patch once the patch for bug 395609 goes in.
Assignee: dbaron → Olli.Pettay
Attachment #307451 - Flags: superreview?(jonas)
Fixing flags to reflect blocking.  +'ing w/ P2.  
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
Depends on: 423269
Comment on attachment 307451 [details] [diff] [review]
this should fix this (with the patch for 395609)

Ugh, sorry, could have sworn i marked this a while ago. Sorry about the delay.
Attachment #307451 - Flags: superreview?(jonas) → superreview+
Target Milestone: --- → mozilla1.9beta5
TM --> mozilla1.9, this won't block beta 5. If you disagree, please reset the TM to beta5 and explain why it needs to block beta.
Target Milestone: mozilla1.9beta5 → mozilla1.9
Bug 421209, Bug 395609 and this one should, IMO, go in together.
Bug 395609 is by far the most scariest one, but this one may cause regressions 
too. I'd really like to get these all in for beta5.

Bug 421209 blocks Bug 395609, which blocks this one.
Target Milestone: mozilla1.9 → mozilla1.9beta5
Priority: P2 → P1
Attachment #307451 - Flags: approval1.9b5?
Comment on attachment 307451 [details] [diff] [review]
this should fix this (with the patch for 395609)

Attachment #307451 - Flags: approval1.9b5? → approval1.9b5+
Checked in.
Closed: 16 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
This testcase gives me a potentially exploitable crash on branch (comment 7) but later comments say this fix depends on bug 395609 which isn't a problem on the 1.8 branch (bug 395609 comment 81).

Is the branch crash a different bug, or do we still need this chain of fixes even though some of the other testcases are masked on the branch?
Depends on: 431082
Another potential regression found (bug 431082) so we need to wait until we get those sorted out.
Flags: blocking1.8.1.15?
Group: core-security
You need to log in before you can comment on or make changes to this bug.