Bug 116871 – Made AudioNode an EventTarget

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

Made AudioNode an EventTarget

[Jer Noble]

Created attachment 203061 [details]
Patch

[WebKit Commit Bot]

Attachment 203061 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/webaudio/audionode-expected.txt', u'LayoutTests/webaudio/audionode.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/Modules/webaudio/AudioNode.cpp', u'Source/WebCore/Modules/webaudio/AudioNode.h', u'Source/WebCore/Modules/webaudio/AudioNode.idl', u'Source/WebCore/Modules/webaudio/ScriptProcessorNode.cpp', u'Source/WebCore/Modules/webaudio/ScriptProcessorNode.h', u'Source/WebCore/Modules/webaudio/ScriptProcessorNode.idl', u'Source/WebCore/dom/EventTarget.h', u'Source/WebCore/dom/EventTargetFactory.in']" exit_code: 1
Source/WebCore/dom/EventTarget.h:43:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 203061 [details]
Patch

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”.

[Jer Noble]

(In reply to comment #3)
> (From update of attachment 203061 [details])
> 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.

Yes, these can be moved into private:.

> > 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”.

Sure thing.  Thanks!

[Jer Noble]

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 203061 [details] [details])
> > 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.
> 
> Yes, these can be moved into private:.

I take that back.  They're called by the wrapper (JSAudioNode) directly, so they need to be public.

> > > 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”.
> 
> Sure thing.  Thanks!

[Jer Noble]

Committed r150810: <http://trac.webkit.org/changeset/150810>