It is possible for (non-worker) XMLHttpRequest::Send to return a non-NS_OK value. For example, in bug 827243, NS_ERROR_FILE_NOT_FOUND can be returned because AsyncOpen may be able to resolve that a file does not exist in an app:// protocol jar in a synchronous fasion. (worker) XMLHttpRequest::SendInternal currently returns when !runnable->Dispatch(cx) without modifying aRv, and as a result the error is not propagated to the caller. However, JS_SetPendingException does get called, so an exception will be tracked on the context, and can end up showing up in misleading places. For example, :jrburke observed that we might observe the error in importScripts if we tried to perform a synchronous XHR request. The attached patch created with the guidance of bent resolves the problem affecting the gaia e-mail app. The specific diff provided is for mozilla-central with crazy context, but the patch applies to mozilla-b2g18 (at least with a smaller context). I have pushed the patch to try at: https://tbpl.mozilla.org/?tree=Try&rev=1757a26ab7b4 This blocks the tef+ bug 814257 to move most of the e-mail app's backend to a worker thread and should therefore be tracked as a blocker and allowed to uplift to v1.0.1 and v1-train. It's a 1-line fix to properly propagate an error code, so I would argue that it is low risk, although bent can be more authoritative on the matter.
Attachment #732617 - Flags: review?(bent.mozilla)
Comment on attachment 732617 [details] [diff] [review] fix as discussed with bent; modify aRV Review of attachment 732617 [details] [diff] [review]: ----------------------------------------------------------------- r=me. Sorry about that!
Attachment #732617 - Flags: review?(bent.mozilla) → review+
landed on mozilla-inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/e418e5123168
tef+, blocks a blocker
blocking-b2g: tef? → tef+
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Andrew, can you comment on how QA can verify this patch?
(In reply to Tony Chung [:tchung] from comment #6) > Andrew, can you comment on how QA can verify this patch? I wouldn't specifically test this patch, but you want to make sure you have a moztrap case that covers creating an account that's not one of the domain names in here: https://github.com/mozilla-b2g/gaia/tree/master/apps/email/autoconfig Without the patch, that will fail *when running with the back-end on worker-thread from bug 814257*.
You need to log in before you can comment on or make changes to this bug.