Bug 124708 - DataCloneError exception is not thrown after invalid postMessage method invocation
Summary: DataCloneError exception is not thrown after invalid postMessage method invoc...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Unspecified
: P2 Normal
Assignee: Michał Poteralski
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-21 03:51 PST by Michał Poteralski
Modified: 2013-12-11 10:53 PST (History)
8 users (show)

See Also:


Attachments
Proposed patch (3.95 KB, patch)
2013-11-21 04:38 PST, Michał Poteralski
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (483.44 KB, application/zip)
2013-11-21 05:38 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion (498.63 KB, application/zip)
2013-11-21 06:05 PST, Build Bot
no flags Details
Path (6.10 KB, application/octet-stream)
2013-11-25 03:10 PST, Michał Poteralski
no flags Details
Proposed Path 2 (6.10 KB, patch)
2013-11-25 03:12 PST, Michał Poteralski
ap: review-
Details | Formatted Diff | Diff
Patch no 3 (5.96 KB, patch)
2013-12-06 00:51 PST, Michał Poteralski
ap: review+
ap: commit-queue-
Details | Formatted Diff | Diff
Patch no 4 (6.06 KB, patch)
2013-12-09 00:43 PST, Michał Poteralski
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michał Poteralski 2013-11-21 03:51:32 PST
According to the specification: http://www.whatwg.org/specs/web-apps/current-work/multipage/web-messaging.html#dom-window-postmessage 

If the method was invoked with a second argument transfer then if any of the objects in the transfer are either the source port or the target port (if any), then a DataCloneError exception should be thrown. Currently an InvalidStateError exception is thrown what is an incorrect behaviour.
Comment 1 Michał Poteralski 2013-11-21 04:38:09 PST
Created attachment 217547 [details]
Proposed patch
Comment 2 Build Bot 2013-11-21 05:38:12 PST
Comment on attachment 217547 [details]
Proposed patch

Attachment 217547 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/30528006

New failing tests:
fast/events/message-port-clone.html
fast/events/message-port-multi.html
Comment 3 Build Bot 2013-11-21 05:38:14 PST
Created attachment 217550 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 4 Build Bot 2013-11-21 06:05:05 PST
Comment on attachment 217547 [details]
Proposed patch

Attachment 217547 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/31728042

New failing tests:
fast/events/message-port-clone.html
fast/events/message-port-multi.html
Comment 5 Build Bot 2013-11-21 06:05:09 PST
Created attachment 217553 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 6 Michał Poteralski 2013-11-25 03:10:29 PST
Created attachment 217781 [details]
Path
Comment 7 Michał Poteralski 2013-11-25 03:12:21 PST
Created attachment 217782 [details]
Proposed Path 2
Comment 8 Alexey Proskuryakov 2013-12-04 13:36:26 PST
Comment on attachment 217782 [details]
Proposed Path 2

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

Seems fine to make this change.

> LayoutTests/http/tests/security/postMessage/postMessage-clone-port-error.html:3
> +<script src="../../../../resources/js-test-pre.js"></script>

In HTTP tests, you don't need to use relative paths, can be just "/resources/js-test-pre.js".

But then I'm not sure why this is an HTTP test. Can it be a regular one? It's better to make non-HTTP tests when possible, because HTTP tests are slower.

> LayoutTests/http/tests/security/postMessage/postMessage-clone-port-error.html:23
> +</html>

You should also include js-test-post.js - it signals that a test has been run in full.
Comment 9 Michał Poteralski 2013-12-06 00:51:21 PST
Created attachment 218577 [details]
Patch no 3
Comment 10 Alexey Proskuryakov 2013-12-08 23:36:30 PST
> You should also include js-test-post.js - it signals that a test has been run in full.

This still needs to be done.
Comment 11 Michał Poteralski 2013-12-09 00:43:53 PST
Created attachment 218730 [details]
Patch no 4
Comment 12 WebKit Commit Bot 2013-12-09 04:39:58 PST
Comment on attachment 218730 [details]
Patch no 4 

Clearing flags on attachment: 218730

Committed r160309: <http://trac.webkit.org/changeset/160309>
Comment 13 Alexey Proskuryakov 2013-12-09 11:01:39 PST
Comment on attachment 218730 [details]
Patch no 4 

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

> LayoutTests/fast/dom/Window/postMessage-clone-port-error-expected.txt:4
> +PASS successfullyParsed is true
> +
> +TEST COMPLETE
> +PASS exName is "DataCloneError"

These results are broken, TEST COMPLETE should be the last line.
Comment 14 Michał Poteralski 2013-12-10 01:55:29 PST
The issue related to the test expectation order will be improved in an external issue: https://bugs.webkit.org/show_bug.cgi?id=125487
Comment 15 Michał Poteralski 2013-12-11 10:53:09 PST
The issue is resolved.