822094 - Implement transfer parameter of window.postMessage

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Masatoshi Kimura [:emk]]

I'm a bit surprised that Firefox doesn't support it yet.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: Part 1: Unwrap the transfable parameter if allowed

We can't post transferable to other window without this fix.

Comment 2

[Masatoshi Kimura [:emk]]

Attachment: Part 2: Add transfer parameter to window.postMessage

Comment 3

[Masatoshi Kimura [:emk]]

https://tbpl.mozilla.org/?tree=Try&rev=65c7343e374c

Comment 4

[Steve Fink [:sfink] [:s:]]

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.)

Comment 5

[Masatoshi Kimura [:emk]]

Attachment: Part 1: Unwrap the transfable parameter if allowed, v2

Resolved review comments.

Comment 6

[Masatoshi Kimura [:emk]]

Attachment: Part 2: Add transfer parameter to window.postMessage. r=jonas

Patch for check-in.
I forgot updating the bug number after copy-and-paste, so I've done it now.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/56f2d3fbcf49
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d5cc2e8c111

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/56f2d3fbcf49
https://hg.mozilla.org/mozilla-central/rev/7d5cc2e8c111

Comment 9

[Kevin Ring]

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.

Comment 10

[Steve Fink [:sfink] [:s:]]

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!

Comment 11

[Kevin Ring]

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!

Comment 12

[Steve Fink [:sfink] [:s:]]

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.)

Comment 13

[Kevin Ring]

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.

Comment 14

[Jasper]

(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...

Comment 15

[Steve Fink [:sfink] [:s:]]

It's done and awaiting review in bug 789593.

Comment 16

[Jean-Yves Perrier [:teoli]]

I've updated:
https://developer.mozilla.org/en-US/docs/Web/API/window.postMessage
and
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/20