From Bugzilla Helper: User-Agent: Mozilla/4.0 (compatible; MSIE 5.01; Windows NT; IE4WDUS-1999052001) BuildID: 2000042708 The CSSImportantRule objects never got freed. I brought up mozilla, visited two sites, www.yahoo.com and www.intel.com, and the bloatview log shows that there were 21 CSSImportantRule objects created and none of them got freed. Reproducible: Always Steps to Reproduce: 1.Bring up the browser. 2.Visit couple of sites, any sites. 3.Look at the bloatview log.

Would the module owner please review the patch. -Thanks-

add marc to cc list.

I agree that it looks like the mImportantRule is being double-addref'd when it has to be allocated, and the patch fixes that. When we are open for checkins this patch should be applied, something I am happy to do. Thanks again, Carl, for a good patch! Also, I'm taking this bug over since I generally handle the memory-leak and performance bugs in style (Pierre won't argue with this I'm sure).

I'm nominating this bug for nsbeta2. This leak is encountered everytime one visits a new site (see initial bug report). Bloatview indicates that we are leaking about 30k per site.

Two comments here: 1) I don't see the leak in a build from yesterday (with a few leak fixes in my tree, but not this one). I see 19 CSSImportantRule objects created, with 927 references, and all references are released and all objects deleted. I only leak the "standard" 6-7K loading http://www.yahoo.com/ and http://www.intel.com/ . Do you have a user stylesheet or something that would make things different for you? 2) I don't see where the double-addref is. The addref you're removing in the patch is so that the mImportantRule (a member variable) is an owning reference (it is freed in the destructor). The other addref is since the getter addrefs its result.

Oops - Looks like David is right and I was too hasty in my analysis. There are two addref's for good reason, one to hold the member-reference and one for passing the interface out. Carl, don't check this patch in please. Since David isn't seeing this leak I think we need to figure out why Carl is and go from there. Carl, can you please retest? Until then I'm clearing the nsbeta2 keyword.

I took another good look at it and David was correct, the patch was bad and I won't check it in. However, I did see a leak in my bloatview log and I did a little investigation and here is what I found. If I use Mozilla, the leak shows up in the log. But if I use Viewer, the leak doesn't show up at all. David, I assume you were using Viewer instead of Mozilla, right?? Should we be using Viewer instead of Mozilla as our testbed????

No, I was using Mozilla. (Today, using Mozilla, I'm seeing a lot more leaked on http://www.intel.com/ , but the root leak looks like an nsParser/HTMLContentSink or something in HTTP, and there are no CSSImportantRule objects leaked.) I'm curious why this is leaking for you. Could you attach the whole leak log that shows all of the objects leaked (assuming you still see only 30K leaked, since the nsParser leak makes it hard to find the other leaks) ? (A leak log, from XPCOM_MEM_LEAK_LOG is a lot easier to read than a complete bloat log, from XPCOM_MEM_BLOAT_LOG.) What date was your tree? [ Also, could the leak be from something other than those 2 pages or startup, for example, your home page or default sidebar panel, or the way you opened the URLs? ]

David,I have attached the memory leak log file and as you can see there is an entry for the CSSImportantRule in the log. The build that I was using was May 18th build. I simply brought up mozilla, the check-in page appeared by default, and then I browsed to the two URLs mentioned above. This is puzzling that it shows up on mine but not yours.

In that log (with 440K of leaks), the CSSImportantRule objects certainly don't look like the root leaks. I would focus on the nsXULDocument, nsJSContext, GlobalWindowImpl, or CSSStyleSheetImpl leaks first... Do the leaks go away if you undo the fix for bug 27739 (as I have in my tree)? The fix was http://bugzilla.mozilla.org/showattachment.cgi?attach_id=6312

Carl, should this be bug closed or are you still seeing a leak that we need to track?

By backing out the fix for bug #2773, most of CSS related leaks went away. You can go ahead and close this. But someone needs to make sure that bug#2773 is fixed the right way.

OOPS. I meant bug 27739 not 2773.

I'll add a dependency to 24645, which is hyatt's bug to replace the keybindings implementation with XBL. This leak is not critical in terms of "real" memory loss; however, it is annoying because it impedes embedding and makes it harder to detect "real" leaks because it can entrain so much memory.

This will be fixed when 24645 is fixed. Keeping open to track it, giving milestone FUTURE to help me organize my list.

=> Carl @ Intel

We converted to XBL, can someone verify this?

Since the leaks described went away when backing out the fix for bug 27739, I'm going to mark this as a duplicate of bug 17390, since my fix for that leak fixed those leaks. *** This bug has been marked as a duplicate of 17390 ***