49648 - stylesheets leaked when manipulated from JS

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The code that does alternate stylesheet selection in the browser (View | Use
Stylesheet) just uses JS to manipulate the stylesheets.  It leaks.  (I just
fixed one minor leak that I'll check in soon, but that's not this bug.)

STEPS TO REPRODUCE:
 * load http://www.people.fas.harvard.edu/~dbaron/
 * Select View | Use Stylesheet | Ultramarine

RESULTS:
 * leak a bunch of CSSStyleSheetImpl and one HTMLStyleSheetImpl

I was looking at the HTMLStyleSheetImpl here -- I assume the others are similar.

We leak multiple references to the object, all acquired on line 91 of
nsJSStyleSheetList.cpp, in the function GetStyleSheetListProperty, when calling
Item.  I'm not sure how these references are supposed to work - i.e., whether
they should be garbage-collected, or what.  But they leak.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Actually, I think this is just more of the same.  Testing this patch right now:
(The QI must return a separate object...)

Index: nsDocument.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/base/src/nsDocument.cpp,v
retrieving revision 3.238
diff -u -d -r3.238 nsDocument.cpp
--- nsDocument.cpp	2000/08/21 13:05:53	3.238
+++ nsDocument.cpp	2000/08/21 13:22:54
@@ -271,7 +271,7 @@
   
     // XXX Not particularly efficient, but does anyone care?
     for (i = 0; (i < imax) && (nsnull == *aReturn); i++) {
-      nsCOMPtr<nsIStyleSheet>
sheet(do_QueryInterface(mDocument->GetStyleSheetAt(i)));
+      nsCOMPtr<nsIStyleSheet>
sheet(dont_AddRef(mDocument->GetStyleSheetAt(i)));
       if (!sheet)
         continue;
       nsCOMPtr<nsIDOMStyleSheet> domss(do_QueryInterface(sheet));
@@ -279,7 +279,7 @@
       if (domss) {
         if (count++ == aIndex) {
           *aReturn = domss;
-          NS_IF_ADDREF(*aReturn = domss);
+          NS_IF_ADDREF(domss);
         }
       }
     }

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

In my latter change, change domss to *aReturn.  domss is an nsCOMPtr...

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: proposed fix

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Taking.  Have r=mjudge.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in 2000-08-21 12:46 PDT.