[Webkit-unassigned] [Bug 93321] Exception should be thrown when connect() creates a cycle without DelayNode

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 13 15:59:22 PDT 2012


--- Comment #2 from Raymond Toy <rtoy at chromium.org>  2012-08-13 15:59:51 PST ---
(From update of attachment 156833)
View in context: https://bugs.webkit.org/attachment.cgi?id=156833&action=review

Thanks for the nice patch to fix this issue.  Just a few small comments.

> Source/WebCore/Modules/webaudio/AudioNode.cpp:73
> +bool AudioNode::checkCycle(AudioNode* startNode, AudioNode* endNode)

Can we make checkCycle more explicit.  Maybe checkForDelayFreeCycle?

Perhaps a short comment can also be added saying we're looking for a path between startNode and endNode that does not contain a delayNode.

Also, have you considered what happens if the delayNode has 0 delay?  Convolvers could also have an arbitrarily long delay. (These are probably outside the scope of this fix.)

> LayoutTests/webaudio/audionode-connect-cycle-expected.txt:1
> +This tests that we should throw an exception if a cycle is created without DelayNode in it.

Typos:  "we should throw" -> "we throw" and "without DelayNode" -> "without a DelayNode"

> LayoutTests/webaudio/audionode-connect-cycle-expected.txt:5
> +PASS connect() is OK.

These aren't very informative.  Is there an easy way to provide a little more context?

> LayoutTests/webaudio/audionode-connect-cycle.html:43
> +    

Is there any significance to the fact that the audio context length is 411 but the buffer length is 441?  Or did you intend both to be 0.01 sec long?

Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

More information about the webkit-unassigned mailing list