Bug 163475

Summary: MessageEvent's source property should be a (DOMWindow or MessagePort)? rather than a EventTarget?
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, buildbot, cdumez, cmarcelo, commit-queue, dbates, esprehn+autocc, kangil.han, kondapallykalyan, rniwa
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews103 for mac-yosemite
none
Archive of layout-test-results from ews106 for mac-yosemite-wk2
none
Archive of layout-test-results from ews113 for mac-yosemite
none
Archive of layout-test-results from ews121 for ios-simulator-wk2
none
Patch simon.fraser: review+

Description Sam Weinig 2016-10-14 17:21:43 PDT
MessageEvent's source property should be a (DOMWindow or MessagePort)? rather than a EventTarget?
Comment 1 Sam Weinig 2016-10-14 17:22:37 PDT
Created attachment 291684 [details]
Patch
Comment 2 WebKit Commit Bot 2016-10-14 17:25:25 PDT
Attachment 291684 [details] did not pass style-queue:


ERROR: Source/WebCore/dom/MessageEvent.h:63:  The parameter name "source" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/dom/MessageEvent.h:64:  The parameter name "source" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/dom/MessageEvent.h:95:  The parameter name "source" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Tools/TestWebKitAPI/Tests/WTF/Optional.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 5 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Build Bot 2016-10-14 18:32:51 PDT
Comment on attachment 291684 [details]
Patch

Attachment 291684 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2287527

New failing tests:
fast/events/constructors/message-event-constructor.html
fast/dom/message-port-deleted-by-accessor.html
Comment 4 Build Bot 2016-10-14 18:32:55 PDT
Created attachment 291692 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 5 Build Bot 2016-10-14 18:36:08 PDT
Comment on attachment 291684 [details]
Patch

Attachment 291684 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2287528

New failing tests:
fast/events/constructors/message-event-constructor.html
fast/dom/message-port-deleted-by-accessor.html
Comment 6 Build Bot 2016-10-14 18:36:11 PDT
Created attachment 291693 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 7 Build Bot 2016-10-14 18:37:46 PDT
Comment on attachment 291684 [details]
Patch

Attachment 291684 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2287524

New failing tests:
fast/events/constructors/message-event-constructor.html
fast/dom/message-port-deleted-by-accessor.html
Comment 8 Build Bot 2016-10-14 18:37:50 PDT
Created attachment 291694 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 9 Build Bot 2016-10-14 18:45:35 PDT
Comment on attachment 291684 [details]
Patch

Attachment 291684 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2287530

New failing tests:
fast/events/constructors/message-event-constructor.html
fast/dom/message-port-deleted-by-accessor.html
Comment 10 Build Bot 2016-10-14 18:45:39 PDT
Created attachment 291695 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 11 Sam Weinig 2016-10-15 13:48:57 PDT
Created attachment 291725 [details]
Patch
Comment 12 WebKit Commit Bot 2016-10-15 13:50:46 PDT
Attachment 291725 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/Variant.h:1947:  Missing space after ,  [whitespace/comma] [3]
ERROR: Source/WebCore/dom/MessageEvent.h:63:  The parameter name "source" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/dom/MessageEvent.h:64:  The parameter name "source" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/dom/MessageEvent.h:95:  The parameter name "source" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 4 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Simon Fraser (smfr) 2016-10-15 14:48:41 PDT
Comment on attachment 291725 [details]
Patch

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

> Source/WebCore/bindings/scripts/test/TestObj.idl:463
> +    (long or Node) unionMember;
> +    (long or Node)? nullableUnionMember = null;

I rather wish we'd stop piling stuff onto this interface and make more targeted tests.
Comment 14 Sam Weinig 2016-10-15 14:55:02 PDT
Committed revision 207381.
Comment 15 Sam Weinig 2016-10-15 14:56:44 PDT
(In reply to comment #13)
> Comment on attachment 291725 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=291725&action=review
> 
> > Source/WebCore/bindings/scripts/test/TestObj.idl:463
> > +    (long or Node) unionMember;
> > +    (long or Node)? nullableUnionMember = null;
> 
> I rather wish we'd stop piling stuff onto this interface and make more
> targeted tests.

Any particular reason? The boilerplate of setting up a new test is relatively high and makes the test run quite a bit longer than just adding a new property to an existing one.  I also haven't found it difficult to debug issues with these larger tests.