Move to new dart:typeddata types for dart2js

Move to new dart:typeddata types

Mads: if you land this, please rerun go.sh from tools/dom/scripts to regenerate html_dart2js.dart.  I didn't include it here to make this patchable.

Stephen: I still need to run the perf tests we talked about to sanity check.

Committed: https://code.google.com/p/dart/source/detail?r=21789

[vsm, 2013-04-19 06:36:28]

https://codereview.chromium.org/14367012/diff/1/tests/html/url_test.dart
File tests/html/url_test.dart (right):

https://codereview.chromium.org/14367012/diff/1/tests/html/url_test.dart#newc...
tests/html/url_test.dart:28: var dataArray = new
Uint8List.view(arrayBuffer.buffer);
Mads: note I had to add the ".buffer".  It apparently works without that on VM
since the same object implements both Uint8List and ByteBuffer, but they are
different for dart2js.

[Mads Ager (google), 2013-04-19 08:19:40]

On 2013/04/19 06:36:28, vsm wrote:
> https://codereview.chromium.org/14367012/diff/1/tests/html/url_test.dart
> File tests/html/url_test.dart (right):
> 
>
https://codereview.chromium.org/14367012/diff/1/tests/html/url_test.dart#newc...
> tests/html/url_test.dart:28: var dataArray = new
> Uint8List.view(arrayBuffer.buffer);
> Mads: note I had to add the ".buffer".  It apparently works without that on VM
> since the same object implements both Uint8List and ByteBuffer, but they are
> different for dart2js.

Urgh, this completely breaks the Dart bindings because the Dart CPP classes for
typed arrays are no longer generated. I'll see what I can do today to fix that.

[Mads Ager (google), 2013-04-19 13:04:26]

LGTM

I have adapted dartium to this as well. I'm going for landing this Monday
morning Danish time! :-)

https://codereview.chromium.org/14367012/diff/1/tests/html/url_test.dart
File tests/html/url_test.dart (right):

https://codereview.chromium.org/14367012/diff/1/tests/html/url_test.dart#newc...
tests/html/url_test.dart:28: var dataArray = new
Uint8List.view(arrayBuffer.buffer);
On 2013/04/19 06:36:28, vsm wrote:
> Mads: note I had to add the ".buffer".  It apparently works without that on VM
> since the same object implements both Uint8List and ByteBuffer, but they are
> different for dart2js.

Makes sense. Thanks.

[vsm, 2013-04-19 14:42:47]

On 2013/04/19 13:04:26, Mads Ager wrote:
> LGTM
> 
> I have adapted dartium to this as well. I'm going for landing this Monday
> morning Danish time! :-)

Sorry for breaking you!  Glad you were able to fix. Monday sounds good.  :-)

> 
> https://codereview.chromium.org/14367012/diff/1/tests/html/url_test.dart
> File tests/html/url_test.dart (right):
> 
>
https://codereview.chromium.org/14367012/diff/1/tests/html/url_test.dart#newc...
> tests/html/url_test.dart:28: var dataArray = new
> Uint8List.view(arrayBuffer.buffer);
> On 2013/04/19 06:36:28, vsm wrote:
> > Mads: note I had to add the ".buffer".  It apparently works without that on
VM
> > since the same object implements both Uint8List and ByteBuffer, but they are
> > different for dart2js.
> 
> Makes sense. Thanks.

[blois, 2013-04-19 16:24:22]

lgtm

[sra1, 2013-04-19 17:56:55]

https://codereview.chromium.org/14367012/diff/5001/sdk/lib/typeddata/dart2js/...
File sdk/lib/typeddata/dart2js/typeddata_dart2js.dart (right):

https://codereview.chromium.org/14367012/diff/5001/sdk/lib/typeddata/dart2js/...
sdk/lib/typeddata/dart2js/typeddata_dart2js.dart:103: int getInt16(int
byteOffset, [Endianness endian=Endianness.BIG_ENDIAN]) => JS('num',
'#.getInt16(#, #)', this, byteOffset, endian._littleEndian);
This is a better pattern:
----
int getInt16(int byteOffset, [Endianness endian=Endianness.BIG_ENDIAN]) =>
  _getInt16(byteOffset, endian._littleEndian);

@JSName('getInt16')
@Returns('int')
int _getInt16(int byteOffset, [bool littleEndian]) native;
----

The plain native method is easily inlined into the public method, so the code
for getInt16 should be identical, but we have more ability in annotating
methods.

@Returns('int')  - returns a non-null int.
Later we can add range annotations and effect annotations.

For perf tests, you could rename _getInt16 to $dom_getInt16 and compare the
performance of a bool vs enum parameter.

[vsm, 2013-04-19 20:08:48]

On 2013/04/19 17:56:55, sra1 wrote:
>
https://codereview.chromium.org/14367012/diff/5001/sdk/lib/typeddata/dart2js/...
> File sdk/lib/typeddata/dart2js/typeddata_dart2js.dart (right):
> 
>
https://codereview.chromium.org/14367012/diff/5001/sdk/lib/typeddata/dart2js/...
> sdk/lib/typeddata/dart2js/typeddata_dart2js.dart:103: int getInt16(int
> byteOffset, [Endianness endian=Endianness.BIG_ENDIAN]) => JS('num',
> '#.getInt16(#, #)', this, byteOffset, endian._littleEndian);
> This is a better pattern:
> ----
> int getInt16(int byteOffset, [Endianness endian=Endianness.BIG_ENDIAN]) =>
>   _getInt16(byteOffset, endian._littleEndian);
> 
> @JSName('getInt16')
> @Returns('int')
> int _getInt16(int byteOffset, [bool littleEndian]) native;
> ----
> 
> The plain native method is easily inlined into the public method, so the code
> for getInt16 should be identical, but we have more ability in annotating
> methods.
> 
> @Returns('int')  - returns a non-null int.
> Later we can add range annotations and effect annotations.
> 
> For perf tests, you could rename _getInt16 to $dom_getInt16 and compare the
> performance of a bool vs enum parameter.

Modified the CL to use Stephen's suggested pattern instead.

[sra1, 2013-04-19 20:48:37]

lgtm

https://codereview.chromium.org/14367012/diff/8002/sdk/lib/typeddata/dart2js/...
File sdk/lib/typeddata/dart2js/typeddata_dart2js.dart (right):

https://codereview.chromium.org/14367012/diff/8002/sdk/lib/typeddata/dart2js/...
sdk/lib/typeddata/dart2js/typeddata_dart2js.dart:100: double _getFloat32(int
byteOffset, [bool littleEndian]) native;
I'd keep the type (both) as 'num' here, since

data.getFloat32(0) is int

might be true (e.g. 0.0)

[vsm, 2013-04-22 05:46:57]

Fixed this last issue and rebased to r21786.

On 2013/04/19 20:48:37, sra1 wrote:
> lgtm
> 
>
https://codereview.chromium.org/14367012/diff/8002/sdk/lib/typeddata/dart2js/...
> File sdk/lib/typeddata/dart2js/typeddata_dart2js.dart (right):
> 
>
https://codereview.chromium.org/14367012/diff/8002/sdk/lib/typeddata/dart2js/...
> sdk/lib/typeddata/dart2js/typeddata_dart2js.dart:100: double _getFloat32(int
> byteOffset, [bool littleEndian]) native;
> I'd keep the type (both) as 'num' here, since
> 
> data.getFloat32(0) is int
> 
> might be true (e.g. 0.0)

[Mads Ager (google), 2013-04-22 08:23:20]

Committed patchset #5 manually as r21789.

[kasperl, 2013-04-23 11:38:32]

https://codereview.chromium.org/14367012/diff/17001/sdk/lib/typeddata/dart2js...
File sdk/lib/typeddata/dart2js/typeddata_dart2js.dart (right):

https://codereview.chromium.org/14367012/diff/17001/sdk/lib/typeddata/dart2js...
sdk/lib/typeddata/dart2js/typeddata_dart2js.dart:236: num operator[](int index)
=> JS("num", "#[#]", this, index);
All the [] and []= operators seem to be missing bounds checks.

https://codereview.chromium.org/14367012/diff/17001/sdk/lib/typeddata/dart2js...
sdk/lib/typeddata/dart2js/typeddata_dart2js.dart:318: throw new
UnsupportedError("Cannot add to immutable List.");
This isn't an immutable list.