192336 - leak when processing key events on zombie pages

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P1)

Description

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

I've been noticing lately that we've been leaking a lot (the little memory
monitor in the corner of my gnome panel drops a lot when I exit Mozilla after
running for a while).  So I tried to do some leak tests on "normal browsing
situations" for me, and I found a major leak while handling key events during
page loads.

Basically, when aaronl fixed the bug about key events on zombie pages (I'm
citing this all, including the patch author, from memory), he added an early
return into a function that didn't have any early returns in the area where he
added it, and that relied on manual refcounting.  The early return prevented the
NS_RELEASE from happening.

I'll attach a patch to convert this manual refcounting to an nsCOMPtr to fix the
leak.

Comment 1

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

Attachment: patch

This patch won't actually apply because I had to chop off some other changes at
the bottom.

Comment 2

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

Attachment: patch

This one should apply correctly.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 113866 [details] [diff] [review]
patch

nice catch

Comment 4

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

Comment on attachment 113866 [details] [diff] [review]
patch

This should be pretty low-risk, although there's a slight chance something
could go wrong if there's matching incorrect code (i.e., code that
over-releases) in the new codepath that aaronl added, which seems pretty
unlikely.

Then again, it also seems like if anything did go wrong, we'd find out from
talkback data, so it might also be reasonable to take this right after 1.3beta
ships.

Comment 5

[Asa Dotzler [:asa]]

Comment on attachment 113866 [details] [diff] [review]
patch

Let's hold this 'till final opens. I think we have all the beta bugs wrapped up
and I'd like to get a build out the door ASAP.

Comment 6

[Asa Dotzler [:asa]]

Comment on attachment 113866 [details] [diff] [review]
patch

a=asa (on behalf of drivers) for checkin to 1.3 final.

Comment 7

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

Fix checked in to trunk, 2003-02-11 12:26 PST.