Bug 73589 – [V8][Chromium] Adjust postMessage to the latest "implementation-ready" spec

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Dmitry Lomov]

1. postMessage should support transfer of MessagePorts
2. the order of arguments to Window::postMessage and Window::webkitPostMessage should be (msg, targetOrigin [, transfer])

[Dmitry Lomov]

Created attachment 117487 [details]
Fix

[David Levin]

Comment on attachment 117487 [details]
Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=117487&action=review

> Source/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:306
> +            if (!extractTransferables(args[2], portArray, arrayBufferArray))

You could combine with the previous if instead of having
if...
 if ...

[WebKit Review Bot]

Comment on attachment 117487 [details]
Fix

Attachment 117487 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10690360

New failing tests:
http/tests/inspector/extensions-headers.html
css2.1/20110323/abspos-containing-block-initial-004b.htm
http/tests/inspector/extensions-ignore-cache.html
inspector/extensions/extensions-audits-api.html
inspector/extensions/extensions-events.html
fast/forms/number/spin-in-multi-column.html
inspector/extensions/extensions-network.html
fast/replaced/width100percent-textarea.html
http/tests/inspector/extensions-network-redirect.html
inspector/extensions/extensions-audits.html
inspector/extensions/extensions-console.html
css2.1/20110323/abspos-containing-block-initial-004d.htm
inspector/extensions/extensions-eval.html
http/tests/inspector/extensions-useragent.html
inspector/extensions/extensions-panel.html
inspector/extensions/extensions-api.html

[Dmitry Lomov]

(In reply to comment #3)
> (From update of attachment 117487 [details])
> Attachment 117487 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/10690360
> 
> New failing tests:
> http/tests/inspector/extensions-headers.html
> css2.1/20110323/abspos-containing-block-initial-004b.htm
> http/tests/inspector/extensions-ignore-cache.html
> inspector/extensions/extensions-audits-api.html
> inspector/extensions/extensions-events.html
> fast/forms/number/spin-in-multi-column.html
> inspector/extensions/extensions-network.html
> fast/replaced/width100percent-textarea.html
> http/tests/inspector/extensions-network-redirect.html
> inspector/extensions/extensions-audits.html
> inspector/extensions/extensions-console.html
> css2.1/20110323/abspos-containing-block-initial-004d.htm
> inspector/extensions/extensions-eval.html
> http/tests/inspector/extensions-useragent.html
> inspector/extensions/extensions-panel.html
> inspector/extensions/extensions-api.html

These tests timeout on my machine even without a patch.

[Dmitry Lomov]

Landed in http://trac.webkit.org/changeset/101831.

[Eric Seidel (no email)]

This is causing a new failure on JSC bots:
http://build.webkit.org/results/Lion%20Intel%20Release%20(Tests)/r101838%20(3185)/fast/dom/Window/window-postmessage-args-pretty-diff.html

[Andrey Kosyakov]

First, this indeed broke all inspector extensions tests (and extensions themselves), as extensions rely on an order of arguments in postMessage(). Doing a code search for postMessage() and paying attention to test results would probably save us from this, but I think this will also break quite a few pages in the internet that rely on postMessage().

Second, the actual order of the arguments, as processed by V8DOMWindowCustom::handlePostMessageCallback() is different from that in DOMWindow.idl -- I find this misleading to say least.

Third, the order of arguments is now different for JSC and V8 bindings -- so getting a code to work in two different WebKit browser is going to be tricky.

I believe the above justifies the roll back for this change.

[Dmitry Lomov]

(In reply to comment #7)
> First, this indeed broke all inspector extensions tests (and extensions themselves), as extensions rely on an order of arguments in postMessage(). Doing a code search for postMessage() and paying attention to test results would probably save us from this, but I think this will also break quite a few pages in the internet that rely on postMessage().
> 
> Second, the actual order of the arguments, as processed by V8DOMWindowCustom::handlePostMessageCallback() is different from that in DOMWindow.idl -- I find this misleading to say least.
> 
> Third, the order of arguments is now different for JSC and V8 bindings -- so getting a code to work in two different WebKit browser is going to be tricky.
> 
> I believe the above justifies the roll back for this change.

I am working on a fix that will return the support for legacy argument order.

[Dmitry Lomov]

(In reply to comment #8)
> (In reply to comment #7)
> > First, this indeed broke all inspector extensions tests (and extensions themselves), as extensions rely on an order of arguments in postMessage(). Doing a code search for postMessage() and paying attention to test results would probably save us from this, but I think this will also break quite a few pages in the internet that rely on postMessage().
> > 
> > Second, the actual order of the arguments, as processed by V8DOMWindowCustom::handlePostMessageCallback() is different from that in DOMWindow.idl -- I find this misleading to say least.
> > 
> > Third, the order of arguments is now different for JSC and V8 bindings -- so getting a code to work in two different WebKit browser is going to be tricky.
> > 
> > I believe the above justifies the roll back for this change.
> 
> I am working on a fix that will return the support for legacy argument order.

The bug is https://bugs.webkit.org/show_bug.cgi?id=74038