610571 - Leak with crazy iframe testcase

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Steps:
1. Run with XPCOM_MEM_LEAK_LOG=2
2. Load the testcase.
3. Quit.

Result:
* trace-refcnt reports leaks (8 nsGlobalWindow, 19 nsDocument, 2 nsDocShell)

Comment 1

[Johnny Stenback (:jst)]

Peter, this may be worth looking into as you're debugging similar leaks. Not sure this is strictly a blocker, but blocking for now.

Comment 2

[Mounir Lamouri (:mounir)]

The test case seems to leak in 1.9.1 and 1.9.2 too.

Comment 3

[Johnny Stenback (:jst)]

Mounir is looking into this, reassigning.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Attachment: an assertion

The testcase causes this assertion to fire.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Don't load if the owner document isn't active

We probably should do this, though we should perhaps also
traverse docshell's mScriptGlobal and mContentViewer
(and make DocumentViewerImpl ccollectable).

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 502221 [details] [diff] [review]
Don't load if the owner document isn't active

Boris, what do you think about this.
We sure shouldn't try to load the frame if ownerDoc
has already been removed from its docshell.

Comment 7

[Mounir Lamouri (:mounir)]

Attachment: WIP Patch

So, this patch should correspond to what Olli suggests in comment 5 but it's crashing :(
Whatever the reason of this crash is, I guess we should stay with Olli's patch for Firefox 4 given that this is fixing the issue and this patch is changing nsIDocShell uuid.

Comment 8

[Mounir Lamouri (:mounir)]

Assigning the bug to Olli given that he has a working and simple patch.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 502221 [details] [diff] [review]
Don't load if the owner document isn't active

This looks fine to me, but the comment needs changing a bit.  r=me with that.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Oh, and can you add a test?

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Attachment: +comment

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

I hope I can use Jesse's test as a chashtest

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

crashtest

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Attachment: +test

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/cee083078957