- Use dart:typedata types in the Dart API calls.

- Use dart:typedata types in the Dart API calls.
- Change the dart/io code to not use dart:scalarlist
Committed: https://code.google.com/p/dart/source/detail?r=19848

[Mads Ager (google), 2013-03-12 08:18:03]

LGTM!

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
File runtime/lib/typeddata.dart (right):

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
runtime/lib/typeddata.dart:1219: List<int> _createList(int length) {
List<int> -> Int8List?

Similarly for the ones below?

[Anton Muhin, 2013-03-12 08:31:46]

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
File runtime/lib/typeddata.dart (right):

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
runtime/lib/typeddata.dart:1786: : super(),
nit: as per style guide, shouldn't super ctor invocation go to the end of the
list?  and do you need to invoke default ctor explicitly at all?

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/vm/dart_api...
File runtime/vm/dart_api_message.cc (right):

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/vm/dart_api...
runtime/vm/dart_api_message.cc:434: case kExternalTypedDataUint8ArrayCid: {
I am not familiar with this part of the system, but apparently external array is
serialized as a pointer to data, not data itself, but apparently again here you
expect all the data back, is it ok?

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/vm/raw_obje...
File runtime/vm/raw_object_snapshot.cc (right):

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/vm/raw_obje...
runtime/vm/raw_object_snapshot.cc:2481: obj.AddFinalizer(peer, callback);
sorry, I am again not sure if it's a valid concern, but have to ask.

So if I send an external array to some different isolate, won't that create a
new weak handle on the receiving end with the same peer and callback?

If yes, it seems very problematic, e.g. in DOM that would lead to very subtle
bugs.

[Mads Ager (google), 2013-03-12 08:36:17]

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/vm/dart_api...
File runtime/vm/dart_api_message.cc (right):

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/vm/dart_api...
runtime/vm/dart_api_message.cc:434: case kExternalTypedDataUint8ArrayCid: {
On 2013/03/12 08:31:46, Anton Muhin wrote:
> I am not familiar with this part of the system, but apparently external array
is
> serialized as a pointer to data, not data itself, but apparently again here
you
> expect all the data back, is it ok?

This is a bit complicated. The data is not serialized with the code below. It is
serialized in raw_object_snapshot.cc and both internal and external are
serialized the same.

The serialization for external typed arrays below is read by the snapshot reader
in raw_object_snapshot.cc which handles the pointers correctly.

The message reader is running independently of the VM in separate threads in a
thread pool. It writes a snapshot that the VM snapshot reader can read. It reads
a snapshot that the VM snapshot writer has created.

[Ivan Posva, 2013-03-12 09:32:02]

DBC -ip

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/bin/file_pa...
File runtime/bin/file_patch.dart (right):

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/bin/file_pa...
runtime/bin/file_patch.dart:38: List<int> _makeUint8ListView(List<int> source,
int offsetInBytes, int length) {
Don't you want to add the correct type to source? Since you are accessing
.buffer on it you know that it is at least a TypedData, no?

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
File runtime/lib/typeddata.dart (left):

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
runtime/lib/typeddata.dart:1213: 
?

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
File runtime/lib/typeddata.dart (right):

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
runtime/lib/typeddata.dart:623: 
Missing line.

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
runtime/lib/typeddata.dart:682: 
ditto here and more below.

[siva, 2013-03-12 10:31:46]

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/bin/file_pa...
File runtime/bin/file_patch.dart (right):

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/bin/file_pa...
runtime/bin/file_patch.dart:38: List<int> _makeUint8ListView(List<int> source,
int offsetInBytes, int length) {
Yes, it is an ExternalTypedData of Uint8List, so I changed the type to
Uint8List.

On 2013/03/12 09:32:02, Ivan Posva wrote:
> Don't you want to add the correct type to source? Since you are accessing
> .buffer on it you know that it is at least a TypedData, no?

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
File runtime/lib/typeddata.dart (left):

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
runtime/lib/typeddata.dart:1213: 
On 2013/03/12 09:32:02, Ivan Posva wrote:
> ?

Done.

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
File runtime/lib/typeddata.dart (right):

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
runtime/lib/typeddata.dart:623: 
It seems to have a blank line.

On 2013/03/12 09:32:02, Ivan Posva wrote:
> Missing line.

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
runtime/lib/typeddata.dart:682: 
Seems to have it. The pattern in the file was one blank line between functions
and two when a different interface started.

On 2013/03/12 09:32:02, Ivan Posva wrote:
> ditto here and more below.

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
runtime/lib/typeddata.dart:1219: List<int> _createList(int length) {
Good point. Changed all the return values.

On 2013/03/12 08:18:03, Mads Ager wrote:
> List<int> -> Int8List?
> 
> Similarly for the ones below?

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/lib/typedda...
runtime/lib/typeddata.dart:1786: : super(),
Removed the explicit invoke of the default ctor, I had added it to debug stuff
and forgot to remove it.

On 2013/03/12 08:31:46, Anton Muhin wrote:
> nit: as per style guide, shouldn't super ctor invocation go to the end of the
> list?  and do you need to invoke default ctor explicitly at all?

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/vm/raw_obje...
File runtime/vm/raw_object_snapshot.cc (right):

https://chromiumcodereview.appspot.com/12730013/diff/9001/runtime/vm/raw_obje...
runtime/vm/raw_object_snapshot.cc:2481: obj.AddFinalizer(peer, callback);
Yes your concern is valid.

We never send these direct pointers to another isolate in the regular snapshot
code. If you looks at the write functionality of ExternalTYpedData it sends the
data out explicitly.

The code above comes in only from the native event handler thread which posts
messages directly from C++.

On 2013/03/12 08:31:46, Anton Muhin wrote:
> sorry, I am again not sure if it's a valid concern, but have to ask.
> 
> So if I send an external array to some different isolate, won't that create a
> new weak handle on the receiving end with the same peer and callback?
> 
> If yes, it seems very problematic, e.g. in DOM that would lead to very subtle
> bugs.

[Anton Muhin, 2013-03-12 10:52:38]

lgtm

[siva, 2013-03-12 10:57:56]

Committed patchset #3 manually as r19848 (presubmit successful).