515 bytes, text/html
22.95 KB, text/plain
11.75 KB, patch
|Details | Diff | Splinter Review|
11.81 KB, patch
|Details | Diff | Splinter Review|
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.
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?
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
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.)
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
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+
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
Comment on attachment 307451 [details] [diff] [review] this should fix this (with the patch for 395609) a=beltzner
Attachment #307451 - Flags: approval1.9b5? → approval1.9b5+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
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?
Another potential regression found (bug 431082) so we need to wait until we get those sorted out.
You need to log in before you can comment on or make changes to this bug.