244554 - [FIX]Non-displayed invalid frames processed - bypasses recursion check resulting in memory leak / crash

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect, P1)

Description

[Aaron Kaluszka]

If a <frame> occurs after the number of frames specified in its containing
<frameset>, it is still processed even though it is not displayed and ought to
be invalid markup.  The real problem with this is that the recursive frame
checks (bug 8065) do not take place so a recursive frame will cause 100% CPU
usage and unbounded memory increase (until crash).
Unfortunately the Blackboard software at Virginia Tech currently does this
(you'll need an account to see the problem, so I'll attach a minimized testcase).

Comment 1

[Aaron Kaluszka]

Attachment: Reduced Testcase

Comment 2

[Aaron Kaluszka]

By the way, save and load the testcase locally for full effect since the network
can slow down the problem.  Since the testcase is reduced, there is no content
on the page; therefore, the memory leak isn't as bad as it would be on a real
world site.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix the docshell parenting

Processing the frame is right, I think.  But the way we were parenting
docshells was absolutely and totally bogus.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Thanks for the excellent testcase, Aaron!

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 149480 [details] [diff] [review]
Fix the docshell parenting

Hmm, I wonder what kind of reason I had for that code when I originally wrote
the frame loader :-)

r+sr=jst

Comment 6

[Boris Zbarsky [:bzbarsky]]

Checked in.

Comment 7

[Aaron Kaluszka]

Can this get into the branches?  I'd really hate to see this in Firefox 1.0.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 149480 [details] [diff] [review]
Fix the docshell parenting

Yeah, it may make sense to get this on the 1.7 branch.	It's a safe change.

For aviary, it's not clear to me what the review or checkin procedures are, so
ping the firefox folks about it if you want this in Firefox 1.0.

Comment 9

[chris hofmann]

Comment on attachment 149480 [details] [diff] [review]
Fix the docshell parenting

a=chofmann for 1.7

Comment 10

[Bernard Alleysson]

fixed on branch:
2004-06-01 08:34 bzbarsky%mit.edu mozilla/content/base/src/nsFrameLoader.cpp
1.37.2.1

Comment 11

[Michael Lefevre]

for the record, mconnor checked this into the aviary branch (along with a bunch
of other 1.7 branch fixes)