Bug 27924 – Need to test throwing exceptions from Workers after calling close()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Andrew Wilson]

After we call close(), any exceptions thrown from Worker context should still be propagated back to page context via the Worker.onerror handler.

This seems to work properly, but we should add a layout test to confirm this.

[Andrew Wilson]

Created attachment 33940 [details]
patch containing layout test

Patch only contains layout tests, since the code seems to work as-is.

[David Levin]

Comment on attachment 33940 [details]
patch containing layout test

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.

> +    }
> +}

[Andrew Wilson]

Created attachment 34077 [details]
Patch addressing levin's comments

That indentation error was due to a tab somehow getting into the code. Weird. Maybe I used vi to edit a file inadvertently...

[Adam Barth]

Comment on attachment 34077 [details]
Patch addressing levin's comments

Clearing review flag on attachment: 34077

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/fast/workers/resources/worker-close.js
	M	LayoutTests/fast/workers/worker-close-expected.txt
	M	LayoutTests/fast/workers/worker-close.html
Committed r46830
	M	LayoutTests/ChangeLog
	M	LayoutTests/fast/workers/worker-close.html
	M	LayoutTests/fast/workers/worker-close-expected.txt
	M	LayoutTests/fast/workers/resources/worker-close.js
r46830 = 515c284335367a97d1e0796b7395e6c92eeaea89 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46830

[Adam Barth]

All reviewed patches have been landed.  Closing bug.