| Summary: | Use IDLTypes in more places | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Sam Weinig <sam> |
| Component: | New Bugs | Assignee: | Sam Weinig <sam> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | achristensen, alecflett, beidson, buildbot, cdumez, cgarcia, commit-queue, esprehn+autocc, jsbell, kondapallykalyan, rniwa |
| Priority: | P2 | ||
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Attachments: | |||
|
Description
Sam Weinig
2016-10-10 15:38:26 PDT
Created attachment 291175 [details]
Patch
Attachment 291175 [details] did not pass style-queue:
ERROR: Source/WebCore/bindings/js/JSDOMConvert.h:324: Code inside a namespace should not be indented. [whitespace/indent] [4]
Total errors found: 1 in 21 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 291175 [details] Patch Attachment 291175 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2258493 New failing tests: fast/events/constructors/message-event-constructor.html Created attachment 291191 [details]
Archive of layout-test-results from ews101 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 291175 [details] Patch Attachment 291175 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2258505 New failing tests: fast/events/constructors/message-event-constructor.html fast/mediastream/MediaStreamConstructor.html Created attachment 291192 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 291175 [details] Patch Attachment 291175 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2258503 New failing tests: fast/events/constructors/message-event-constructor.html Created attachment 291193 [details]
Archive of layout-test-results from ews114 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 291175 [details] Patch Attachment 291175 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2258521 New failing tests: fast/events/constructors/message-event-constructor.html Created attachment 291194 [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
Created attachment 291203 [details]
Patch
Attachment 291203 [details] did not pass style-queue:
ERROR: Source/WebCore/bindings/js/JSDOMConvert.h:324: Code inside a namespace should not be indented. [whitespace/indent] [4]
Total errors found: 1 in 24 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 291203 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291203&action=review I like this approach. > Source/WebCore/bindings/scripts/IDLParser.pm:147 > - extendedAttributes => '$', # Extended attributes > - type => '$', # Type of data > + extendedAttributes => '$', # Extended attributes > + idlType => '$', # Type of data I don't like this kind of "lining up" and our style guide explicitly forbids it. > LayoutTests/fast/events/constructors/message-event-constructor-expected.txt:84 > -PASS new MessageEvent('eventType', { ports: [1, 2, 3] }).ports[2] threw exception TypeError: Invalid Array element type. > +PASS new MessageEvent('eventType', { ports: [1, 2, 3] }).ports[2] threw exception TypeError: Type error. Seems like the new message is not quite as good. > LayoutTests/fast/mediastream/MediaStreamConstructor-expected.txt:15 > -PASS new MediaStream([document]) threw exception TypeError: Invalid Array element type. > -PASS new MediaStream([stream.getAudioTracks()[0], document]) threw exception TypeError: Invalid Array element type. > -PASS new MediaStream([null]) threw exception TypeError: Invalid Array element type. > -PASS new MediaStream([undefined]) threw exception TypeError: Invalid Array element type. > +PASS new MediaStream([document]) threw exception TypeError: Type error. > +PASS new MediaStream([stream.getAudioTracks()[0], document]) threw exception TypeError: Type error. > +PASS new MediaStream([null]) threw exception TypeError: Type error. > +PASS new MediaStream([undefined]) threw exception TypeError: Type error. Ditto. (In reply to comment #13) > Comment on attachment 291203 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=291203&action=review > > I like this approach. > > > Source/WebCore/bindings/scripts/IDLParser.pm:147 > > - extendedAttributes => '$', # Extended attributes > > - type => '$', # Type of data > > + extendedAttributes => '$', # Extended attributes > > + idlType => '$', # Type of data > > I don't like this kind of "lining up" and our style guide explicitly forbids > it. > > > LayoutTests/fast/events/constructors/message-event-constructor-expected.txt:84 > > -PASS new MessageEvent('eventType', { ports: [1, 2, 3] }).ports[2] threw exception TypeError: Invalid Array element type. > > +PASS new MessageEvent('eventType', { ports: [1, 2, 3] }).ports[2] threw exception TypeError: Type error. > > Seems like the new message is not quite as good. > > > LayoutTests/fast/mediastream/MediaStreamConstructor-expected.txt:15 > > -PASS new MediaStream([document]) threw exception TypeError: Invalid Array element type. > > -PASS new MediaStream([stream.getAudioTracks()[0], document]) threw exception TypeError: Invalid Array element type. > > -PASS new MediaStream([null]) threw exception TypeError: Invalid Array element type. > > -PASS new MediaStream([undefined]) threw exception TypeError: Invalid Array element type. > > +PASS new MediaStream([document]) threw exception TypeError: Type error. > > +PASS new MediaStream([stream.getAudioTracks()[0], document]) threw exception TypeError: Type error. > > +PASS new MediaStream([null]) threw exception TypeError: Type error. > > +PASS new MediaStream([undefined]) threw exception TypeError: Type error. > > Ditto. Indeed. I want to improve the exceptions across the board here, but it will take a few iterations. Considering a few options, including passing in a lambda which creates the exception, so it can be call-site specific. (In reply to comment #14) > (In reply to comment #13) > > Comment on attachment 291203 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=291203&action=review > > > > I like this approach. > > > > > Source/WebCore/bindings/scripts/IDLParser.pm:147 > > > - extendedAttributes => '$', # Extended attributes > > > - type => '$', # Type of data > > > + extendedAttributes => '$', # Extended attributes > > > + idlType => '$', # Type of data > > > > I don't like this kind of "lining up" and our style guide explicitly forbids > > it. > > > > > LayoutTests/fast/events/constructors/message-event-constructor-expected.txt:84 > > > -PASS new MessageEvent('eventType', { ports: [1, 2, 3] }).ports[2] threw exception TypeError: Invalid Array element type. > > > +PASS new MessageEvent('eventType', { ports: [1, 2, 3] }).ports[2] threw exception TypeError: Type error. > > > > Seems like the new message is not quite as good. > > > > > LayoutTests/fast/mediastream/MediaStreamConstructor-expected.txt:15 > > > -PASS new MediaStream([document]) threw exception TypeError: Invalid Array element type. > > > -PASS new MediaStream([stream.getAudioTracks()[0], document]) threw exception TypeError: Invalid Array element type. > > > -PASS new MediaStream([null]) threw exception TypeError: Invalid Array element type. > > > -PASS new MediaStream([undefined]) threw exception TypeError: Invalid Array element type. > > > +PASS new MediaStream([document]) threw exception TypeError: Type error. > > > +PASS new MediaStream([stream.getAudioTracks()[0], document]) threw exception TypeError: Type error. > > > +PASS new MediaStream([null]) threw exception TypeError: Type error. > > > +PASS new MediaStream([undefined]) threw exception TypeError: Type error. > > > > Ditto. > > Indeed. I want to improve the exceptions across the board here, but it will > take a few iterations. Considering a few options, including passing in a > lambda which creates the exception, so it can be call-site specific. Yes, we could pass a lambda which would be super flexible and allow call-site specific messages. Alternatively, we could simply take a string literal representing the expected type so that the convert function can generate a better message on its own. Committed revision 207150 Looks like this broke the bindings tests *** Bug 160926 has been marked as a duplicate of this bug. *** |