Closed Bug 822094 Opened 7 years ago Closed 7 years ago
Implement transfer parameter of window
I'm a bit surprised that Firefox doesn't support it yet.
We can't post transferable to other window without this fix.
Comment on attachment 692733 [details] [diff] [review] Part 1: Unwrap the transfable parameter if allowed Review of attachment 692733 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsclone.cpp @@ +490,5 @@ > > JSObject* tObj = &v.toObject(); > + // If the object is a security wrapper, see if we're allowed to unwrap it. > + // If we aren't, throw. > + if (tObj->isWrapper()) Don't bother with the comment or the isWrapper() call. Just do JSObject* tObj = UnwrapObjectChecked(&v.toObject()); and do your null check. (The v.isObject() above already guarantees that the original object is non-null.) Also, if the security check fails, it would be more in line with existing code to do JS_ReportError(context(), "Permission denied to access object"); instead of reportErrorTransferable(). (I'm not saying the existing code is right, just that I'd rather match it for now.)
Attachment #692734 - Flags: review?(jonas) → review+
Resolved review comments.
Patch for check-in. I forgot updating the bug number after copy-and-paste, so I've done it now.
Attachment #693089 - Flags: review?(sphink) → review+
Assignee: nobody → VYV03354
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Apologies if this is already old news, but as of Aurora 19.0a2 (2012-12-20), passing an ArrayBuffer in the transfer parameter array neuters the ArrayBuffer but also passes a neutered version to the other thread. I have a simple test case for this, and can attach it to this issue or as a separate one if that would be helpful.
This is known (will be fixed by bug 789593) if you're passing a typed array with an ArrayBuffer in the transfer parameter array. It is not known if no typed arrays (or any kind of ArrayBufferView) is involved. If that is the case, can you give me a test case? I don't know how to set up worker-y stuff. Thanks!
My test case passes a Float32Array with the arrayInstance.buffer in the transferable object array. So it sounds like it's a known issue. Thanks!
It's a known issue, but nobody had actually needed to do it yet, so it's been languishing as a mentored bug for quite a while. Let me know if you need this to work, and I can prioritize that bug higher (i.e., give up and fix it myself.)
Transferable objects are a pretty big performance win in our app, but I think we should be able to pass the ArrayBuffer instead of the wrapping Float32Array as a fairly painless workaround. If so, there's no real hurry from me. I'll let you know of that doesn't work out for some reason. Thanks again.
(In reply to Steve Fink [:sfink] from comment #12) > It's a known issue, but nobody had actually needed to do it yet, so it's > been languishing as a mentored bug for quite a while. Let me know if you > need this to work, and I can prioritize that bug higher (i.e., give up and > fix it myself.) I'd love to take you up on that. I've been trying to work around the issue, but haven't found a satisfying way yet. In particular, I'm trying to pass an ImageData object (imageData) around, and as far as I can tell the only thing that is accepted by Firefox (and Chrome) is to specify imageData.data.buffer as transferable object, except that this results in the buffer ending up lost, inaccessible to either party. Also, as far as I can tell I cannot "reassemble" an ImageData object from just an ArrayBuffer, so the only viable option is to just let Firefox copy the object...
It's done and awaiting review in bug 789593.
You need to log in before you can comment on or make changes to this bug.