Bug 117547

Summary:

Support byte and octet types in bindings generators

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

Bindings

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, cgarcia, commit-queue, darin, esprehn+autocc, ggaren, haraken, laszlo.gombos, mrobinson, rniwa

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

URL:

http://www.khronos.org/registry/typedarray/specs/latest/#8

Attachments:

Description	Flags
WIP Patch	none
Patch	haraken: review-
JSDataView.cpp diff	none
Patch	none
Patch	none
Patch	haraken: review+
Patch for landing	buildbot: commit-queue-
Patch	none

Chris Dumez

Reported 2013-06-12 08:34:11 PDT

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.

Attachments
WIP Patch (39.42 KB, patch) 2013-06-12 09:07 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (40.99 KB, patch) 2013-06-12 09:55 PDT, Chris Dumez	haraken: review-	Details Formatted Diff Diff
JSDataView.cpp diff (6.36 KB, patch) 2013-06-12 09:55 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (49.03 KB, patch) 2013-06-13 01:02 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (58.13 KB, patch) 2013-06-13 03:39 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (58.45 KB, patch) 2013-06-13 04:02 PDT, Chris Dumez	haraken: review+	Details Formatted Diff Diff
Patch for landing (58.45 KB, patch) 2013-06-13 04:08 PDT, Chris Dumez	buildbot: commit-queue-	Details Formatted Diff Diff
Patch (60.10 KB, patch) 2013-06-13 07:24 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.

Chris Dumez

Comment 1 2013-06-12 09:07:10 PDT

Created attachment 204445 [details] WIP Patch

WebKit Commit Bot

Comment 2 2013-06-12 09:09:39 PDT

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

Comment 3 2013-06-12 09:55:06 PDT

Created attachment 204452 [details] Patch

Chris Dumez

Comment 4 2013-06-12 09:55:36 PDT

Created attachment 204453 [details] JSDataView.cpp diff

WebKit Commit Bot

Comment 5 2013-06-12 09:58:05 PDT

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 6 2013-06-12 17:26:22 PDT

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

Comment 7 2013-06-12 22:37:42 PDT

(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

Comment 8 2013-06-12 22:46:45 PDT

(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

Comment 9 2013-06-12 23:09:33 PDT

(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

Comment 10 2013-06-13 00:41:10 PDT

(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

Comment 11 2013-06-13 00:43:26 PDT

(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

Comment 12 2013-06-13 00:45:32 PDT

(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

Comment 13 2013-06-13 01:02:02 PDT

Created attachment 204559 [details] Patch

Chris Dumez

Comment 14 2013-06-13 01:02:51 PDT

(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

Comment 15 2013-06-13 01:04:03 PDT

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 16 2013-06-13 01:04:28 PDT

Comment on attachment 204559 [details] Patch Clearing flags until I add checks to LayoutTests/fast/js/webidl-type-mapping.html.

Chris Dumez

Comment 17 2013-06-13 03:39:22 PDT

Created attachment 204567 [details] Patch Add tests to fast/js/webidl-type-mapping.html. Much better indeed, thanks.

WebKit Commit Bot

Comment 18 2013-06-13 03:41:41 PDT

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 19 2013-06-13 03:48:38 PDT

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

Comment 20 2013-06-13 04:02:00 PDT

Created attachment 204569 [details] Patch Makes sense. I added the fast path for both methods, as suggested.

WebKit Commit Bot

Comment 21 2013-06-13 04:03:25 PDT

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 22 2013-06-13 04:05:36 PDT

Comment on attachment 204569 [details] Patch Looks OK.

Chris Dumez

Comment 23 2013-06-13 04:08:33 PDT

Created attachment 204570 [details] Patch for landing Fixed style issue in JSDOMBinding.cpp.

WebKit Commit Bot

Comment 24 2013-06-13 04:10:21 PDT

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 25 2013-06-13 04:57:31 PDT

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

Comment 26 2013-06-13 07:24:13 PDT

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

Comment 27 2013-06-13 07:25:30 PDT

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 28 2013-06-13 07:33:25 PDT

Comment on attachment 204584 [details] Patch Looks better!

WebKit Commit Bot

Comment 29 2013-06-13 11:13:36 PDT

Comment on attachment 204584 [details] Patch Clearing flags on attachment: 204584 Committed r151563: <http://trac.webkit.org/changeset/151563>

WebKit Commit Bot

Comment 30 2013-06-13 11:13:41 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.