Bug 117547 – Support byte and octet types in bindings generators

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

[Chris Dumez]

The JSC bindings generator does not currently support the "byte" and "octet" IDL types:
http://dev.w3.org/2006/webapi/WebIDL/#idl-byte
http://dev.w3.org/2006/webapi/WebIDL/#idl-octet

This forces us to use custom code for DataView.

[Chris Dumez]

Created attachment 204445 [details]
WIP Patch

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/bindings/js/JSDataViewCustom.cpp', u'Source/WebCore/bindings/scripts/CodeGeneratorCPP.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.mm', u'Source/WebCore/bindings/scripts/test/TestObj.idl', u'Source/WebCore/html/canvas/DataView.idl']" exit_code: 1
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:613:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:615:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:616:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:617:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:620:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:622:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:624:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:625:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:626:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:629:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:1812:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:1814:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 12 in 14 files


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

[Chris Dumez]

Created attachment 204452 [details]
Patch

[Chris Dumez]

Created attachment 204453 [details]
JSDataView.cpp diff

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/js/JSDataViewCustom.cpp', u'Source/WebCore/bindings/scripts/CodeGeneratorCPP.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.mm', u'Source/WebCore/bindings/scripts/test/TestObj.idl', u'Source/WebCore/html/canvas/DataView.idl']" exit_code: 1
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:613:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:615:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:616:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:617:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:620:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:622:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:624:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:625:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:626:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:629:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 10 in 15 files


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

[Kentaro Hara]

Comment on attachment 204452 [details]
Patch

I think you need to implement toInt8() and toUInt8() to JSDOMBinding.h. It would be wrong to use existing toInt32() and toUint32() for the "byte" and "octet" conversions.

Also I'd like to see test cases (e.g. what happens when we pass -1, 0, 1, 127, 128, 255, 256, null, "foo" etc).

[Chris Dumez]

(In reply to comment #6)
> (From update of attachment 204452 [details])
> I think you need to implement toInt8() and toUInt8() to JSDOMBinding.h. It would be wrong to use existing toInt32() and toUint32() for the "byte" and "octet" conversions.

I see, I will look into it. For the record, this is currently identical to the custom code, meaning that there is no behavior change.

[Kentaro Hara]

(In reply to comment #7)
> (In reply to comment #6)
> > (From update of attachment 204452 [details] [details])
> > I think you need to implement toInt8() and toUInt8() to JSDOMBinding.h. It would be wrong to use existing toInt32() and toUint32() for the "byte" and "octet" conversions.
> 
> I see, I will look into it. For the record, this is currently identical to the custom code, meaning that there is no behavior change.

Yeah, but we might want to implement the correct implementation described in the Web IDL spec. Custom code is always buggy.

[Chris Dumez]

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > (From update of attachment 204452 [details] [details] [details])
> > > I think you need to implement toInt8() and toUInt8() to JSDOMBinding.h. It would be wrong to use existing toInt32() and toUint32() for the "byte" and "octet" conversions.
> > 
> > I see, I will look into it. For the record, this is currently identical to the custom code, meaning that there is no behavior change.
> 
> Yeah, but we might want to implement the correct implementation described in the Web IDL spec. Custom code is always buggy.

Ok, I will implemented to following algorithms:
http://dev.w3.org/2006/webapi/WebIDL/#es-byte
http://dev.w3.org/2006/webapi/WebIDL/#es-octet

[Chris Dumez]

(In reply to comment #6)
> (From update of attachment 204452 [details])
> I think you need to implement toInt8() and toUInt8() to JSDOMBinding.h. It would be wrong to use existing toInt32() and toUint32() for the "byte" and "octet" conversions.
> 
> Also I'd like to see test cases (e.g. what happens when we pass -1, 0, 1, 127, 128, 255, 256, null, "foo" etc).

Existing fast/canvas/webgl/data-view-test.html test seems to check several of those cases already.

[Kentaro Hara]

(In reply to comment #10)
> > Also I'd like to see test cases (e.g. what happens when we pass -1, 0, 1, 127, 128, 255, 256, null, "foo" etc).
> 
> Existing fast/canvas/webgl/data-view-test.html test seems to check several of those cases already.

Good. Thanks for working on this.

[Kentaro Hara]

(In reply to comment #11)
> (In reply to comment #10)
> > > Also I'd like to see test cases (e.g. what happens when we pass -1, 0, 1, 127, 128, 255, 256, null, "foo" etc).
> > 
> > Existing fast/canvas/webgl/data-view-test.html test seems to check several of those cases already.

You might want to add tests to LayoutTests/fast/js/webidl-type-mapping.html as well.

[Chris Dumez]

Created attachment 204559 [details]
Patch

[Chris Dumez]

(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > > Also I'd like to see test cases (e.g. what happens when we pass -1, 0, 1, 127, 128, 255, 256, null, "foo" etc).
> > > 
> > > Existing fast/canvas/webgl/data-view-test.html test seems to check several of those cases already.
> 
> You might want to add tests to LayoutTests/fast/js/webidl-type-mapping.html as well.

Sorry, I saw that comment after uploading the patch. I'll take a look.

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/canvas/webgl/data-view-test-expected.txt', u'LayoutTests/fast/canvas/webgl/data-view-test.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/js/JSDOMBinding.cpp', u'Source/WebCore/bindings/js/JSDOMBinding.h', u'Source/WebCore/bindings/js/JSDataViewCustom.cpp', u'Source/WebCore/bindings/scripts/CodeGeneratorCPP.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.mm', u'Source/WebCore/bindings/scripts/test/TestObj.idl', u'Source/WebCore/html/canvas/DataView.idl']" exit_code: 1
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:613:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:615:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:616:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:617:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:620:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:622:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:624:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:625:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:626:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:629:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 10 in 20 files


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

[Chris Dumez]

Comment on attachment 204559 [details]
Patch

Clearing flags until I add checks to LayoutTests/fast/js/webidl-type-mapping.html.

[Chris Dumez]

Created attachment 204567 [details]
Patch

Add tests to fast/js/webidl-type-mapping.html. Much better indeed, thanks.

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/js/webidl-type-mapping-expected.txt', u'LayoutTests/fast/js/webidl-type-mapping.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/js/JSDOMBinding.cpp', u'Source/WebCore/bindings/js/JSDOMBinding.h', u'Source/WebCore/bindings/js/JSDataViewCustom.cpp', u'Source/WebCore/bindings/scripts/CodeGeneratorCPP.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.mm', u'Source/WebCore/bindings/scripts/test/TestObj.idl', u'Source/WebCore/html/canvas/DataView.idl', u'Source/WebCore/testing/TypeConversions.h', u'Source/WebCore/testing/TypeConversions.idl']" exit_code: 1
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:613:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:615:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:616:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:617:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:620:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:622:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:624:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:625:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:626:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:629:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 10 in 22 files


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

[Kentaro Hara]

Comment on attachment 204567 [details]
Patch

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

> Source/WebCore/bindings/js/JSDOMBinding.cpp:315
> +    double x = value.toNumber(exec);

You might want to add a fast path just like toInt64() and toInt32EnforceRange(). The fast path will be something like:

if (value.isInt32()) {
  int32_t value = value.toInt32(exec);
  if (kMinInt8 <= value && value <= kMaxInt8)
    return value;
}

> Source/WebCore/bindings/js/JSDOMBinding.cpp:334
> +    double x = value.toNumber(exec);

Ditto.

[Chris Dumez]

Created attachment 204569 [details]
Patch

Makes sense. I added the fast path for both methods, as suggested.

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/js/webidl-type-mapping-expected.txt', u'LayoutTests/fast/js/webidl-type-mapping.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/js/JSDOMBinding.cpp', u'Source/WebCore/bindings/js/JSDOMBinding.h', u'Source/WebCore/bindings/js/JSDataViewCustom.cpp', u'Source/WebCore/bindings/scripts/CodeGeneratorCPP.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.mm', u'Source/WebCore/bindings/scripts/test/TestObj.idl', u'Source/WebCore/html/canvas/DataView.idl', u'Source/WebCore/testing/TypeConversions.h', u'Source/WebCore/testing/TypeConversions.idl']" exit_code: 1
Source/WebCore/bindings/js/JSDOMBinding.cpp:318:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/js/JSDOMBinding.cpp:343:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:613:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:615:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:616:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:617:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:620:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:622:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:624:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:625:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:626:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:629:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 12 in 22 files


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

[Kentaro Hara]

Comment on attachment 204569 [details]
Patch

Looks OK.

[Chris Dumez]

Created attachment 204570 [details]
Patch for landing

Fixed style issue in JSDOMBinding.cpp.

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/js/webidl-type-mapping-expected.txt', u'LayoutTests/fast/js/webidl-type-mapping.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/js/JSDOMBinding.cpp', u'Source/WebCore/bindings/js/JSDOMBinding.h', u'Source/WebCore/bindings/js/JSDataViewCustom.cpp', u'Source/WebCore/bindings/scripts/CodeGeneratorCPP.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.mm', u'Source/WebCore/bindings/scripts/test/TestObj.idl', u'Source/WebCore/html/canvas/DataView.idl', u'Source/WebCore/testing/TypeConversions.h', u'Source/WebCore/testing/TypeConversions.idl']" exit_code: 1
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:613:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:615:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:616:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:617:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:620:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:622:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:624:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:625:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:626:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:629:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 10 in 22 files


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

[Build Bot]

Comment on attachment 204570 [details]
Patch for landing

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

[Chris Dumez]

Created attachment 204584 [details]
Patch

I have improved the fast code path for the toInt8 / toUInt8 methods:
- Use asInt32() / asUInt32() instead of toInt32() / toUInt32() to avoid double isInt32() / isUInt32() check.
- Handle cases where the value is an integer but not in the range to avoid falling back to the slow path and convert to a number again

I have also exported the symbols for mac.

Would you mind taking another look please?

[WebKit Commit Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/js/webidl-type-mapping-expected.txt', u'LayoutTests/fast/js/webidl-type-mapping.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/bindings/js/JSDOMBinding.cpp', u'Source/WebCore/bindings/js/JSDOMBinding.h', u'Source/WebCore/bindings/js/JSDataViewCustom.cpp', u'Source/WebCore/bindings/scripts/CodeGeneratorCPP.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp', u'Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.h', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.h', u'Source/WebCore/bindings/scripts/test/ObjC/DOMTestObj.mm', u'Source/WebCore/bindings/scripts/test/TestObj.idl', u'Source/WebCore/html/canvas/DataView.idl', u'Source/WebCore/testing/TypeConversions.h', u'Source/WebCore/testing/TypeConversions.idl']" exit_code: 1
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:613:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:615:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:616:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:617:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:620:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:622:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:624:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:625:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:626:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:629:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 10 in 23 files


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

[Kentaro Hara]

Comment on attachment 204584 [details]
Patch

Looks better!

[WebKit Commit Bot]

Comment on attachment 204584 [details]
Patch

Clearing flags on attachment: 204584

Committed r151563: <http://trac.webkit.org/changeset/151563>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.