117547 – Support byte and octet types in bindings generators

Bug 117547 - Support byte and octet types in bindings generators

Summary: Support byte and octet types in bindings generators

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	Bindings (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Chris Dumez

URL:	http://www.khronos.org/registry/typed...
Keywords:

Depends on:
Blocks:

Reported:	2013-06-12 08:34 PDT by Chris Dumez
Modified:	2013-06-13 11:13 PDT (History)
CC List:	10 users (show)

See Also:

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 an attachment (proposed patch, testcase, etc.)

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

Description Chris Dumez 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.

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

Created attachment 204445 [details]
WIP Patch

Comment 2 WebKit Commit Bot 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.

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

Created attachment 204452 [details]
Patch

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

Created attachment 204453 [details]
JSDataView.cpp diff

Comment 5 WebKit Commit Bot 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.

Comment 6 Kentaro Hara 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).

Comment 7 Chris Dumez 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.

Comment 8 Kentaro Hara 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.

Comment 9 Chris Dumez 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

Comment 10 Chris Dumez 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.

Comment 11 Kentaro Hara 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.

Comment 12 Kentaro Hara 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.

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

Created attachment 204559 [details]
Patch

Comment 14 Chris Dumez 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.

Comment 15 WebKit Commit Bot 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.

Comment 16 Chris Dumez 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.

Comment 17 Chris Dumez 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.

Comment 18 WebKit Commit Bot 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.

Comment 19 Kentaro Hara 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.

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

Created attachment 204569 [details]
Patch

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

Comment 21 WebKit Commit Bot 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.

Comment 22 Kentaro Hara 2013-06-13 04:05:36 PDT

Comment on attachment 204569 [details]
Patch

Looks OK.

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

Created attachment 204570 [details]
Patch for landing

Fixed style issue in JSDOMBinding.cpp.

Comment 24 WebKit Commit Bot 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.

Comment 25 Build Bot 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

Comment 26 Chris Dumez 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?

Comment 27 WebKit Commit Bot 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.

Comment 28 Kentaro Hara 2013-06-13 07:33:25 PDT

Comment on attachment 204584 [details]
Patch

Looks better!

Comment 29 WebKit Commit Bot 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>

Comment 30 WebKit Commit Bot 2013-06-13 11:13:41 PDT

All reviewed patches have been landed.  Closing bug.