[webkit-reviews] review granted: [Bug 116798] Support the 'onended' EventListener property for AudioBufferSourceNode and OscillatorNode. : [Attachment 203071] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 116798: Support the 'onended' EventListener property for
AudioBufferSourceNode and OscillatorNode.
https://bugs.webkit.org/show_bug.cgi?id=116798

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

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=203071&action=review


> Source/WebCore/ChangeLog:18
> +	   (WebCore::AudioScheduledSourceNode::onended): Added; boilerplate.

Might want to remove this inadvertent semicolon too (which is an equally bad
band name, even when spelled correctly :¬) ).

> Source/WebCore/Modules/webaudio/AudioScheduledSourceNode.cpp:200
> +    if (!onended())
> +	   return;
> +
> +    RefPtr<Event> event = Event::create(eventNames().endedEvent, FALSE,
FALSE);
> +    event->setTarget(this);
> +    onended()->handleEvent(context()->scriptExecutionContext(),
event.get());

You might as well cache the listener in a local instead of calling onended()
twice because getAttributeEventListener() does a linear search of all listeners
so it can be relative expensive.

> LayoutTests/webaudio/audiobuffersource-ended.html:10
> +	   function runTest() {

A function's opening brace should be on a new line.

> LayoutTests/webaudio/audiobuffersource-ended.html:22
> +	       source.onended = function() {

Ditto.

> LayoutTests/webaudio/oscillator-ended.html:10
> +	   function runTest() {

Ditto.

> LayoutTests/webaudio/oscillator-ended.html:22
> +	       osc.onended = function() {

Ditto.