DelayNode must set the context on delayTime AudioParam to support automation
Created attachment 108768 [details] Patch
Comment on attachment 108768 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108768&action=review The change and tests look good but there's a cleanup I think you should make to the tests. > LayoutTests/webaudio/resources/delay-testing.js:4 > +var toneBuffer; These globals make the code difficult to understand. I think you should remove them. Suggestions below on how to do so. > LayoutTests/webaudio/resources/delay-testing.js:10 > +function createToneBuffer(frequency, numberOfCycles, sampleRate) { Change this to accept context as an argument. > LayoutTests/webaudio/resources/delay-testing.js:25 > +function checkDelayedResult(event) { Take the toneBuffer as an argument to this function and return a function taking the event: function checkDelayedResult(toneBuffer) { return function(event) { // ... } } Then in the tests, set context.oncomplete = checkDelayedResult(toneBuffer).
Created attachment 109082 [details] Patch
Ken, thanks for having a look. I've refactored as you suggested...
Comment on attachment 109082 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109082&action=review Looks good. Minor cleanup in the tests is still desired but feel free to do this upon landing the patch. r=me > LayoutTests/webaudio/delaynode-scheduling.html:18 > +var context; This doesn't need to be here. > LayoutTests/webaudio/delaynode-scheduling.html:29 > + context = new webkitAudioContext(1, sampleRate * renderLengthSeconds, sampleRate); You can just use "var context" here. > LayoutTests/webaudio/delaynode.html:18 > var context; Same change here -- no global variable needed. > LayoutTests/webaudio/delaynode.html:29 > context = new webkitAudioContext(1, sampleRate * renderLengthSeconds, sampleRate); Can use local variable declaration here too.
Committed r96270: <http://trac.webkit.org/changeset/96270>