Closed Bug 42895 Opened 24 years ago Closed 24 years ago

event handling on inputs leaks containing XUL document

Categories

(Core :: DOM: Editor, defect, P3)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: waterson, Assigned: dbaron)

References

Details

(Keywords: memory-leak, Whiteboard: [nsbeta2+])

Attachments

(1 file)

To reproduce, turn on your bloat log,

1. Start mozilla
2. Move the mouse over the URL bar
3. Close the window

You'll see about 700-800Kb of leaks. If you type in the URL bar, this goes up
over 1Mb.

dbaron and I looked at this for a few hours today. It turns out that ender lite
is not cleaning up its script objects. It needs to SetDocument(nsnull) on the
anonymous content, we think.

hyatt suggested that this should just be re-written in XBL, which will
automagically take care of this.
Also, I tried a stupid hack in nsGfxTextControlFrame2::Destroy() that didn't
work:

cvs server: Diffing .
Index: nsGfxTextControlFrame2.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/forms/src/nsGfxTextControlFrame2.cpp,v
retrieving revision 1.37
diff -u -r1.37 nsGfxTextControlFrame2.cpp
--- nsGfxTextControlFrame2.cpp	2000/06/16 06:09:33	1.37
+++ nsGfxTextControlFrame2.cpp	2000/06/17 00:09:21
@@ -752,6 +752,11 @@
     }
   }
 
+  nsCOMPtr<nsIDOMElement> root;
+  mEditor->GetRootElement(getter_AddRefs(root));
+  nsCOMPtr<nsIContent> rootContent = do_QueryInterface(root);
+  rootContent->SetDocument(nsnull, PR_TRUE, PR_FALSE);
+
   mSelCon = 0;
   mEditor = 0;
   
I stepped through rootcontent->SetDocument() in the debugger, and it turns out
that none of the children had script objects! So maybe the analysis that dbaron
and I came up with is wrong...
Keywords: mlk
Are you sure this isn't bug 42530? 42530 is a nasty thing that could have lots of 
ramifications, it seems. I wouldn't trust any leak data until that one is nailed.
*** Bug 42934 has been marked as a duplicate of this bug. ***
Nominating this for nsbeta2.  Here is a table of leaks I think this causes.
(Although not all the leaks in the mail numbers may be caused by this, the
incremental amount per compose window almost definitely is, since the
nsXULDocument leaked went from 3 to 4 when going from 1 to 2 compose windows.)
Any windows opened are immediately closed, in reverse order (except the first
mail compose window was closed and then the second opened).

 657K  Open one browser window, mouse over URLbar
 665K  Ditto, but type in URL bar too.
 960K  Open two browser windows (2d via File | New ...),mouseover both URLbars
1342K  Same for 3 windows

1586K  Open browser window, mail window, and one mail compose window
1874K  Open browser window, mail window, and 2 mail compose windows
Keywords: nsbeta2
Summary: mouseover URL bar leaks 700Kb → event handling on inputs leaks containing XUL document
setting to m17
Target Milestone: --- → M17
Putting on [nsbeta2+] radar for beta2 fix. 
Whiteboard: [nsbeta2+]
so what do I do now then? wait for xbl magic? if so should this be on hyatts 
plate?  I dont mind doing the work, I am just worried that this bug is sitting 
here with no one paying attention to it.
Status: NEW → ASSIGNED
This leak was not fixed by Brendan's fix for bug 42530.  It still exists today.
Sending over to hyatt per Composer Beta2 bug review today.
Assignee: mjudge → hyatt
Status: ASSIGNED → NEW
Dan has agreed to take this.  Isn't he nice?
Assignee: hyatt → danm
I think the reason waterson's fix didn't work is that the code he wrote gets 
executed after the rest of the unrooting.  The unrooting is normally triggered 
by nsDocument::SetScriptGlobalObject(nsnull), which calls SetDocument(nsnull, 
PR_TRUE, PR_TRUE) on all the children and then sets the global object to null.  
nsGenericElement::SetDocument won't be able to remove the root if the global 
object is null because it needs the global object to get the script context.  At 
least that's how it gets it right now.
I'll take this bug since I have a fix for the leak.  If you want to rewrite this 
stuff in XBL for other reasons, that should probably be a separate bug.  I'll 
try to check this in later today.  (I clobbered and am waiting for the build to 
finish in the hopes that my failure to start was a dependencies problem.)
Assignee: danm → dbaron
Fix checked in, 2000-07-14 16:20-0700.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
verified in 7/17 build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: