http://dev.w3.org/html5/spec/common-dom-interfaces.html#transferable-objects https://developer.mozilla.org/En/Using_web_workers#Passing_data_by_transferring_.C2.A0ownership_(transferable_objects) "additional way to pass data to/from a worker using Transferable Objects with high performance. With transferable objects, data is transferred from one context to another with a zero-copy operation. This means a vast performance improvement when sending large data. Think of it as pass-by-reference if you're from the C/C++ world. However, unlike pass-by-reference, the 'version' from the calling context is no longer available once transferred. It's ownership is transferred to the new context." Supported in Chrome 17 (under a prefixed webkitPostMessage method for now)

Do we have plans for this?

Yes, I think Andrea is going to work on this. However, we need bug 720949 first.

This patch changes many things: 1. PostSyncMessage and PostMessage have an optional argument that is an array of arraybuffers. This is the transferable array object. http://www.whatwg.org/specs/web-apps/current-work/multipage/web-messaging.html#posting-messages http://updates.html5rocks.com/2011/12/Transferable-Objects-Lightning-Fast 2. JSStructuredCloneBuffer supports the transferable array. - it validates the jsval transferable array object and if something goes wrong, it uses reportError() callback with errorId == JS_SCERR_TRANSFERABLE - the buffer created by the JSStructuredCloneBuffer starts with a list of SCTAG_TRANSFER_MAP (only if the buffer contains some transferred object). **This makes easier to know if the buffer contains transferable objects** - The algorithm works as described here: http://dev.w3.org/html5/spec/common-dom-interfaces.html#transferable-object - A JSStructuredCloneBuffer with transferred objects cannot be copied. - Multiple read() calls are not allowed for JSStructuredCloneBuffer with transferred objects - The memory is deallocated by the JSStructuredCloneBuffer destructor if any read() is called. This patch builds on top of https://bugzilla.mozilla.org/show_bug.cgi?id=720949 Note: I'm not sure about writePtr/readPtr for 32/64bits and bit/little endian byte order.

Comment on attachment 657289 [details] [diff] [review] patch 1 Review of attachment 657289 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsStructuredCloneContainer.cpp @@ +62,2 @@ > bool success = JS_WriteStructuredClone(aCx, jsData, &jsBytes, &mSize, > + nullptr, nullptr, transferable); Why not pass in JSVAL_VOID directly? ::: dom/workers/test/sync_transferable_worker.js @@ +5,5 @@ > + > +onmessage = function(event) { > + if (event.data.data == 0) { > + try { > + ab = new ArrayBuffer(128); Can you test very small buffers (eg 4 bytes) too? They'll start out with their data inline. (To be fair, it would be testing the transferable typedarray JSAPI, not the cloning.) ::: js/src/jsapi.cpp @@ +6482,5 @@ > bool > JSAutoStructuredCloneBuffer::copy(const uint64_t *srcData, size_t nbytes, uint32_t version) > { > + // transferable objects cannot be copied > + if (data_ && StructuredCloneHasTransferObjects(data_, nbytes_)) { no curly brackets here If you're checking data_ here instead of inside StructuredCloneHasTransferObjects, it seems like you'd want to check whether nbytes_ is big enough too. ::: js/src/jsclone.cpp @@ +217,5 @@ > +bool > +SCInput::getPair(uint32_t *tagp, uint32_t *datap) > +{ > + uint64_t u = 0; /* initialize to shut GCC up */ > + bool ok = get(&u); I think in spidermonkey-land, you'd spell this uint64_t u; if (!get(&u)) return false; ... return true; @@ +421,5 @@ > +JSStructuredCloneWriter::parseTransferable() > +{ > + transferableObjects.clear(); > + > + if (!JSVAL_IS_NULL(transferable) && !JSVAL_IS_VOID(transferable)) { Similarly, just do an |if (null or void) return true| early exit. @@ +427,5 @@ > + reportErrorTransferable(); > + return false; > + } > + > + JSObject* array = JSVAL_TO_OBJECT(transferable); array = &transferable.toObject() @@ +439,5 @@ > + return false; > + } > + > + for (uint32_t i = 0; i < length; ++i) { > + jsval v; Value v; @@ +449,5 @@ > + reportErrorTransferable(); > + return false; > + } > + > + JSObject* tObj = JSVAL_TO_OBJECT(v); &v.toObject() @@ +450,5 @@ > + return false; > + } > + > + JSObject* tObj = JSVAL_TO_OBJECT(v); > + if (!tObj || !tObj->isArrayBuffer()) { !tObj is unnecessary; v.isObject() doesn't allow NULL (unlike the old JSVAL_IS_OBJECT). @@ +455,5 @@ > + reportErrorTransferable(); > + return false; > + } > + > + // No duplicate: Ugh. This really wants to be a map. It probably wouldn't even be much more code. And jsclone already has CloneMemory to use as an example, though you'd probably want HashSet instead of HashMap. @@ +574,5 @@ > + > + // Transferable Array Buffer: > + if (!transferableObjects.empty()) { > + CloneMemory::AddPtr p = memory.lookupForAdd(obj); > + if (p) { no brackets for single-line consequents in js/src @@ +687,5 @@ > + if (!memory.put(*begin, memory.count())) > + return false; > + > + uint8_t *content; > + if (!JS_StealArrayBufferContents(context(), *begin, &content)) I changed this parameter to a void** as a result of luke's review, so content can now be void*. @@ +1059,5 @@ > bool > +JSStructuredCloneReader::readTransferMap() > +{ > + uint32_t tag, data; > + while (in.getPair(&tag, &data) && tag == SCTAG_TRANSFER_MAP) { readTransferMap() returns true if in.getPair() fails. Seems wrong. @@ +1060,5 @@ > +JSStructuredCloneReader::readTransferMap() > +{ > + uint32_t tag, data; > + while (in.getPair(&tag, &data) && tag == SCTAG_TRANSFER_MAP) { > + uint8_t *content; void* ::: js/src/shell/js.cpp @@ +3427,5 @@ > jsval v = argc > 0 ? JS_ARGV(cx, vp)[0] : JSVAL_VOID; > uint64_t *datap; > size_t nbytes; > + jsval t(JSVAL_VOID); > + if (!JS_WriteStructuredClone(cx, v, &datap, &nbytes, NULL, NULL, t)) Why the separate variable? Why not pass in JSVAL_VOID directly?

This new patch does: 1. test for 1024*1024*32, 128 and 4 bytes 2. HashSet instead Vector 3. Any other comment from Steve's review 4. the previous patch was green on try. I'll post the results of this new one soon.

Comment on attachment 657543 [details] [diff] [review] patch 2 Review of attachment 657543 [details] [diff] [review]: ----------------------------------------------------------------- Everything looks good except for apparently pre-existing problems, and you don't have to fix those if you're not making them worse (though you're welcome to!), but I can't r+ the |if (!transferableObject.empty())| until I understand what it's supposed to be doing. So perhaps just an explanation is required for that part. ::: js/src/jsapi.h @@ +5849,5 @@ > /* Note: On success, the caller is responsible for calling js::Foreground::free(*datap). */ > JS_PUBLIC_API(JSBool) > JS_WriteStructuredClone(JSContext *cx, jsval v, uint64_t **datap, size_t *nbytesp, > const JSStructuredCloneCallbacks *optionalCallbacks, > + void *closure, jsval transferable); Don't you need a JS_ClearStructuredClone to avoid memory leaks when handling errors? You have JSAutoStructuredClone::clear(), but how do you free transferables within clone buffers when you're not using that class? ::: js/src/jsclone.cpp @@ +465,5 @@ > + reportErrorTransferable(); > + return false; > + } > + > + transferableObjects.putNew(tObj); This is not infallible, so you'll need to check the return value. @@ +572,5 @@ > ArrayBufferObject &buffer = obj->asArrayBuffer(); > + > + // Transferable Array Buffer: > + if (!transferableObjects.empty()) { > + CloneMemory::AddPtr p = memory.lookupForAdd(obj); You're not adding, right? Seems like this should be CloneMemory::Ptr p = memory.lookup(obj); But also, I'm confused by the logic here. Why check !transferableObjects.empty()? If you have a non-transferred ArrayBuffer, you'd still want to do the |memory| check, no? Only you're also not adding objects to |memory| anywhere here like::startObject does; why not? Shouldn't multiple references to the same ArrayBuffer end up pointing to the same thing? (This question is also valid for the code before your change, btw.) And if transferrable array buffers should indeed be treated differently, why aren't you doing transferableObjects.has(obj) instead of just checking whether anything at all is supposed to be transferred? Same problem with typed arrays. Structured cloning just seems broken (before this patch) for array buffers and typed arrays. Not to mention that DataViews are treated as plain objects... argh. @@ +993,5 @@ > } > > + case SCTAG_TRANSFER_MAP: > + // A map cannot be here but just at the beginning of the buffer. > + JS_ASSERT(false); An assertion seems extreme for actual use. Shouldn't this just be reported as bad serialized data?

The pre-existing problem with backreferences is bug 748309. Seems like there was a spec change that added the dag/cycle handling, and our implementation was never updated.

2 new functions should fix the problem for the allocated memory. /* Note: set freeTransferable to true if the data has never been read */ JSBool JS_ClearStructuredClone(const uint64_t *data, size_t nbytes, JSBool freeTransferable); JSBool JS_StructuredCloneHasTransferables(const uint64_t *data, size_t nbytes,JSBool *hasTransferable); For the other bug, I prefer to fix it in a separated patch.

JS_ClearStructuredBuffer used everywhere.

I meant: JS_ClearStructuredBuffer is used everywhere for freeing the buffers.

Carrying over Martin Best's [games:p2] whiteboard annotation from bug 735474.

I wrote a demo/test/benchmark: https://people.mozilla.com/~amarchesini/code/transferrable/ I think the performances are good: transferrable: postMessage roundtrip took: 29 ms transferrable: postMessage roundtrip rate: 2207 MB/s non-transferrable: postMessage roundtrip took: 127 ms non-transferrable: postMessage roundtrip rate: 504 MB/s

Comment on attachment 658405 [details] [diff] [review] patch 3a Review of attachment 658405 [details] [diff] [review]: ----------------------------------------------------------------- There are two bugs that this is related to, but it looks like the performance hit is going to be avoidable (bug 789295) and I can fix up the object cloning after this lands (bug 748309), so I see no reason not to let this land. Sorry for the delay. ::: js/src/jsclone.cpp @@ +132,5 @@ > +{ > + SCInput in(cx, data, nbytes); > + > + /* XXX disallow callers from using internal pointers to GC things. */ > + SkipRoot skip(cx, &in); Ouch. Properly rooting this is going to be a pain. Not your problem, though; this is preexisting. @@ +582,5 @@ > + CloneMemory::Ptr p = memory.lookup(obj); > + if (p) > + return out.writePair(SCTAG_BACK_REFERENCE_OBJECT, p->value); > + } > + Ok, with the understanding that the current code barely handles repeated objects at all currently, this makes sense. @@ +1002,5 @@ > + // A map cannot be here but just at the beginning of the buffer. > + JS_ReportErrorNumber(context(), js_GetErrorMessage, NULL, > + JSMSG_SC_BAD_SERIALIZED_DATA, > + "invalid input"); > + break; return false

Thanks for the review. We cannot land this patch yet because it's based on Bug 748309 - Typed Array structured clone doesn't do back-references. If you think it's OK, I can rebase this patch without including your patch for 748309. Let me know.

(In reply to Andrea Marchesini (:baku) from comment #15) > Thanks for the review. > We cannot land this patch yet because it's based on Bug 748309 - Typed Array > structured clone doesn't do back-references. > > If you think it's OK, I can rebase this patch without including your patch > for 748309. Let me know. Oh. I have a newer version of that patch that seems to work, unlike the one currently attached. But I'm still working on modifying the tests for it -- they're still testing the old behavior. I am fine with you rebasing this patch so that it is not on top of 748309, but that shouldn't affect bent's review. I think we'll have to race -- if you get bent's review first, then you can rebase and land. If I both finish bug 748309 and get review on it first, I'll land that and you can rebase on top of it. Does that work? Regardless, I'll update the patch in bug 748309 right now.

> I am fine with you rebasing this patch so that it is not on top of 748309, > but that shouldn't affect bent's review. I think we'll have to race -- if > you get bent's review first, then you can rebase and land. If I both finish > bug 748309 and get review on it first, I'll land that and you can rebase on > top of it. Does that work? Sounds like a plan.

Comment on attachment 658405 [details] [diff] [review] patch 3a Kyle can you review this patch? I want to see this code landed :) Thanks!

Comment on attachment 658405 [details] [diff] [review] patch 3a Review of attachment 658405 [details] [diff] [review]: ----------------------------------------------------------------- r=me on the worker bits. ::: dom/workers/Worker.cpp @@ +290,5 @@ > if (!JS_ConvertArguments(aCx, aArgc, JS_ARGV(aCx, aVp), "v", &message)) { > return false; > } > > + // FIXME: the targetOrigin (argv[1]) is currently ignored That seems bad. Is there a bug on file for this? ::: js/src/jsapi.h @@ +5856,5 @@ > + void *closure, jsval transferable); > + > +/* Note: set freeTransferable to true if the data has never been read */ > +JS_PUBLIC_API(JSBool) > +JS_ClearStructuredClone(const uint64_t *data, size_t nbytes, JSBool freeTransferable); I'm not a JS peer, but magic booleans are kind of annoying in APIs, since you end up having to look up the API to figure out what the boolean means. Could we make this an enum instead?

One question I do have, is it possible for the embedding to specify that objects are transferable somehow?

yes, it's possible do specify which object you want to have transferred: var ab = new ArrayBuffer(42); var ab2 = new ArrayBuffer(42); postMessage({a: ab, b: ab2 }, [ab]); just ab will be transferred. Can I mark this patch to be checked in? Or should I ask for another review?

I think you should check this in and talk to sfink about changing the booleans in the API to enums in another bug.

https://tbpl.mozilla.org/?tree=Try&rev=e557dc49ee49 /me waiting for green and comments if needed.

https://tbpl.mozilla.org/?tree=Try&rev=e849927f2709 test_xhr_timeout failed because of the second argument of the postMessage.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #20) > One question I do have, is it possible for the embedding to specify that > objects are transferable somehow? (In reply to Andrea Marchesini (:baku) from comment #21) > Created attachment 664858 [details] [diff] [review] > patch 3c > > yes, it's possible do specify which object you want to have transferred: > > var ab = new ArrayBuffer(42); > var ab2 = new ArrayBuffer(42); > postMessage({a: ab, b: ab2 }, [ab]); > > just ab will be transferred. Was that you were asking, khuey? I thought you were asking if there's a general mechanism for an embedding to label certain types of objects as Transferable, and therefore eligible for inclusion in the 2nd argument to postMessage(). The answer is no, currently; the ArrayBuffer handling is currently hardcoded into the clone algorithm. I guess we'd need to add another function pointer or 3 to JSClass or something if we wanted to generalize it. I don't see that happening until we have a need for it. Do you have a need for it, or am I just making up my own questions here? :-) baku: have you rebased this on top of bug 748309? I won the race, and I thought I remembered that this patch would need to change to accommodate it. I want to look at this again, at any rate -- can you flag me for re-review once you've rebased on bug 748309 (or confirmed that you already have)?

(In reply to Steve Fink [:sfink] from comment #25) > (In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #20) > > One question I do have, is it possible for the embedding to specify that > > objects are transferable somehow? > > (In reply to Andrea Marchesini (:baku) from comment #21) > > Created attachment 664858 [details] [diff] [review] > > patch 3c > > > > yes, it's possible do specify which object you want to have transferred: > > > > var ab = new ArrayBuffer(42); > > var ab2 = new ArrayBuffer(42); > > postMessage({a: ab, b: ab2 }, [ab]); > > > > just ab will be transferred. > > Was that you were asking, khuey? I thought you were asking if there's a > general mechanism for an embedding to label certain types of objects as > Transferable, and therefore eligible for inclusion in the 2nd argument to > postMessage(). The answer is no, currently; the ArrayBuffer handling is > currently hardcoded into the clone algorithm. I guess we'd need to add > another function pointer or 3 to JSClass or something if we wanted to > generalize it. I don't see that happening until we have a need for it. Do > you have a need for it, or am I just making up my own questions here? :-) Yes, you are correct about the question I posed. I don't have a need for it, I was just wondering, since handling Gecko objects in the structured clone hooks requires a bunch of logic (to ensure that they live long enough, that we delete them when they're no longer needed, etc).

> baku: have you rebased this on top of bug 748309? I won the race, and I > thought I remembered that this patch would need to change to accommodate it. > I want to look at this again, at any rate -- can you flag me for re-review > once you've rebased on bug 748309 (or confirmed that you already have)? I confirm this patch has been rebased to the current central repo.

Comment on attachment 664963 [details] [diff] [review] patch 3d Review of attachment 664963 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsStructuredCloneContainer.cpp @@ +63,5 @@ > NS_ENSURE_STATE(success); > NS_ENSURE_STATE(jsBytes); > > // Copy jsBytes into our own buffer. > mData = (uint64_t*) malloc(mSize); Nothing to do with your patch, I'm just curious: Yuck! Is this really necessary? Is this just to avoid mixing up allocators/deallocators? If so, we probably ought to file a bug to pass in an allocator or something. @@ +68,5 @@ > if (!mData) { > mSize = 0; > mVersion = 0; > > + JS_ClearStructuredClone(jsBytes, mSize, false); (hm, Splinter review doesn't seem to give me an option for commenting on a file, just a chunk) The error handling in nsStructuredCloneContainer::DeserializeToVariant leaks. I think that instead of just NS_ENSURE_STATE(success), you need to free the structured clone data on failure. Also, see the comments in jsapi.h. I think you want to leave these as JS_free. ::: js/src/jsapi.h @@ +2038,5 @@ > * This is called when JS_WriteStructuredClone finds that the object to be > + * written is recursive or when the transferable object is not valid. > + * To follow HTML5, the application must throw a > + * DATA_CLONE_ERR DOMException. errorid is always JS_SCERR_RECURSION or > + * JS_SCERR_TRANSFERABLE. Oops, I should have removed this. Can you change the comment to just say: This is called when JS_WriteStructuredClone is given an invalid transferable. To follow HTML5, the application must throw a DATA_CLONE_ERR DOMException with error set to one of the JS_SCERR_* values. @@ +5718,5 @@ > + void *closure, jsval transferable); > + > +/* Note: set freeTransferable to true if the data has never been read */ > +JS_PUBLIC_API(JSBool) > +JS_ClearStructuredClone(const uint64_t *data, size_t nbytes, JSBool freeTransferable); Sorry, I hadn't looked closely at this before. This seems error-prone. Initially, I couldn't figure out why you needed to pass in freeTransferable; shouldn't the buffer remember whether it has been read yet? But I guess this is for where you JS_WriteStructuredClone and make a copy of the resulting bytes -- you don't want to free the transferables when you deallocate the old buffer. I think I would prefer if you removed this parameter and made the buffer remember whether it has been read yet, and use a plain JS_free on the clone buffer that you copied from. (Ok, I would *really* prefer if that memcpy were just nuked, but I wouldn't bog down this bug for that.) Unless there's something I'm missing, which is totally possible? Let me know what you think. ::: js/src/jsclone.cpp @@ +1093,5 @@ > + return false; > + > + JSObject *obj = JS_NewArrayBufferWithContents(context(), content); > + if (!obj || !allObjs.append(ObjectValue(*obj))) > + return false; Yay, this is much nicer now that back references are handled properly so you don't have to treat the transferables differently.

take a look.

Comment on attachment 665150 [details] [diff] [review] patch 4 Review of attachment 665150 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsStructuredCloneContainer.cpp @@ +68,5 @@ > if (!mData) { > mSize = 0; > mVersion = 0; > > + JS_ClearStructuredClone(jsBytes, mSize); I'm not sure whether this would be better as JS_free or JS_ClearStructuredClone, but I guess this is fine. Can you add a MOZ_ASSERT that mData doesn't contain transferables somewhere? Both in the destructor and after the JS_ReadStructuredClone in DeserializeToVariant would be good, to avoid someone else haivng to figure out why it's ok. It's nonlocal knowledge where something like nsStructuredCloneContainer::InitFromBase64 could get its data. ::: js/src/jsclone.cpp @@ +159,5 @@ > + uint64_t u = SwapBytes(*point++); > + uint32_t tag = uint32_t(u >> 32); > + if (tag == SCTAG_TRANSFER_MAP) { > + u = SwapBytes(*point++); > + js_free((void *)u); reinterpret_cast<void*>(u) @@ +438,5 @@ > > bool > +SCOutput::writePtr(const void *p) > +{ > + return write((uint64_t)p); reinterpret_cast<uint64_t>(p)

https://tbpl.mozilla.org/?tree=Try&rev=699ad4790836 JSBool vs bool for some architecture breaks mochitests.

> Do you have a need for it Won't we eventually want that for Blob or File or whatever?

I don't think so. Transferring blobs/files doesn't require copying because they're immutable.

Comment on attachment 665500 [details] [diff] [review] patch 4e Review of attachment 665500 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/workers/Worker.cpp @@ +289,5 @@ > if (!JS_ConvertArguments(aCx, aArgc, JS_ARGV(aCx, aVp), "v", &message)) { > return false; > } > > + jsval transferable = JSVAL_VOID; This should be wrapped into the JS_ConvertArguments call. ::: dom/workers/WorkerScope.cpp @@ +842,5 @@ > if (!JS_ConvertArguments(aCx, aArgc, JS_ARGV(aCx, aVp), "v", &message)) { > return false; > } > > + jsval transferable = JSVAL_VOID; Here too.

is it enough: jsval transferable = JSVAL_VOID; to have it VOID if not set by the callee?

Comment on attachment 665512 [details] [diff] [review] patch 4f Review of attachment 665512 [details] [diff] [review]: ----------------------------------------------------------------- I only looked at the JS_ConvertArguments changes, but r=me on that!

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/9600a66b7bcc - the Windows build bustage in https://tbpl.mozilla.org/php/getParsedLog.php?id=15612554&tree=Mozilla-Inbound may just mean that the build system is busted like usual, and you'll have to land with a clobber; the assertion and failure in https://tbpl.mozilla.org/php/getParsedLog.php?id=15612800&tree=Mozilla-Inbound looks pretty directly related.

https://tbpl.mozilla.org/?tree=Try&rev=77b0627abb44 it looks green on try. Any hints?

I'll try pushing it again this weekend with a clobber first when things are quiet.

Actually, this is causing bug 795205, so please don't just reland as-is. Andrea: Any ideas why bug 795205 happened? I wouldn't have thought that any existing code would be affected by this patch.

Among the many possibilities is the fact that we know it wasn't properly built as a dep build on one platform, and it's entirely possible that it wasn't properly built as a dep build on other platforms, it just didn't happen to cause the build to fail elsewhere. Does a clobber b2g build still hit bug 795205?

(In reply to Andrea Marchesini (:baku) from comment #43) > https://tbpl.mozilla.org/?tree=Try&rev=77b0627abb44 it looks green on try. Jonas or Chris, you can you try this build out and see if it fails the same? Try builds are always clobbers.

FWIW, I just ran a B2G clobber build on mozilla-central with the attached patch, and it looks like RIL is failing. Not sure if this patch is the cause though, I know Kyle Machulis was looking into / fixed some other RIL bug last night as well. Some relevant messages I saw in logcat: 09-29 17:31:27.674 106 236 Gecko I RIL Worker: RIL Worker errorundefined 09-29 17:31:37.433 106 106 GeckoConsole E [JavaScript Error: "TypeError: conn.voice.network is null" {file: "app://sms.gaiamobile.org/js/background.js" line: 11}] Entire logcat link: http://pastebin.mozilla.org/1849876 CCing Kyle in case he knows what's going on.

postMessage for workers doesn't have the origin target param. Line: 3709 file: ril_worker.js - postMessage(message, "*") -> it should be postMessage(message) Can we try to apply the patch again with this line changed?

We probably will want to be able to transfer ImageData objects at some point.

patch for RIL_worker.js

https://hg.mozilla.org/integration/mozilla-inbound/rev/db347c212f80 https://hg.mozilla.org/integration/mozilla-inbound/rev/439eafb9c71e

https://hg.mozilla.org/mozilla-central/rev/db347c212f80 https://hg.mozilla.org/mozilla-central/rev/439eafb9c71e

This really needs to be documented on MDN.

(In reply to David Rajchenbach Teller [:Yoric] from comment #54) > This really needs to be documented on MDN. Agreed, it's a very important capability of the web platform. Do you feel like you're capable of documenting it? If so, I can provide help and review. The upcoming doc sprint (~Feb 9th I heard) may be a good opportunity to do this.

I'll try and do that this week.

I may be wrong, but the tests don't seem to test the spec, and I have the impression that the implementation also doesn't quite implement it. According to the spec, we should accept as second argument to |postMessage| an association list from transferable objects to substitution values, i.e. an array of arrays. Here, we test (and I believe we implement) a method that accepts a list of objects to be transferred. That looks fishy.

Ah, looks like it's just a case of obfuscated specifications. The whatwg specs are slightly clearer insofar as they initialize the "transfer map" from an array provided by the user.

I went ahead and edited some doc related to transferable objects: https://developer.mozilla.org/en-US/docs/DOM/Worker#postMessage%28%29 https://developer.mozilla.org/en-US/docs/DOM/Using_web_workers#Passing_data_by_transferring_.C2.A0ownership_%28transferable_objects%29 Tell me what you think.

Thank you for the documentation. Only a small issue: "According to the spec, only MessagePort, ArrayBuffers and CanvasProxy objects can be transferred." Right. But we support only ArrayBuffers right now.

(In reply to Andrea Marchesini (:baku) from comment #60) > Thank you for the documentation. Most of it was already written ;-) > Only a small issue: > "According to the spec, only MessagePort, ArrayBuffers and CanvasProxy > objects can be transferred." Right. But we support only ArrayBuffers right > now. WebKit does implement MessagePort as Transferable (and Chrome supports it), so that's valuable information. CanvasProxy is spec-only at this point... There is a stub on Webkit, but apparently, it's not there yet. So you're right, I'm removing that from the doc.

(In reply to David Bruant from comment #61) > (In reply to Andrea Marchesini (:baku) from comment #60) > > Thank you for the documentation. > Most of it was already written ;-) > > > Only a small issue: > > "According to the spec, only MessagePort, ArrayBuffers and CanvasProxy > > objects can be transferred." Right. But we support only ArrayBuffers right > > now. > WebKit does implement MessagePort as Transferable (and Chrome supports it), > so that's valuable information. > CanvasProxy is spec-only at this point... There is a stub on Webkit, but > apparently, it's not there yet. So you're right, I'm removing that from the > doc. A description about browser support should be added instead of deleting the text.

I have seen that it is somewhat documented on MDN.