Add support for byte and octet Web IDL types to the bindings generator

Add support for byte and octet Web IDL types to the bindings generator

Add support for byte and octet Web IDL types to the bindings generator:
- http://dev.w3.org/2006/webapi/WebIDL/#idl-byte
- http://dev.w3.org/2006/webapi/WebIDL/#idl-octet

Get rid of custom code for DataView's getInt8, getUInt8, setInt8, setUInt8
methods and use the newly supported byte / octet types.

Updated fast/js/webidl-type-mapping.html layout test and the bindings test
to cover byte and octet types.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=152423

[arv (Not doing code reviews), 2013-06-13 14:41:13]

LGTM

Joshua, please take a look as well.

[haraken, 2013-06-13 14:45:30]

LGTM

https://codereview.chromium.org/16951005/diff/1/Source/bindings/scripts/CodeG...
File Source/bindings/scripts/CodeGeneratorV8.pm (right):

https://codereview.chromium.org/16951005/diff/1/Source/bindings/scripts/CodeG...
Source/bindings/scripts/CodeGeneratorV8.pm:4813: return "int" if $type eq "long"
or $type eq "int" or $type eq "short" or $type eq "unsigned short" or $type eq
"byte";

Nit: Would you move the "unsigned short" to the below branch so that it returns
"unsigned"? It's confusing.

https://codereview.chromium.org/16951005/diff/1/Source/bindings/v8/V8Binding.cpp
File Source/bindings/v8/V8Binding.cpp (right):

https://codereview.chromium.org/16951005/diff/1/Source/bindings/v8/V8Binding....
Source/bindings/v8/V8Binding.cpp:182: if (value->IsUint32()) {

I'd remove this branch. The range that IsInt32() cannot cover but IsUint32() can
cover is just [2^31, 2^32). It wouldn't be worth adding a fast path for such
uncommon cases.

[jsbell, 2013-06-13 16:51:30]

lgtm as well with a few suggestions

https://codereview.chromium.org/16951005/diff/8001/LayoutTests/fast/js/webidl...
File LayoutTests/fast/js/webidl-type-mapping-expected.txt (right):

https://codereview.chromium.org/16951005/diff/8001/LayoutTests/fast/js/webidl...
LayoutTests/fast/js/webidl-type-mapping-expected.txt:566: PASS
converter.testByte is 0x7F
It would be nice to have tests for values well outside the range to demonstrate
the modulus behavior, e.g. 0x1234 -> 0x34

https://codereview.chromium.org/16951005/diff/8001/LayoutTests/fast/js/webidl...
LayoutTests/fast/js/webidl-type-mapping-expected.txt:665: PASS
converter.testOctet is 0x7F
Ditto.

https://codereview.chromium.org/16951005/diff/8001/Source/bindings/v8/V8Bindi...
File Source/bindings/v8/V8Binding.cpp (right):

https://codereview.chromium.org/16951005/diff/8001/Source/bindings/v8/V8Bindi...
Source/bindings/v8/V8Binding.cpp:154: return static_cast<int8_t>(result >
kMaxInt8 ? result - 256 : result);
These two lines can be simplified to just: return static_cast<int8_t>(result); -
integral conversions have the modulus behavior built-in.
(http://en.cppreference.com/w/cpp/language/implicit_cast)

https://codereview.chromium.org/16951005/diff/8001/Source/bindings/v8/V8Bindi...
Source/bindings/v8/V8Binding.cpp:174: return static_cast<int8_t>(numberValue >
kMaxInt8 ? numberValue - 256 : numberValue);
Alternately, this line could be: return
static_cast<int8_t>(static_cast<uint8_t>(numberValue));

https://codereview.chromium.org/16951005/diff/8001/Source/bindings/v8/V8Bindi...
Source/bindings/v8/V8Binding.cpp:190: return static_cast<uint8_t>(result % 256);
// 2^8.
The modulus should not be necessary here.

[do-not-use, 2013-06-13 19:11:19]

https://codereview.chromium.org/16951005/diff/8001/LayoutTests/fast/js/webidl...
File LayoutTests/fast/js/webidl-type-mapping-expected.txt (right):

https://codereview.chromium.org/16951005/diff/8001/LayoutTests/fast/js/webidl...
LayoutTests/fast/js/webidl-type-mapping-expected.txt:566: PASS
converter.testByte is 0x7F
On 2013/06/13 16:51:31, jsbell wrote:
> It would be nice to have tests for values well outside the range to
demonstrate
> the modulus behavior, e.g. 0x1234 -> 0x34

Ok.

https://codereview.chromium.org/16951005/diff/8001/Source/bindings/v8/V8Bindi...
File Source/bindings/v8/V8Binding.cpp (right):

https://codereview.chromium.org/16951005/diff/8001/Source/bindings/v8/V8Bindi...
Source/bindings/v8/V8Binding.cpp:154: return static_cast<int8_t>(result >
kMaxInt8 ? result - 256 : result);
On 2013/06/13 16:51:31, jsbell wrote:
> These two lines can be simplified to just: return static_cast<int8_t>(result);
-
> integral conversions have the modulus behavior built-in.
> (http://en.cppreference.com/w/cpp/language/implicit_cast)

AFAIK, modulus behavior is only guaranteed for unsigned types. For signed types,
the behavior is implementation specific. From your link:
- If the destination type is unsigned, the resulting value is the value modulo
2n
- If the destination type is signed, the value does not change if the source
integer can be represented in the destination type. Otherwise the result is
implementation-defined.

https://codereview.chromium.org/16951005/diff/8001/Source/bindings/v8/V8Bindi...
Source/bindings/v8/V8Binding.cpp:190: return static_cast<uint8_t>(result % 256);
// 2^8.
On 2013/06/13 16:51:31, jsbell wrote:
> The modulus should not be necessary here.

Yes, here it is not technically required since the type is unsigned.

[jsbell, 2013-06-13 19:14:23]

https://codereview.chromium.org/16951005/diff/8001/Source/bindings/v8/V8Bindi...
File Source/bindings/v8/V8Binding.cpp (right):

https://codereview.chromium.org/16951005/diff/8001/Source/bindings/v8/V8Bindi...
Source/bindings/v8/V8Binding.cpp:154: return static_cast<int8_t>(result >
kMaxInt8 ? result - 256 : result);
On 2013/06/13 19:11:20, Christophe Dumez wrote:
> On 2013/06/13 16:51:31, jsbell wrote:
> > These two lines can be simplified to just: return
static_cast<int8_t>(result);
> -
> > integral conversions have the modulus behavior built-in.
> > (http://en.cppreference.com/w/cpp/language/implicit_cast)
> 
> AFAIK, modulus behavior is only guaranteed for unsigned types. For signed
types,
> the behavior is implementation specific. From your link:
> - If the destination type is unsigned, the resulting value is the value modulo
> 2n
> - If the destination type is signed, the value does not change if the source
> integer can be represented in the destination type. Otherwise the result is
> implementation-defined. 

Right, sorry. Carry on!

[do-not-use, 2013-06-13 19:35:56]

I updated the patch to take Joshua's feedback into consideration (Patch Set 3).

[commit-bot: I haz the power, 2013-06-14 06:00:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/16951005/15001

[commit-bot: I haz the power, 2013-06-14 07:39:00]

Change committed as 152423