[webkit-reviews] review denied: [Bug 27924] Need to test throwing exceptions from Workers after calling close() : [Attachment 33940] patch containing layout test

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Andrew Wilson
<atwilson at google.com>'s request for review:
Bug 27924: Need to test throwing exceptions from Workers after calling close()
https://bugs.webkit.org/show_bug.cgi?id=27924

Attachment 33940: patch containing layout test
https://bugs.webkit.org/attachment.cgi?id=33940&action=review

------- Additional Comments from David Levin <levin at chromium.org>
Just a few minor things to consider or address which will make it possible to
land this easily


> diff --git a/LayoutTests/fast/workers/resources/worker-close.js
b/LayoutTests/fast/workers/resources/worker-close.js
> @@ -22,6 +22,9 @@ onmessage = function(evt)
>	   postMessage("pong");
>      } else if (evt.data == "throw") {
>	   throw "should never be executed";
> +    } else if (evt.data == "closeWithError") {
> +	   close();
> +	   generateError();  // Undefined function - throws exception

Even time I see this I want to look for the generateError function.  Just an
idea but what do you think about calling it nonExistantFunction();



> diff --git a/LayoutTests/fast/workers/worker-close.html
b/LayoutTests/fast/workers/worker-close.html
> +    // Test that errors are delivered after close

Please end with "."

> +    worker = new Worker('resources/worker-close.js');
> +    worker.postMessage("closeWithError");
> +    worker.onerror = function(event) {
> +	   log("PASS: Error arrived after close: " + event.message);
> +	   testPendingEvents();
> +	return false;

Indentation is off.

> +    }
> +}