[webkit-reviews] review denied: [Bug 22720] Make XMLHttpRequest work in Workers : [Attachment 27071] Part 6: Small adjustments to the ThreadableLoader::loadResourceSynchronously to make it easier to implement for Workers

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied David Levin
<levin at chromium.org>'s request for review:
Bug 22720: Make XMLHttpRequest work in Workers
https://bugs.webkit.org/show_bug.cgi?id=22720

Attachment 27071: Part 6: Small adjustments to the
ThreadableLoader::loadResourceSynchronously to make it easier to implement for
Workers
https://bugs.webkit.org/attachment.cgi?id=27071&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
You may be right, but I'm not convinced yet. We don't have special versions of
ResourceRequest and ResourceResponse for the threadable loader, so it feels
strange to have a special version of ResourceError. In fact, ResourceError is a
relatively new class, as we used to have multiple ad hoc error reporting
mechanisms similar to the one proposed here - and subtle mismatches between the
meanings of errors were an issue.

Our XMLHttpRequest implementation is not sufficiently mature when it comes to
error handling (see bug 17426, bug 13075 and some others), so it is quite
likely that more information from ResourceError will be needed.

I think that the names are not sufficiently descriptive - e.g. when you write
"if (!document->frame()) errorType = FailedLoaderError;" it seems that the
problem is that there is no loader, so I'd expect to see "NoLoaderError" here.
But that's not what NoLoaderError means!

r-, due to at least the names.