[webkit-reviews] review granted: [Bug 24330] Sync xhr in workers should send abort error when the worker is terminated. : [Attachment 28232] Proposed fix.

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted David Levin
<levin at chromium.org>'s request for review:
Bug 24330: Sync xhr in workers should send abort error when the worker is
terminated.
https://bugs.webkit.org/show_bug.cgi?id=24330

Attachment 28232: Proposed fix.
https://bugs.webkit.org/attachment.cgi?id=28232&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
+	 Bug 24330: Sync xhr in workers should send abort error when the worker
is terminated.

"Should raise an exception" maybe? But what really counts is that we shouldn't
keep waiting for load to finish, as the various ways to stop execution are
indistinguishable.

+	 // This is a small race condition here in that the worker may not have
started the sync xhr call at this point,
+	 // but I couldn't find a way to eliminate it.

Maybe wait 100ms to make it less likely?

\ No newline at end of file

Please add one.

+    if (!loader->done() && result == MessageQueueTerminated) {
+	 loader->cancel();
+    }

No braces around single line blocks.

+	 // If the client hasn't finished at this point, then send a
cancellation error because there will be no
+	 // more callbacks from the main thread.

As discussed on IRC, this comment could be better.

r=me