XMLHttpRequest throws when sending a Uint8Array from a web worker

RESOLVED FIXED in Firefox 67

Status

()

defect
P3
normal
RESOLVED FIXED
5 years ago
2 months ago

People

(Reporter: dmxoss42, Assigned: twisniewski, Mentored)

Tracking

36 Branch
mozilla67
x86
macOS
Points:
---

Firefox Tracking Flags

(firefox67 fixed)

Details

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

Attachments

(3 attachments, 5 obsolete attachments)

(Reporter)

Description

5 years ago
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.
(Reporter)

Comment 1

5 years ago
Posted file testcase - HTML page (obsolete) —
(Reporter)

Comment 2

5 years ago
Posted file testcase - HTML Page (obsolete) —
Attachment #8507478 - Attachment is obsolete: true
(Reporter)

Comment 3

5 years ago
Posted file testcase - HTML page (obsolete) —
Attachment #8507479 - Attachment is obsolete: true
(Reporter)

Comment 4

5 years ago
Posted file testcase - HTML page (obsolete) —
Attachment #8507480 - Attachment is obsolete: true
(Reporter)

Comment 5

5 years ago
Posted 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
Last Resolved: a year 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 → ---
(Assignee)

Comment 15

10 months ago
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)
(Assignee)

Comment 17

2 months ago

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.

(Assignee)

Comment 18

2 months ago

update WPTs for XHR sends to also test with workers

Comment 20

2 months ago
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

Comment 21

2 months ago
bugherder
Status: REOPENED → RESOLVED
Last Resolved: a year ago2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
(Assignee)

Updated

2 months ago
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.