Closed Bug 347824 Opened 18 years ago Closed 18 years ago

RSS leaks windows and documents

Categories

(Firefox Graveyard :: RSS Discovery and Preview, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 2 beta2

People

(Reporter: sayrer, Assigned: sayrer)

Details

(Keywords: fixed1.8.1, memory-leak)

Attachments

(1 file)

We need to break the cycle with the window and document on close.
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?
Keywords: mlk
OS: Mac OS X 10.3 → All
Hardware: PC → All
Attachment #232647 - Flags: review?(bugs)
Whiteboard: [has patch][needs review ben]
Attachment #232647 - Flags: review?(bugs) → review?
Attachment #232647 - Flags: review?
Whiteboard: [has patch][needs review ben]
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
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
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)
Whiteboard: [has patch][needs review ben]
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+
/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]
Attachment #232647 - Flags: approval1.8.1?
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+
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
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: