Closed Bug 366320 Opened 14 years ago Closed 14 years ago

[reflow branch] Crash [@ PresShell::ProcessReflowCommands] with unminimised testcase that uses a lot of iframes

Categories

(Core :: Layout, defect)

defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: martijn.martijn, Assigned: mats)

References

Details

(Keywords: crash, regression, Whiteboard: post 1.8-branch)

Crash Data

Attachments

(1 file, 1 obsolete file)

Attached file testcase
See testcase. Download the testcase to your computer, unzip it, then open the file named 'iframecrash1.htm'.
I usually crash within 10 seconds after opening. If it hasn't crashed after 30 seconds, try a reload.

I'm pretty sure this is a regression, I'm not seeing this with the latest 1.8.x build.
I've tried to come up with a regression range, and I think (not completely sure, though) it regressed between 2006-12-07 and 2006-12-08:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-01-07+04&maxdate=2008-01-08+06&cvsroot=%2Fcvsroot

Talkback ID: TB28146788W
0x00000000
PresShell::ProcessReflowCommands  [mozilla\layout\base\nspresshell.cpp, line 6179]
PresShell::FlushPendingNotifications  [mozilla\layout\base\nspresshell.cpp, line 4799]
nsDocument::FlushPendingNotifications  [mozilla\content\base\src\nsdocument.cpp, line 4424]
nsHTMLDocument::FlushPendingNotifications  [mozilla\content\html\document\src\nshtmldocument.cpp, line 1296]
nsDocShell::GetPositionAndSize  [mozilla\docshell\base\nsdocshell.cpp, line 3564]
nsDocShell::SetupNewViewer  [mozilla\docshell\base\nsdocshell.cpp, line 6019]

If desired, I can try and come up with a more minimal testcase (which will probably be a painful process, because it doesn't seem to crash in a consistent way).

I'm filing this as security sensitive for the moment, because the testcase uses all kinds of 'evil' code.
Patch coming up...
Assignee: nobody → mats.palmgren
OS: Windows XP → All
Hardware: PC → All
When we hit the "wasDirty lied" assertion we will append the frame again
so we have two or more entries in mDirtyRoots for 'f'. If the frame is
destroyed while there is more than one entry we will have entries for
the destroyed frame in mDirtyRoots because PresShell::NotifyDestroyingFrame
only removes one entry.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/base/nsPresShell.cpp&rev=3.954&root=/cvsroot&mark=3488-3491#3479
Summary: Crash [@ PresShell::ProcessReflowCommands] with unminimised testcase that uses a lot of iframes → [reflow branch] Crash [@ PresShell::ProcessReflowCommands] with unminimised testcase that uses a lot of iframes
Attached patch Patch rev. 1 (obsolete) — Splinter Review
This patch assumes we only want one entry per frame in mDirtyRoots.
An alternative would be to allow that and make NotifyDestroyingFrame
handle multiple entries instead.
Attachment #250903 - Flags: superreview?(dbaron)
Attachment #250903 - Flags: review?(dbaron)
I suppose we should probably do this; I think the main cause of this assertion is some stuff the scrollbars do in the middle of reflow.

That said, you don't need the IndexOf call; just do the RemoveElement loop, and put the assertion in the loop body.
Depends on: 363253
Attached patch Patch rev. 2Splinter Review
Attachment #250903 - Attachment is obsolete: true
Attachment #254531 - Flags: superreview?(dbaron)
Attachment #254531 - Flags: review?(dbaron)
Attachment #250903 - Flags: superreview?(dbaron)
Attachment #250903 - Flags: review?(dbaron)
Comment on attachment 254531 [details] [diff] [review]
Patch rev. 2

Probably nice to have braces around the loop body, in case somebody makes the macro not-quite-well-behaved.  r+sr=dbaron
Attachment #254531 - Flags: superreview?(dbaron)
Attachment #254531 - Flags: superreview+
Attachment #254531 - Flags: review?(dbaron)
Attachment #254531 - Flags: review+
Nit fixed.
Checked in to trunk at 2007-02-22 18:29 PST.

-> FIXED
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070228 Minefield/3.0a3pre
(the testcase tends to hang Firefox for quite a time, though)
Status: RESOLVED → VERIFIED
Verified not to affect the 1.8 branch (Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.4pre) Gecko/20070322 BonEcho/2.0.0.4pre).
Whiteboard: post 1.8-branch
Group: security
Flags: in-testsuite?
Crash Signature: [@ PresShell::ProcessReflowCommands]
You need to log in before you can comment on or make changes to this bug.