Closed
Bug 14813
Opened 25 years ago
Closed 25 years ago
MLK [Memory Leak] We're leaking our urls in nsGlobalWindow
Categories
(Core :: DOM: Core & HTML, defect, P3)
Tracking
()
VERIFIED
FIXED
M11
People
(Reporter: mscott, Assigned: mscott)
Details
Attachments
(1 file)
2.02 KB,
patch
|
Details | Diff | Splinter Review |
I recently started running mailnews urls through the webshell when displaying a message. We have a lot of things that hang off mailnews nsIURI subclasses. So leaking a url is really expensive for us. I noticed that urls which are run through the webshell all leak! I've spent the last two days trying to figure out where they are and I now have fixes and am filing bugs for these fixes so I can get things checked in. Vidur, I found two leaks of the url in nsGlobalWindow. I was hoping you could code review my changes for me. The first one was: nsCOMPtr<nsIURI> uri = doc->GetDocumentURL(); This leaks because GetDocumentURL returns a referenced uri which the com ptr assignment than ref cnts again. So the object has been ref counted twice for the com ptr. It should look like: nsCOMPtr<nsIURI> uri = dont_AddRef(doc->GetDocumentURL()); The second instance was: docURL = doc->GetDocumentURL(); docURL wasn't a com ptr (which would have helped prevent the leak) and there was no release on it. I changed it to be: docURL = dont_AddRef(doc->GetDocumentURL()); I'm attaching a patch to this bug report of what I would like to get code reviewed. The changes are a few more lines than the few I listed here because I ripped out a couple #ifdef NECKO lines which were near the code I was touching. Can you review this Vidur? And then I'll get approval from Chris H. to check in. Thanks!
Assignee | ||
Comment 1•25 years ago
|
||
Assignee | ||
Updated•25 years ago
|
Status: NEW → ASSIGNED
Target Milestone: M11
Assignee | ||
Comment 2•25 years ago
|
||
accepting...marking as M11.
Updated•25 years ago
|
Summary: [Memory Leak] We're leaking our urls in nsGlobalWindow → MLK [Memory Leak] We're leaking our urls in nsGlobalWindow
Comment 3•25 years ago
|
||
Go for it, mscott.
Assignee | ||
Updated•25 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Comment 4•25 years ago
|
||
Scott MacGregor, would you like to verify this one ?
Assignee | ||
Updated•25 years ago
|
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 5•25 years ago
|
||
Sure thing. Since I checked it in =) Marking as verified.
Comment 6•25 years ago
|
||
Thanks scott
You need to log in
before you can comment on or make changes to this bug.
Description
•