[webkit-reviews] review denied: [Bug 109674] start and stop attributes in AudioBufferSourceNode should include "raises (exception)" in IDL files : [Attachment 188470] Patch

[bugzilla-daemon at webkit.org]

Kentaro Hara <haraken at chromium.org> has denied Praveen Jadhav
<praveen.j at samsung.com>'s request for review:
Bug 109674: start and stop attributes in AudioBufferSourceNode should include
"raises (exception)" in IDL files
https://bugs.webkit.org/show_bug.cgi?id=109674

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

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


> LayoutTests/webaudio/audiobuffersource-note.html:12
> +<div id="description"></div>
> +<div id="console"></div>

Nit: They are not needed.

> LayoutTests/webaudio/audiobuffersource-note.html:19
> +	   testRunner.dumpAsText();

Nit: This is not needed.

> LayoutTests/webaudio/audiobuffersource-note.html:23
> +	   testRunner.waitUntilDone();
> +    }
> +
> +    window.jsTestIsAsync = true;

This test needs not to be async.

> LayoutTests/webaudio/audiobuffersource-note.html:37
> +    try {
> +	   bufferSource.start(0);
> +	   buffersource.stop(0.5);
> +	   buffersource.stop(0.8);
> +	   testFailed("Exception should be thrown if stop is called twice.");
> +    } catch(e) {
> +	   testPassed("Exception been thrown as stop is called twice.");
> +    }

You can use shouldThrow() in js-test-pre.js.

> LayoutTests/webaudio/audiobuffersource-note.html:45
> +    try {
> +	   buffersource.start(0);
> +	   buffersource.start(0);
> +	   testFailed("Exception should be thrown if start is called twice.");
> +    } catch(e) {
> +	   testPassed("Exception been thrown as start is called twice.");
> +    }

Ditto.

> LayoutTests/webaudio/audiobuffersource-note.html:52
> +    try {
> +	   buffersource.stop(0);
> +	   testFailed("Exception should be thrown if stop is called before
start.");
> +    } catch(e) {
> +	   testPassed("Exception been thrown as stop is called before start.");

> +    }

Ditto.

> LayoutTests/webaudio/audiobuffersource-note.html:54
> +    finishJSTest();

Remove this. This test needs not to be async.

> LayoutTests/webaudio/audiobuffersource-note.html:57
> +runTest();

Nit: You can directly write test cases here instead of calling runTest().