490071 - nsTypedSelection cycle collection should traverse to mSelectionListeners

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect, P4)

Description

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

I just got a leak monitor alert after closing a view-source window:

Leaks in window 0x20839d00:
[+] [leaked object] (20840b00) = [Object]
 [ ] timeout = 0
 [ ] notifySelectionChanged (20840b40, chrome://global/content/viewSource.js, 25-29) = [Function]

The leak went away after the next GC (I got a "reclaimed leak alert").

I can't figure out how to get this to happen again.


However, from looking at code, I'm willing to guess that it might not have happened in the first place if nsTypedSelection's cycle collection implementation traversed and unlinked its mSelectionListeners.

Comment 1

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

I'm not sure why peterv didn't do this in bug 471126.

We can also use this patch to address bug 471126 comment 20.

Comment 2

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

Attachment: patch

Comment 3

[Peter Van der Beken [:peterv]]

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

I think I was worried about selection listeners not getting notifications of the removal of the ranges. I guess we can try it out.

Comment 4

[Peter Van der Beken [:peterv]]

BTW, I don't think the notifications from the removal of the ranges in unlink is a problem, because unlink happens from the GC callback for JSGC_END.

Comment 5

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

http://hg.mozilla.org/mozilla-central/rev/443174a68358