Demonstrate that Web Audio does not keep callbacks alive long enough.

Demonstrate that Web Audio does not keep callbacks alive long enough.

This is discussed on Bug 82975. Unfortunately, not keeping the
callback alive long enough encourages authors to implement a
workaround which is prone to leaks by retaining (and typically
leaking) a reference to the callback themselves.

BUG=82795
R=crogers@google.com

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=155194

[dominicc (has gone to gerrit), 2013-07-30 07:03:03]

PTAL

Chris--for context, this is to demonstrate the utility of a new function in the
test harness, observeGC, which is being reviewed in Issue 21046002.

This demonstrates the underlying issue with Bug 82795. I actually fixed this
once locally a long time ago, but found it impossible to write a test for.

It is still actually difficult to write a comprehensive test for this because of
the eagerness of the offline audio context. However this test demonstrates the
bug to some extent.

[dominicc (has gone to gerrit), 2013-07-30 07:06:06]

https://codereview.chromium.org/21216002/diff/1/LayoutTests/webaudio/javascri...
File LayoutTests/webaudio/javascriptaudionode-premature-death-expected.txt
(right):

https://codereview.chromium.org/21216002/diff/1/LayoutTests/webaudio/javascri...
LayoutTests/webaudio/javascriptaudionode-premature-death-expected.txt:6: FAIL
wasCollectedPrematurely should be false. Was true.
This is an expected failure. When bug 82795 is fixed this expectation should be
updated to 'pass'.

[Chris Rogers, 2013-07-30 18:55:54]

Dominic, thanks for looking at this.  lgtm with nits

https://codereview.chromium.org/21216002/diff/1/LayoutTests/webaudio/javascri...
File LayoutTests/webaudio/javascriptaudionode-premature-death.html (right):

https://codereview.chromium.org/21216002/diff/1/LayoutTests/webaudio/javascri...
LayoutTests/webaudio/javascriptaudionode-premature-death.html:13: /* channels:
*/ 2, /* length, frames: */ 512, /* sample rate: */ 44100.0);
Because of internal buffering used in the ScriptProcessorNode, I believe you'll
need to render for a longer time than 512 frames or the actual underlying engine
will not even attempt to invoke the callback --- I'd suggest 4096

https://codereview.chromium.org/21216002/diff/1/LayoutTests/webaudio/javascri...
LayoutTests/webaudio/javascriptaudionode-premature-death.html:18: /* source
channels: */ 2, /* length, frames: */ 512,
Similar to above comment - I'd increase 512 -> 4096, while leaving the
ScriptProcessorNode buffer at 512

https://codereview.chromium.org/21216002/diff/1/LayoutTests/webaudio/javascri...
LayoutTests/webaudio/javascriptaudionode-premature-death.html:22: var node =
context.createJavaScriptNode(
nit: the new method name is called "createScriptProcessor()"

https://codereview.chromium.org/21216002/diff/1/LayoutTests/webaudio/javascri...
LayoutTests/webaudio/javascriptaudionode-premature-death.html:25: // source ->
JavaScript audio node -> destination
"JavaScript audio node" -> "script processor node"

https://codereview.chromium.org/21216002/diff/1/LayoutTests/webaudio/javascri...
LayoutTests/webaudio/javascriptaudionode-premature-death.html:44:
source.noteOn(0);
new method name: noteOn() -> start()

[dominicc (has gone to gerrit), 2013-07-31 00:45:59]

Thanks for the feedback, Chris; I have improved the test.

Hopefully this test gives someone fixing the bug a headstart in verifying that
their fix works.

https://codereview.chromium.org/21216002/diff/1/LayoutTests/webaudio/javascri...
File LayoutTests/webaudio/javascriptaudionode-premature-death.html (right):

https://codereview.chromium.org/21216002/diff/1/LayoutTests/webaudio/javascri...
LayoutTests/webaudio/javascriptaudionode-premature-death.html:13: /* channels:
*/ 2, /* length, frames: */ 512, /* sample rate: */ 44100.0);
On 2013/07/30 18:55:54, Chris Rogers wrote:
> Because of internal buffering used in the ScriptProcessorNode, I believe
you'll
> need to render for a longer time than 512 frames or the actual underlying
engine
> will not even attempt to invoke the callback --- I'd suggest 4096

I cribbed these values from
LayoutTests/webaudio/resources/javascriptaudionode-testing.js and they work, but
your comment gave me an idea--

I have restructured this test into a function that gets called twice.

The first time, the test sets up a reference from the global to the
ScriptProcessorNode and the callback, which will obviously prevent GC.

The second time, it doesn't set up that reference.

The same set of assertions are run each time. This lets you verify that, yes,
the ScriptProcessorNode would have been called if it wasn't prematurely
collected.

I have also changed the buffer sizes to the values you suggest, maybe there are
implementation changes in the pipeline you have in mind. You should change
javascriptaudionode-testing.js in that case.

[dominicc (has gone to gerrit), 2013-07-31 00:47:02]

Committed patchset #2 manually as r155194 (presubmit successful).