Closed Bug 384230 Opened 18 years ago Closed 17 years ago

leak input element when password manager prefills a form

Categories

(Toolkit :: Password Manager, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dbaron, Assigned: Dolske)

References

Details

(Keywords: memory-leak)

Steps to reproduce: 1. create an empty directory (called <directory> below) 2. run /path/to/firefox -profile <directory> (with a debug build) 3. go to http://bugzilla.mozilla.org/enter_bug.cgi?product=Core 4. enter your login and password, hit the log in button, and tell firefox to remember the password 5. log out 6. quit firefox 7. set the environment variable XPCOM_MEM_LEAK_LOG=leak.log, and run firefox again with the same profile, passing http://bugzilla.mozilla.org/enter_bug.cgi?product=Core as the URL on the command line 8. close the window 9. look at leak.log Actual results: leaked a bunch more than normal, including an nsHTMLInputElement Expected results: didn't leak more than normal
I think what's happening here is that Satchel is never deleting entries from it's mPwmgrInputs hashtable. The hashtable used to live in pwmgr until I did the JS rewrite and moved it to satchel, but it looks like it leaked the same way there too. http://mxr.mozilla.org/seamonkey/source/toolkit/components/satchel/src/nsFormFillController.cpp?rev=1.88#549 The mPwmgrInputs.Enumerate(RemoveForDOMDocumentEnumerator, domDoc) call is only invoked for pagehide events, so if you close a tab/window with an input field on it (which pwmgr is watching as a username field), it'll leak.
Assignee: nobody → dolske
OS: Linux → All
Hardware: PC → All
Target Milestone: --- → Firefox 3 M7
So, the fix here ought to be trivial -- just have unload events execute the same code as the pagehide event handler current does. The existing code already even registers for unload events for another purpose. Unfortunately, the unload event doesn't seem to be triggering here, and I don't understand why. I added some simple printf()s to nsFormFillController::Unload, but it only seems to be invoked sporadically. Oddly, I also added a printf() to nsFormFillController::BeforeUnload, and it *does* seem to be getting invoked.
Target Milestone: Firefox 3 M7 → Firefox 3 M8
Not going to get to this for M8.
Target Milestone: Firefox 3 M8 → Firefox 3 M9
Well, I filed bug 376563 for unload not being fired, but I tested it yesterday and it seemed to be working now. But probably it would be wise if someone else could test it too.
Depends on: 376563
[After more debugging over in bug 397753, it looks like bug 376563 isn't actually fixed so I've reopened and marked it as blocking this bug]
Untargeting, can't do anything here until either: * bug 376563 is fixed (then we can add code to Unload to fix the leak) * bug 397753 is fixed (which removes the need for Unload, but is itself blocked on bug 398502 (making "pagehide" fire properly))
Target Milestone: Firefox 3 M9 → ---
Depends on: 397753
Whiteboard: [blocked]
This is actually just waiting on bug 397753, now. That fix should resolve this one, although since it wasn't specifically about the leak I won't DUPE this one to it.
Whiteboard: [blocked]
Is this now fixed, now that bug 397753 is fixed?
It should be, but I ran dbaron's testcase from comment 0 and was still seeing the document leak. :( Could be something else, though, but I didn't finish narrowing it down last night.
Yes, this is fixed now. The leaks I was seeing can be reproduced by just loading about:blank, and moving the mouse in a certain way (moving it a different way doesn't leak). If I load a bugzilla login page (with or without a stored login), and close it in varying ways (being careful to avoid the other leak I found), nothing is leaked. I'll file a new bug on the different leak I found.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.