[webkit-reviews] review denied: [Bug 65209] Make MessagePort Transferable. : [Attachment 102407] First proposed patch.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 1 13:26:57 PDT 2011


Oliver Hunt <oliver at apple.com> has denied Luke Zarko
<lukezarko+bugzilla at gmail.com>'s request for review:
Bug 65209: Make MessagePort Transferable.
https://bugs.webkit.org/show_bug.cgi?id=65209

Attachment 102407: First proposed patch.
https://bugs.webkit.org/attachment.cgi?id=102407&action=review

------- Additional Comments from Oliver Hunt <oliver at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=102407&action=review


I am still concerned that this API is badly thought out.  It seems very complex
for very little gain, and generally seems to be a solution for a problem that
has not yet been demonstrated to occur in the real world.

In terms of webkit code when you modify the serialization data you need to bump
the serialization version.  JSValueRefArray seems like a bad idea, you'd be
much better off simply using Vector<JSValue, ...  rather than JSValueRef. 
JSValueRef is really intended only for API usage.

I'm also concerned that it is not clear how this is expected to work for
cross-process messages.

>
LayoutTests/platform/chromium-mac/fast/dom/Window/window-postmessage-args-expec
ted.txt:5
> -PASS: Posting message ('1', 1): threw exception TypeError: MessagePortArray
argument must be an object
> -PASS: Posting message ('2', ): threw exception TypeError: MessagePortArray
argument must be an object
> -PASS: Posting message ('3', [object Object]): threw exception TypeError:
MessagePortArray argument has no length attribute
> +PASS: Posting message ('1', 1): threw exception TypeError: TransferableArray
argument must be an object
> +PASS: Posting message ('2', ): threw exception TypeError: TransferableArray
argument must be an object
> +PASS: Posting message ('3', [object Object]): threw exception TypeError:
TransferableArray argument has no length attribute

This error message change is concerning it implies that the new API behavior
conflicts with existing behavior


More information about the webkit-reviews mailing list