Closed Bug 1085060 Opened 5 years ago Closed 6 months ago

XMLHttpRequest throws when sending a Uint8Array from a web worker

Categories

(Core :: DOM: Workers, defect, P3)

36 Branch
x86
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: dmxoss42, Assigned: twisniewski, Mentored)

Details

(Whiteboard: [tw-dom][wptsync upstream])

Attachments

(3 files, 5 obsolete files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/37.0.2062.120 Safari/537.36

Steps to reproduce:

1. Open the attached testcase or visit it online at http://plnkr.co/edit/vooLd13AygHzTrQcrvYr
2. Click "Test Main Thread" to perform 3 POSTs using an XMLHttpRequest, one each for a string, Uint8Array, and ArrayBuffer. See the results below the buttons in the "Main Thread Results" section.
3. Click "Test Worker Thread" to do the same using a web worker. See the results in the "Worker Thread Results" section.

In addition to the 36.0a1 (2014-10-18) nightly, this is also seen in 32.0.3 and was observed on both Mac and Windows (64-bit) installations. Chrome 37, Safari 6.0.5, and Internet Explorer 11 pass all 6 test cases.


Actual results:

On the main thread, the XMLHttpRequest accepts all three data types as shown by the output "ok". In the web worker, the string and ArrayBuffer pass with "ok" but the Uint8Array fails with the exception: "Data conversion error"  nsresult: "0x80460001 (NS_ERROR_CANNOT_CONVERT_DATA)".


Expected results:

As the main thread succeeds for all three data types, the web worker should have succeeded for the types as well.
Attached file testcase - HTML page (obsolete) —
Attached file testcase - HTML Page (obsolete) —
Attachment #8507478 - Attachment is obsolete: true
Attached file testcase - HTML page (obsolete) —
Attachment #8507479 - Attachment is obsolete: true
Attached file testcase - HTML page (obsolete) —
Attachment #8507480 - Attachment is obsolete: true
Attached file testcase - HTML page (obsolete) —
Attachment #8507481 - Attachment is obsolete: true
This is a problem in the variant handling.  Attempting to turn the variant into a string is failing for some reason.
Status: UNCONFIRMED → NEW
Ever confirmed: true
XPCVariant doesn't handle TypedArrays.  It ends up creating an XPCWrappedJS for them.  It does have code to handle regular Arrays though.  Should this be something we can expect XPConnect to handle?
Flags: needinfo?(bobbyholley)
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #7)
> XPCVariant doesn't handle TypedArrays.  It ends up creating an XPCWrappedJS
> for them.  It does have code to handle regular Arrays though.  Should this
> be something we can expect XPConnect to handle?

We could add support yes, though I doubt that teaching XPConnect how to convert a TypedArray into an XPCOM array (which is what adding support in XPCVariant would mean) is actually what we want here. We just want to treat it as a jsval, which nsIVariant doesn't do.

Shouldn't SendRunnable::MainThreadRun create the appropriate RequestBody directly rather than using the nsIVariant stuff?
Flags: needinfo?(bobbyholley)
(In reply to Bobby Holley (:bholley) from comment #8)
> Shouldn't SendRunnable::MainThreadRun create the appropriate RequestBody
> directly rather than using the nsIVariant stuff?

Yeah.  I was asking about the XPConnect stuff mostly for completeness.
Assignee: khuey → nobody
This will be resolved by one of the patches pending to land in bug 792808.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 792808
Marion, the cleanup in bug 792808 doesn't seem like it's really going to land anytime.  Should we really block this web compat issue on that cleanup?  Seems like we should reopen and fix this on its own terms, unless we're going to push bug 792808 over the line.
Flags: needinfo?(mdaly)
You raise a good point.
Flags: needinfo?(mdaly)
Status: RESOLVED → REOPENED
Priority: -- → P3
Resolution: DUPLICATE → ---
bz, this is safe to close as a dupe of bug 792808 (now that it has landed), right?
Flags: needinfo?(bzbarsky)
Assuming the testcases here are fixed and we have wpt coverage for them, yes.
Flags: needinfo?(bzbarsky)

I just confirmed that the testcase here does work as expected now, and that there are relevant WPTs which can be easily refactored to also work with workers/sharedworkers. I'll request a review for that refactoring shortly.

update WPTs for XHR sends to also test with workers

Pushed by twisniewski@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/01cbcad8bb67
update WPTs for XHR sends to also test with workers; r=annevk
Status: REOPENED → RESOLVED
Closed: 2 years ago6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Assignee: nobody → twisniewski
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/15897 for changes under testing/web-platform/tests
Whiteboard: [tw-dom] → [tw-dom][wptsync upstream]
You need to log in before you can comment on or make changes to this bug.