Closed
Bug 347824
Opened 18 years ago
Closed 18 years ago
RSS leaks windows and documents
Categories
(Firefox Graveyard :: RSS Discovery and Preview, defect)
Firefox Graveyard
RSS Discovery and Preview
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 2 beta2
People
(Reporter: sayrer, Assigned: sayrer)
Details
(Keywords: fixed1.8.1, memory-leak)
Attachments
(1 file)
859 bytes,
patch
|
vlad
:
review+
beltzner
:
approval1.8.1+
|
Details | Diff | Splinter Review |
We need to break the cycle with the window and document on close.
Assignee | ||
Comment 1•18 years ago
|
||
here's what we need to do: 487 /** 488 * See nsIFeedWriter 489 */ 490 close: function FW_close() { + this._document = null; + this._window = null; 491 var prefs = 492 Cc["@mozilla.org/preferences-service;1"]. 493 getService(Ci.nsIPrefBranch2); 494 prefs.removeObserver(PREF_SELECTED_ACTION, this); 495 prefs.removeObserver(PREF_SELECTED_READER, this); 496 prefs.removeObserver(PREF_SELECTED_APP, this); 497 }, 498
Status: NEW → ASSIGNED
Flags: blocking-firefox2?
Updated•18 years ago
|
Assignee | ||
Comment 2•18 years ago
|
||
Attachment #232647 -
Flags: review?(bugs)
Assignee | ||
Updated•18 years ago
|
Whiteboard: [has patch][needs review ben]
Assignee | ||
Updated•18 years ago
|
Attachment #232647 -
Flags: review?(bugs) → review?
Assignee | ||
Updated•18 years ago
|
Attachment #232647 -
Flags: review?
Assignee | ||
Updated•18 years ago
|
Whiteboard: [has patch][needs review ben]
Assignee | ||
Comment 3•18 years ago
|
||
The patch here fixes this bug for me, in combination with the one in bug 344983. Adam hasn't been able to verify, though. Steps to reproduce from Adam Guthrie: 1. Start Firefox with ./firefox -P trunk http://slashdot.org/. 2. Click on the RSS icon in the location bar. 2. Go to mozilla.com (I just had a bookmark in my bookmarks toolbar and it was convenient to click). ------------ Leaked inner window 8ca9628 (outer 89d0870) at address 8ca9628. ... with URI "http://www.mozilla.com/". Leaked inner window 8d7b080 (outer 89d0870) at address 8d7b080. ... with URI "http://rss.slashdot.org/Slashdot/slashdot". Leaked outer window 89d0870 at address 89d0870. Leaked document at address 913b1b8. ... with URI "chrome://global/content/bindings/scrollbar.xml". ... with URI "jar:file:///moz/trunk/mozilla/ff-debug/dist/bin/chrome/toolkit.jar!/content/global/bindings/scrollbar.xml". Leaked document at address 9075f30. ... with URI "http://rss.slashdot.org/Slashdot/slashdot". ... with URI "jar:file:///moz/trunk/mozilla/ff-debug/dist/bin/chrome/browser.jar!/content/browser/feeds/subscribe.xhtml". Summary: Leaked 3 out of 9 DOM Windows Leaked 2 out of 56 documents Leaked 0 out of 3 docshells
Assignee | ||
Comment 4•18 years ago
|
||
My results before patch: Leaked inner window 1a33ad20 (outer 192f6900) at address 1a33ad20. ... with URI "http://www.mozilla.com/firefox/central/". Leaked inner window 1a198960 (outer 192f6900) at address 1a198960. ... with URI "http://rss.slashdot.org/Slashdot/slashdot". Leaked outer window 192f6900 at address 192f6900. Leaked document at address 1d3c000. ... with URI "jar:file:///Users/sayrer/firefox/mozilla/fb-debug/dist/MinefieldDebug.app/Contents/MacOS/chrome/browser.jar!/content/browser/feeds/subscribe.xhtml". ... with URI "http://rss.slashdot.org/Slashdot/slashdot". Summary: Leaked 3 out of 9 DOM Windows Leaked 1 out of 41 documents Leaked 0 out of 3 docshells My results after: Summary: Leaked 0 out of 9 DOM Windows Leaked 0 out of 40 documents Leaked 0 out of 3 docshells
Assignee | ||
Comment 5•18 years ago
|
||
Comment on attachment 232647 [details] [diff] [review] clear the reference to the document and window Okay, this patch may not get all of Adam's leaks, but it certainly helps. Sorry for the spam.
Attachment #232647 -
Flags: review?(bugs)
Assignee | ||
Updated•18 years ago
|
Whiteboard: [has patch][needs review ben]
Updated•18 years ago
|
Flags: blocking-firefox2? → blocking-firefox2+
Comment on attachment 232647 [details] [diff] [review] clear the reference to the document and window r=me
Attachment #232647 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 7•18 years ago
|
||
/cvsroot/mozilla/browser/components/feeds/src/FeedWriter.js,v <-- FeedWriter.js new revision: 1.9; previous revision: 1.8
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][needs review ben]
Assignee | ||
Updated•18 years ago
|
Attachment #232647 -
Flags: approval1.8.1?
Comment 8•18 years ago
|
||
Comment on attachment 232647 [details] [diff] [review] clear the reference to the document and window a=drivers for MOZILLA_1_8_BRANCH
Attachment #232647 -
Flags: approval1.8.1? → approval1.8.1+
Assignee | ||
Comment 9•18 years ago
|
||
Checking in FeedWriter.js; /cvsroot/mozilla/browser/components/feeds/src/FeedWriter.js,v <-- FeedWriter.js new revision: 1.2.2.11; previous revision: 1.2.2.10 done
Keywords: fixed1.8.1
Updated•5 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•