Bug 170926 – [WebIDL] Make annotated types first class allowing them to be used in sequences and unions

• Component: New Bugs
• Status: RESOLVED FIXED

[Sam Weinig]

[WebIDL] Make annotated types first class allowing them to be used in sequences and unions

[Sam Weinig]

Created attachment 307330 [details]
Patch

[Build Bot]

Attachment 307330 [details] did not pass style-queue:


ERROR: Source/WebCore/domjit/DOMJITIDLType.h:43:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/domjit/DOMJITIDLType.h:44:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:42:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:43:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:67:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:68:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 6 in 46 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Sam Weinig]

Created attachment 307341 [details]
Patch

[Build Bot]

Attachment 307341 [details] did not pass style-queue:


ERROR: Source/WebCore/domjit/DOMJITIDLType.h:43:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/domjit/DOMJITIDLType.h:44:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:42:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:43:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:67:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:68:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 6 in 47 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 307341 [details]
Patch

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

New failing tests:
http/tests/inspector/network/resource-sizes-network.html

[Build Bot]

Created attachment 307353 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Sam Weinig]

The failure looks to be a flaky test. Pay it no mind.

[Chris Dumez]

Comment on attachment 307341 [details]
Patch

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

r=me, nice.

> Source/WebCore/bindings/js/JSDOMConvertNumbers.cpp:252
>      if (value.isInt32())

We may even want to LIKELY() this branch.

> Source/WebCore/bindings/js/JSDOMConvertNumbers.cpp:265
> +    if (value.isUInt32())

We may even want to LIKELY() this branch.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:5379
> +sub GetAnnotatedIDLType

Having 2 subroutines that need to be kept in sync seems a bit unfortunate. Could we maybe drop the IsAnnotatedType() one and rely on GetAnnotatedIDLType() returning something or not?

[Sam Weinig]

(In reply to Chris Dumez from comment #8)
> Comment on attachment 307341 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=307341&action=review
> 
> r=me, nice.
> 
> > Source/WebCore/bindings/js/JSDOMConvertNumbers.cpp:252
> >      if (value.isInt32())
> 
> We may even want to LIKELY() this branch.
> 
> > Source/WebCore/bindings/js/JSDOMConvertNumbers.cpp:265
> > +    if (value.isUInt32())
> 
> We may even want to LIKELY() this branch.

I'm not sure if these are likely honestly. Since the number can often be a double, I'm not sure it makes sense here.
> 
> > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:5379
> > +sub GetAnnotatedIDLType
> 
> Having 2 subroutines that need to be kept in sync seems a bit unfortunate.
> Could we maybe drop the IsAnnotatedType() one and rely on
> GetAnnotatedIDLType() returning something or not?

It would make the calling code a bit odd, and this is kind of the idiom in the code generator. I'll think about how to improve it more generally.

Thanks for the review.

[Sam Weinig]

Committed r215477: <http://trac.webkit.org/changeset/215477>