I think our implementation does check the transferList before serializing in our "SerializedScriptValue::ExtractTransferables" method). After all the first test in the PR does pass. That method just doesn't implement most of the checks that are required to be done on the transferList.

This is mostly minor variations in how we check.
e.g. per spec we ought to check detachment immediately, we ought to throw DataCloneError for an invalid argument but currently throw TypeError, etc.

I'm gonna quickly deal with at least one of these cases, because I got curious.

Also interesting to note: spec is kinda vague on what should happen if the ArrayBuffer becomes detached during StructuredSerializeInternal, like this:

let arrayBuffer = new ArrayBuffer(32);
postMessage({
  get a() { postMessage(null, '*', [arrayBuffer]); }
}, '*', [arrayBuffer]);

Given that it seems nonsensical to complete posting the message with the buffer, it does seem like we ought to still check after serialization, even if we add a check before serialization. And maybe the spec ought to clarify; currently it seems to suggest that we just do set dataHolder.[[ArrayBufferData]] to null and serialization succeeds.

Anne, WDYT?

(quick Chromium CL: https://chromium-review.googlesource.com/c/chromium/src/+/956254)

That's a somewhat compelling scenario to change the specification for. It crashes Edge. Firefox just gives you a detached copy (which seems somewhat reasonable and arguably defined).

https://github.com/whatwg/html/pull/3557 has a proposal for such a change. I'd appreciate review from you jbroman and perhaps you can state how you'd like to be acknowledged?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ade1dc7a1a967b82df4f1f7af2bec379efa26ce1

commit ade1dc7a1a967b82df4f1f7af2bec379efa26ce1
Author: Jeremy Roman <jbroman@chromium.org>
Date: Mon Mar 12 15:20:45 2018

Throw a DataCloneError if a neutered ArrayBuffer occurs in the transfer list.

Currently the exception is not thrown until after serialization.
This makes us consistent with Gecko and WebKit, which throw before serialization.

This fixes the second test case in https://github.com/w3c/web-platform-tests/pull/9672
(not yet pushed to upstream WPT).

Bug: 816447
Change-Id: I30b798b5d21dc8d6f2a40d049ced6aa7f60dd090
Reviewed-on: https://chromium-review.googlesource.com/956254
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542484}
[add] https://crrev.com/ade1dc7a1a967b82df4f1f7af2bec379efa26ce1/third_party/WebKit/LayoutTests/fast/dom/Window/window-postmessage-neutered-arraybuffer.html
[modify] https://crrev.com/ade1dc7a1a967b82df4f1f7af2bec379efa26ce1/third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValue.cpp

It seems that revision isn't quite aligned with what I proposed doing, unless I'm missing something.

Ah, sorry. I authored that CL before whatwg/html/pull/3557, and didn't realize (because the GitHub UI doesn't expand the diff properly) that you had moved it after StructuredSerializeInternal.

IIUC, this matches Chrome's previous behavior (i.e., only throw for detach *after* serialization), and I can just revert the Chromium CL in #7. Double-checking so I don't mess this up again. :D

Yeah, that is what that PR would do. Given the thumbs up I think Firefox is at least supportive of aligning the specification with Chrome here and given the crash in Edge I assume they would be too.

It doesn't make much sense to me to duplicate these steps (or allow Firefox's behavior).

Chrome was also failing the "Cannot transfer all objects" test, though that seems mostly due to it being the wrong exception, and not an exception at the wrong time.

Shall I acknowledge you as "Jeremy Roman" in the HTML Standard then?

I'll also update the tests PR, not sure if I get to it today though.

Yeah, that all sgtm. (Looks like the acknowledgment format is full name, so "Jeremy Roman" is correct for me. Thanks.)

(Not sure about TypeError vs DataCloneError for the wrong type of thing occurring in the transfer list. I haven't checked what various browsers do for that.)

To be clear, if you prefer jbroman or some other variant that's okay too.

Sorry for continuing to be unclear. "Jeremy Roman" is great.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f8e2ba434e0a21f81b817bd107739a39a3deb105

commit f8e2ba434e0a21f81b817bd107739a39a3deb105
Author: Jeremy Roman <jbroman@chromium.org>
Date: Wed Mar 14 15:38:05 2018

Revert "Throw a DataCloneError if a neutered ArrayBuffer occurs in the transfer list."

This reverts commit ade1dc7a1a967b82df4f1f7af2bec379efa26ce1.

Reason for revert: Further discussion on the bug (https://bugs.chromium.org/p/chromium/issues/detail?id=816447#c8); moving towards Chrome's previous behavior.

Original change's description:
> Throw a DataCloneError if a neutered ArrayBuffer occurs in the transfer list.
> 
> Currently the exception is not thrown until after serialization.
> This makes us consistent with Gecko and WebKit, which throw before serialization.
> 
> This fixes the second test case in https://github.com/w3c/web-platform-tests/pull/9672
> (not yet pushed to upstream WPT).
> 
> Bug: 816447
> Change-Id: I30b798b5d21dc8d6f2a40d049ced6aa7f60dd090
> Reviewed-on: https://chromium-review.googlesource.com/956254
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
> Commit-Queue: Jeremy Roman <jbroman@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#542484}

TBR=jbroman@chromium.org,mek@chromium.org,haraken@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 816447
Change-Id: I79950f8ba97e5481a872cc4897d62506fb2381b0
Reviewed-on: https://chromium-review.googlesource.com/960926
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543092}
[delete] https://crrev.com/7ceeebafcb1000b3107f9ec91a51f2851569a5d8/third_party/WebKit/LayoutTests/fast/dom/Window/window-postmessage-neutered-arraybuffer.html
[modify] https://crrev.com/f8e2ba434e0a21f81b817bd107739a39a3deb105/third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValue.cpp

The failure remaining in http://w3c-test.org/html/infrastructure/safe-passing-of-structured-data/transfer-errors.window.html is the type of error for non-transferable objects.