257649 - Leaks introduced by checkin to nsDOMClassInfo.cpp

Status: RESOLVED FIXED

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

Description

[Robert Kaiser]

Brad tinderbox show that leaks increased by to the level before you fixed the
ones from bug 256932

The increase happend late on 2004-08-30, and the only checkin in this time
window is to nsDOMClassInfo.cpp - see
http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?module=MozillaTinderboxAll&date=explicit&mindate=1093908180&maxdate=1093917179

jst, you probably know that change, as you did it yourself, but as a reminder,
here's the diff:
http://tinderbox.mozilla.org/bonsai/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/dom/src/base&command=DIFF_FRAMESET&file=nsDOMClassInfo.cpp&rev1=1.240&rev2=1.241&root=/cvsroot

brad shows an increase of refcnt leaks (RLk) from 268B to 7.22KB and of leaks
(Lk) from 302KB to 318KB.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Chances are, we're pulling the pulluter out of the proto chain before we call
GetInvalidatedGlobalScopePolluter, so that function does nothing...

Comment 2

[Johnny Stenback (:jst)]

The odd thing is that I do *not* see these leaks on my system. Last time I did,
but this time all but five nsEventListenerManagers are deleted when I start
firefox and immediately exit it. Last time I saw 137 nsEventListenerManagers
leaked, just like tinderbox did...

Comment 3

[Johnny Stenback (:jst)]

Attachment: Don't hand the old document to the gsp when setting a new document.

Found it. We were handing the old document to the gsp when
GlobalWindowImpl::SetNewDocument(nsnull) was called, which was fine in all
cases except when closing a window where there wouldn't be a following call
with a new document to SetNewDocument(). That's how the old document got left
in the gsp on shutdown.

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 157631 [details] [diff] [review]
Don't hand the old document to the gsp when setting a new document.

r+sr=me in lieu of dbaron, with his consent.   Also approving for branches.

/be

Comment 5

[Johnny Stenback (:jst)]

Fixed on trunk and aviary.

Comment 6

[Daniel Veditz [:dveditz]]

Did this land on 1.7 branch also?

Comment 7

[Johnny Stenback (:jst)]

Not yet, it's waiting on the initial document.all and global name/id resolve
code to land. Once I get some spare time, I'll land it all...

Comment 8

[Mike Kaply [:mkaply]]

This should be able to land on 1.7 now, correct?