leak input element when password manager prefills a form

RESOLVED FIXED

Status

()

Toolkit
Password Manager
RESOLVED FIXED
11 years ago
9 years ago

People

(Reporter: dbaron, Assigned: Dolske)

Tracking

({mlk})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

11 years ago
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
Is this related to bug 385082?
(Assignee)

Comment 2

11 years ago
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
(Assignee)

Comment 3

11 years ago
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.
(Assignee)

Updated

11 years ago
Target Milestone: Firefox 3 M7 → Firefox 3 M8
(Assignee)

Comment 4

10 years ago
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.
(Assignee)

Updated

10 years ago
Depends on: 376563
(Assignee)

Comment 6

10 years ago
[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]
(Assignee)

Comment 7

10 years ago
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 → ---
(Assignee)

Updated

10 years ago
Depends on: 397753
Whiteboard: [blocked]
(Assignee)

Comment 8

10 years ago
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?
(Assignee)

Comment 10

10 years ago
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.
(Assignee)

Comment 11

10 years ago
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
Last Resolved: 10 years ago
Resolution: --- → FIXED
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.