SVGElementInstance should have EventTarget on the prototype chain

[Dominic Cooney]

Per the spec here: <http://www.w3.org/TR/SVG/struct.html#InterfaceSVGElementInstance> SVGElementInstance is declared to extend EventTarget. So it should have EventTarget on the prototype chain instead of declaring addEventListener, removeEventListener and dispatchEvent itself.

[Dominic Cooney]

Created attachment 145586 [details]
WIP Patch

[Dominic Cooney]

Created attachment 145647 [details]
WIP Patch

[Dominic Cooney]

Created attachment 145793 [details]
WIP Patch

[Gyuyoung Kim]

Comment on attachment 145793 [details]
WIP Patch

Attachment 145793 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/12894661

[Dominic Cooney]

Comment on attachment 145793 [details]
WIP Patch

EFL needs more IDL includes.

[Dominic Cooney]

Created attachment 145861 [details]
WIP Patch

[Gyuyoung Kim]

Comment on attachment 145861 [details]
WIP Patch

Attachment 145861 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/12902644

[Dominic Cooney]

Created attachment 146017 [details]
Patch

[Adam Barth]

Sorry for the delay.  Looking now.

[Adam Barth]

Comment on attachment 146017 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=146017&action=review

Cool!

> Source/WebCore/ChangeLog:51
> +        * svg/SVGElementInstance.h: Must extend EventTarget first so that
> +        static_cast<EventTarget*>(elementInstance) is the same pointer as
> +        elementInstance, similar to how static_cast<Node*>(element) is the
> +        same pointer as element.

Should we add an ASSERT for this?

> Source/WebCore/bindings/js/JSEventTargetCustom.cpp:76
> +    TRY_TO_UNWRAP_WITH_INTERFACE(EventTarget)

Can you add a FIXME about removing this once all the event targets are smarter?

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1339
> +    my $eventTarget = $dataNode->extendedAttributes->{"EventTarget"} || $codeGenerator->IsStrictSubtype($dataNode, "EventTarget");

Should we add a helper function for this pattern since we're using it in a bunch of places?

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3480
> +    # TODO: When EventTarget is an interface and not a mixin, fix this so that

TODO -> FIXME

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3609
> +    # TODO: When EventTarget is an interface and not a mixin, fix this to use

TODO -> FIXME

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3735
> +    # TODO: When EventTarget is an interface and not a mixin, fix this so that

TODO -> FIXME

[Dominic Cooney]

Created attachment 156281 [details]
To run by bots

[Early Warning System Bot]

Comment on attachment 156281 [details]
To run by bots

Attachment 156281 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13430315

[Early Warning System Bot]

Comment on attachment 156281 [details]
To run by bots

Attachment 156281 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13432117

[Build Bot]

Comment on attachment 156281 [details]
To run by bots

Attachment 156281 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13426514

[Dominic Cooney]

Created attachment 156836 [details]
To run by bots

[Dominic Cooney]

Commited r125251.

[Darin Adler]

This change broke garbage collection in JavaScriptCore. See bug 132148 for details.