Bug 24330 – Sync xhr in workers should send an abort exception when the worker is terminated.

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[David Levin]

See summary.

[David Levin]

Created attachment 28232 [details]
Proposed fix.


2009-03-03  David Levin  <[e-mail]>

        Reviewed by NOBODY (OOPS!).

        Bug 24330: Sync xhr in workers should send abort error when the worker is terminated.
        <https://bugs.webkit.org/show_bug.cgi?id=24330>

        Test: http/tests/xmlhttprequest/workers/abort-error-assert.html

        * dom/ExceptionCode.cpp:
        (WebCore::xmlHttpRequestExceptionNames):
        Added missing ABORT_ERR whse absence caused an assert.

        * loader/WorkerThreadableLoader.cpp:
        (WebCore::WorkerThreadableLoader::loadResourceSynchronously):
        (WebCore::WorkerThreadableLoader::MainThreadBridge::cancel):
        Add more logic to handle the termination case for sync xhr.

LayoutTests:

2009-03-03  David Levin  <[e-mail]>

        Reviewed by NOBODY (OOPS!).

        Bug 24330: Sync xhr in workers should send abort error when the worker is terminated.
        <https://bugs.webkit.org/show_bug.cgi?id=24330>

        Added test to verify to terminate a worker while it is doing a synchronous xhr.
        It does not verify that the exception is a ABORT_ERR because I couldn't figure out a way
        to actually do this.

        * http/tests/xmlhttprequest/workers/abort-error-assert-expected.txt: Added.
        * http/tests/xmlhttprequest/workers/abort-error-assert.html: Added.
        * http/tests/xmlhttprequest/workers/resources/endless-response.php: Copied from LayoutTests/http/tests/xmlhttprequest/resources/endlessxml.php.
        * http/tests/xmlhttprequest/workers/resources/endless-sync-xhr.js: Added.
---
 LayoutTests/ChangeLog                              |   16 +++++++++
 .../workers/abort-error-assert-expected.txt        |    5 +++
 .../xmlhttprequest/workers/abort-error-assert.html |   34 ++++++++++++++++++++
 .../workers/resources/endless-response.php         |    3 ++
 .../workers/resources/endless-sync-xhr.js          |    7 ++++
 WebCore/ChangeLog                                  |   18 ++++++++++
 WebCore/dom/ExceptionCode.cpp                      |    3 +-
 WebCore/loader/WorkerThreadableLoader.cpp          |   18 +++++++++-
 8 files changed, 101 insertions(+), 3 deletions(-)

[Alexey Proskuryakov]

Comment on attachment 28232 [details]
Proposed fix.

+        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

[David Levin]

Committed as r41433.