Bug 110029 - [JSC] Remove custom mark function for NamedNodeMap.
Summary: [JSC] Remove custom mark function for NamedNodeMap.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-16 13:48 PST by Andreas Kling
Modified: 2013-02-16 17:04 PST (History)
6 users (show)

See Also:

Attachments
Patch (2.42 KB, patch)
2013-02-16 13:49 PST, Andreas Kling 		darin: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2013-02-16 13:48:19 PST 
NamedNodeMap already keeps its Element owner alive, so we can dump the FIXME and custom visitChildren() in the JSC wrapper.
Comment 1 Andreas Kling 2013-02-16 13:49:12 PST 
Created attachment 188728 [details]
Patch
Comment 2 Darin Adler 2013-02-16 14:01:06 PST 
Comment on attachment 188728 [details]
Patch

I believe this is no longer needed. But do we have tests that cover it?
Comment 3 Geoffrey Garen 2013-02-16 15:17:23 PST 
(In reply to comment #0)
> NamedNodeMap already keeps its Element owner alive, so we can dump the FIXME and custom visitChildren() in the JSC wrapper.

Has this always been true, or is it a recent change?

I'm pretty sure I originally learned about the need for this function the hard way.
Comment 4 Andreas Kling 2013-02-16 15:23:47 PST 
(In reply to comment #3)
> (In reply to comment #0)
> > NamedNodeMap already keeps its Element owner alive, so we can dump the FIXME and custom visitChildren() in the JSC wrapper.
> 
> Has this always been true, or is it a recent change?
> 
> I'm pretty sure I originally learned about the need for this function the hard way.

I changed it with https://trac.webkit.org/changeset/104130
Comment 5 Andreas Kling 2013-02-16 15:40:55 PST 
(In reply to comment #2)
> (From update of attachment 188728 [details])
> I believe this is no longer needed. But do we have tests that cover it?

Yup! You added fast/dom/Attr/access-after-element-destruction.html for this issue here: http://trac.webkit.org/changeset/48769

That one also covers Attr node wrappers which are slightly less trivial to fix, but might be worth doing next.
Comment 6 Andreas Kling 2013-02-16 17:04:02 PST 
Committed r143118: <http://trac.webkit.org/changeset/143118>