[webkit-reviews] review granted: [Bug 112521] ScriptProcessorNode is garbage collected while still active if unreachable : [Attachment 193487] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has granted Russell McClellan
<russell.mcclellan at gmail.com>'s request for review:
Bug 112521: ScriptProcessorNode is garbage collected while still active if
unreachable
https://bugs.webkit.org/show_bug.cgi?id=112521

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

------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=193487&action=review


Looks good with nits.

> Source/WebCore/ChangeLog:10
> +	   Fix for issue where ScriptProcessorNodes (and AudioNode js wrappers
generally)
> +	   would be garbage collected before their time.  Made AudioNode an
ActiveDOMElement
> +	   marked pending if there are any open audio connections.
> +	   https://bugs.webkit.org/show_bug.cgi?id=112521
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Test: webaudio/javascriptaudionode.html

Nit: WebKit convention is a bug title (which should be concise), a bug URL, a
reviewer, a description, and a test.

> LayoutTests/webaudio/javascriptaudionode.html:89
> +	     testFailed("onaudioprocess was called not called on unreachable
but connected script node.");

Typo: was not called

> LayoutTests/webaudio/javascriptaudionode.html:90
> +	   if(audioprocessWasCalled) {
> +	     testPassed("onaudioprocess was called even though node was
unreachable.");
> +	   } else {
> +	     testFailed("onaudioprocess was called not called on unreachable
but connected script node.");
> +	   }

This can simply be shouldBeTrue('audioprocessWasCalled');