Closed
Bug 323534
Opened 19 years ago
Closed 19 years ago
createTreeWalker can cause leaks due to cycles created by closures
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: dbaron, Assigned: dbaron)
References
Details
(Keywords: fixed1.8.1, memory-leak, Whiteboard: [patch])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
mrbkap
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
In bug 285065 I fixed a bug where createTreeWalker led to a cycle by fixing the caller. Jesse pointed out that we should fix the general problem. This could be fixed using the same approach as bug 241518.
Assignee | ||
Comment 2•19 years ago
|
||
I need to check if anything has treewalker member variables, though.
Assignee | ||
Comment 3•19 years ago
|
||
(FWIW, this approach in general is sort of bad for the C++ API -- it makes it hard for somebody to use TreeWalker from C++ with a NodeFilter implementation written in JS.)
Assignee | ||
Updated•19 years ago
|
Whiteboard: [patch]
Assignee | ||
Comment 4•19 years ago
|
||
Oops, forgot to change inheritance of nsEventReceiverSH. Still not tested.
Attachment #208567 -
Attachment is obsolete: true
Assignee | ||
Updated•19 years ago
|
Attachment #208569 -
Flags: superreview?(jst)
Attachment #208569 -
Flags: review?(mrbkap)
Comment 6•19 years ago
|
||
Comment on attachment 208569 [details] [diff] [review] patch >+ // XXX nsEventReceiverSH::Finalize: clear event handlers in mListener... Uber-nit: Line this up with the leftmost column? r=mrbkap with that.
Attachment #208569 -
Flags: review?(mrbkap) → review+
Comment 7•19 years ago
|
||
Comment on attachment 208569 [details] [diff] [review] patch sr=jst
Attachment #208569 -
Flags: superreview?(jst) → superreview+
Assignee | ||
Comment 8•19 years ago
|
||
Checked in to trunk.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Updated•19 years ago
|
Flags: blocking1.8.0.3?
Updated•19 years ago
|
Flags: blocking1.8.0.3? → blocking1.8.0.3+
Comment 9•19 years ago
|
||
The addEventListener dependency (bug 241518) isn't going to land on the 1.8.0 branch so this can't either.
Flags: blocking1.8.0.3+ → blocking1.8.0.3-
Updated•19 years ago
|
Flags: blocking1.8.1?
Assignee | ||
Comment 10•19 years ago
|
||
Fixed on MOZILLA_1_8_BRANCH by checkin of bug 336791.
Keywords: fixed1.8.1
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•