Bug 73589

Created attachment 117487 [details] Fix

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

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

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

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

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

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.

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

(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