Closed Bug 405032 Opened 14 years ago Closed 13 years ago

Changing feed handler to web handler leaks nsGlobalWindow

Categories

(Firefox :: Preferences, defect, P4)

defect

Tracking

()

RESOLVED FIXED
Firefox 3.1b1

People

(Reporter: cbook, Assigned: peterv)

References

Details

(Keywords: memory-leak)

Attachments

(4 files)

Attached file leak log
Changing RSS Handlers via Tools -> Options -> Applications leak

steps to reproduce
-Start Firefox
-Go to tools - Options - Application
-Change the RSS handler from ask me to Google (or bloglines)
- LEAK !
Flags: blocking-firefox3?
Assignee: nobody → myk
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P3
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox 3 M10
Summary: Changing RSS Handlers via Tools -> Options -> Applications leak → Changing RSS Handlers via Tools -> Options -> Applications leaks nsGlobalWindow
When you set the feed handler to a web app, feedHandlerInfo calls nsIWebContentConverterService::setAutoHandler with the nsIWebContentHandlerInfo representing the app.  That nsIWebContentHandlerInfo comes from nsIWebContentConverterService and is referenced by feedHandlerInfo in its _possibleApplicationHandlers property.

It's not clear what is happening, but I suspect that somewhere we're creating a circular reference between the converter service and the preferences dialog via the handler info object.
Note that the problem doesn't occur if you set the feed to "preview in Firefox".  It only happens if you set it to a handler app.
I just checked what happens if I change the feed handler to a web handler via the feed preview page, and it leaks the same way there too, so this is not specific to the Applications prefpane, it's a problem with the web content converter service and its setAutoHandler method.

To test with the feed preview page:

1. set your feed pref in Preferences > Applications to "preview in Firefox";
2. load a web page with a feed (f.e. http://planet.mozilla.org/);
3. click the feed icon in the location bar to load the feed preview page;
4. select a web handler from the "Subscribe to this feed using" dropdown menu;
5. check the "Always use [handler] to subscribe to feeds" checkbox;
6. press the "Subscribe Now" button.

cc: a couple other folks who have touched the service recently for their input.
Summary: Changing RSS Handlers via Tools -> Options -> Applications leaks nsGlobalWindow → Changing feed handler to web handler leaks nsGlobalWindow
Mano might also have some insight here, since he put together the previous incarnation of the feed handling prefs.
Attached file new leak log
new Leak Log with a debug build (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008012113 Minefield/3.0b3pre) after Bug 412491 was checked in
Target Milestone: Firefox 3 beta2 → Firefox 3 beta4
I've taken several looks at this, and I still can't figure out what's leaking.  I'm using XPCOM_MEM_LEAK_LOG and bloatdiff.pl, but it's returning different output than in the leak log Carsten posted.  How is that one generated?
Hi Myk, here is a new leak log from Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4pre)
Gecko/2008022515 Firefox/3.0b4pre.

I was only able to reproduce this leak when switching the web-handler in the Applications Options tab (like from Google to Bloglines etc).

From the Leak Log it seems that the nsGlobalWindows Leak was fixed by our other fixes that were checked in during the last weeks.

In general i use TraceRefcnt Debug Build http://wiki.mozilla.org/MozillaQualityAssurance:Home_Page:Firefox_3.0_TestPlan:Leaks:LeakTesting-How-To:Debug_Builds
Target Milestone: Firefox 3 beta4 → Firefox 3
Priority: P3 → P4
Not blocking on this bug for final ship. Would take a safe enough patch if one comes through.
Flags: wanted-firefox3+
Flags: blocking-firefox3-
Flags: blocking-firefox3+
Attached patch v1Splinter Review
Putting XPCOM objects on the prototype keeps them alive until the nsIModule/nsIFactory are released, which is after the last cycle collection. This component is used as a service, so not putting things on the prototype is ok, we don't need to share between multiple instances.
Assignee: myk → peterv
Attachment #338343 - Flags: review?(gavin.sharp)
OS: Windows XP → All
Hardware: PC → All
Attachment #338343 - Flags: review?(gavin.sharp) → review+
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: Firefox 3 → Firefox 3.1b1
You need to log in before you can comment on or make changes to this bug.