[webkit-reviews] review granted: [Bug 88232] SVGElementInstance should have EventTarget on the prototype chain : [Attachment 146017] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Dominic Cooney
<dominicc at chromium.org>'s request for review:
Bug 88232: SVGElementInstance should have EventTarget on the prototype chain
https://bugs.webkit.org/show_bug.cgi?id=88232

Attachment 146017: Patch
https://bugs.webkit.org/attachment.cgi?id=146017&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
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