[webkit-reviews] review granted: [Bug 116871] Made AudioNode an EventTarget : [Attachment 203061] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jer Noble <jer.noble at apple.com>'s
request for review:
Bug 116871: Made AudioNode an EventTarget
https://bugs.webkit.org/show_bug.cgi?id=116871

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=203061&action=review


> Source/WebCore/Modules/webaudio/AudioNode.h:186
> +    // EventTarget
> +    virtual const AtomicString& interfaceName() const OVERRIDE;
> +    virtual ScriptExecutionContext* scriptExecutionContext() const OVERRIDE;

> +    virtual EventTargetData* eventTargetData() OVERRIDE { return
&m_eventTargetData; }
> +    virtual EventTargetData* ensureEventTargetData() OVERRIDE { return
&m_eventTargetData; }

Could these overrides be private? I can’t think of any reason someone would
call these on an AudioNode directly rather than through an EventTarget&, so
making them private would be nice.

> Source/WebCore/Modules/webaudio/AudioNode.idl:62
> +    void addEventListener(DOMString type,
> +	   EventListener listener,
> +	   optional boolean useCapture);
> +
> +    void removeEventListener(DOMString type,
> +	   EventListener listener,
> +	   optional boolean useCapture);
> +
> +    boolean dispatchEvent(Event evt)
> +	   raises(EventException);

These read better on single lines, to my taste. And I see no reason to name it
“evt” instead of event”.