RESOLVED FIXED 94581
window.postMessage() / MessagePort.postMessage() throw wrong exception for invalid ports argument 
https://bugs.webkit.org/show_bug.cgi?id=94581
Summary window.postMessage() / MessagePort.postMessage() throw wrong exception for in...
Chris Dumez
Reported 2012-08-21 03:14:54 PDT
1. According to the latest specification for window.postMessage() (http://www.w3.org/TR/html5/comms.html#dom-window-postmessage-2): "Throws an INVALID_STATE_ERR if the ports array is not null and it contains either null entries or duplicate ports." We currently throw a DATA_CLONE_ERR. 2. According to the latest specification for MessagePort.postMessage() (http://www.w3.org/TR/html5/comms.html#messageport): "Throws an INVALID_STATE_ERR if the ports array is not null and it contains either null entries, duplicate ports, or the source or target port." We currently throw a DATA_CLONE_ERR for those cases. We should fix this to comply with the latest specification.
Attachments
Patch (24.47 KB, patch)
2012-08-21 04:21 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (24.47 KB, patch)
2012-08-21 04:38 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (24.48 KB, patch)
2012-08-21 04:41 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2012-08-21 04:21:05 PDT
Created attachment 159652 [details] Patch
Chris Dumez
Comment 2 2012-08-21 04:38:04 PDT
Created attachment 159655 [details] Patch
Chris Dumez
Comment 3 2012-08-21 04:41:44 PDT
Created attachment 159656 [details] Patch
Kentaro Hara
Comment 4 2012-08-21 04:50:11 PDT
Comment on attachment 159656 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159656&action=review The change looks OK. I just want to confirm cross-browser compatibility before r+ing it. > Source/WebCore/bindings/v8/V8Utilities.cpp:117 > + if (ports.contains(port)) { > + setDOMException(INVALID_STATE_ERR, isolate); > + return false; > + } This change is trying to throw exception for the code that had not thrown exception before. What are the behaviors of other browsers (IE, Firefox and Opera)?
Chris Dumez
Comment 5 2012-08-21 05:05:46 PDT
Firefox does not seem to support MessageChannel / MessagePort at all: https://bugzilla.mozilla.org/show_bug.cgi?id=677638 https://bugzilla.mozilla.org/show_bug.cgi?id=741618
Kentaro Hara
Comment 6 2012-08-21 05:06:25 PDT
(In reply to comment #5) > Firefox does not seem to support MessageChannel / MessagePort at all: > https://bugzilla.mozilla.org/show_bug.cgi?id=677638 > https://bugzilla.mozilla.org/show_bug.cgi?id=741618 Thanks, how about Opera and IE?
Chris Dumez
Comment 7 2012-08-21 05:16:08 PDT
On Opera, it throws a DATA_CLONE_ERR instead of an INVALID_STATE_ERR, but at least it throws: PASS channel.port1.postMessage("duplicate port", [channel3.port1, channel3.port1]) threw exception DOMException: DATA_CLONE_ERR. I'm not sure how I can check for IE.
Kentaro Hara
Comment 8 2012-08-21 05:37:23 PDT
Comment on attachment 159656 [details] Patch I checked it on IE. IE doesn't implement MessageChannel. - IE and Firefox don't implement MessageChannel. - Given that Opera throws exceptions, it makes sense for WebKit to throw some exception. - Although Opera throws DATA_CLONE_ERR, the spec requires to throw INVALID_STATE_ERR. Since people wouldn't much care about the exception type, it sounds reasonable to change WebKit to throw INVALID_STATE_ERR according to the spec. Let me r+ for now. But let's wait for landing the patch for one day, as someone might have a concern.
Chris Dumez
Comment 9 2012-08-22 03:09:17 PDT
Comment on attachment 159656 [details] Patch No one has raised any concerned and the EWS bots are all green. Setting cq flag. Could someone please commit?
Kentaro Hara
Comment 10 2012-08-22 03:10:46 PDT
Comment on attachment 159656 [details] Patch OK, thanks for the patch.
WebKit Review Bot
Comment 11 2012-08-22 03:36:47 PDT
Comment on attachment 159656 [details] Patch Clearing flags on attachment: 159656 Committed r126286: <http://trac.webkit.org/changeset/126286>
WebKit Review Bot
Comment 12 2012-08-22 03:36:51 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.