Make writes consistent across socket and file synchronous/asynchronus writes in terms of truncation…

Make writes consistent across socket and file synchronous/asynchronus writes in terms of truncation semantics (reports an argument error if the list element contains a non byte value).

Remove Common_IsBuiltinList as it is not used anymore

Change synchronous file writes to use Dart_TypedDataAcquireData and
Dart_TypedDataReleaseData since the buffer object is always a Uint8List
instead of the more expensive Dart_ListGetAsBytes.

R=ajohnsen@google.com, iposva@google.com

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

[Anders Johnsen, 2013-06-28 05:21:48]

LGTM. What was the motivation for this change?

https://codereview.chromium.org/18115002/diff/9002/tests/standalone/io/file_t...
File tests/standalone/io/file_test.dart (left):

https://codereview.chromium.org/18115002/diff/9002/tests/standalone/io/file_t...
tests/standalone/io/file_test.dart:532: openedFile.writeFrom([260], 0, 1);  //
260 = 256 + 4 = 0x104.
If this is illegal now, please add a test for it.

[siva, 2013-06-28 18:44:56]

This change essentially started with trying to eliminate
all uses of Dart_GetClass in the embedder code. As part of that we noticed that
there were some inconsistency in the
way we were writing out data:
socket writes - seems to always support only byte elements
file writes - seems to always truncates elements to a byte
              when synchronous writes are used
            - seems to truncate elements to a byte for user
              defined list, _ObjectArray, _ImmutableArray
              or _GrowableObjectArray lists but does not
              truncate for typed_data lists.
This CL makes them consistent and eliminates use of
Dart_GetClass

https://codereview.chromium.org/18115002/diff/9002/tests/standalone/io/file_t...
File tests/standalone/io/file_test.dart (left):

https://codereview.chromium.org/18115002/diff/9002/tests/standalone/io/file_t...
tests/standalone/io/file_test.dart:532: openedFile.writeFrom([260], 0, 1);  //
260 = 256 + 4 = 0x104.
On 2013/06/28 05:21:48, Anders Johnsen wrote:
> If this is illegal now, please add a test for it.

Writing values > 255 throws an ArgumentError which is what the socket write
function was doing. So this behaviour is now
replicated to the file write function also for consistency.

I will add a negative test which ensures that an ArgumentError is
thrown if values > 255 are written.

[Ivan Posva, 2013-06-28 23:10:13]

LGTM with comment about truncation.

-Ivan

https://codereview.chromium.org/18115002/diff/9002/sdk/lib/io/common.dart
File sdk/lib/io/common.dart (right):

https://codereview.chromium.org/18115002/diff/9002/sdk/lib/io/common.dart#new...
sdk/lib/io/common.dart:114: value < 0 || 255 < value) {
I was under the impression that we agree on this working at all times:

var l = rnd.nextBool() ? new Uint8List(1) : new List(1);
l[0] = 260;
file.writeFromSync(l, 0, 1);

[siva, 2013-06-29 18:19:01]

https://codereview.chromium.org/18115002/diff/9002/sdk/lib/io/common.dart
File sdk/lib/io/common.dart (right):

https://codereview.chromium.org/18115002/diff/9002/sdk/lib/io/common.dart#new...
sdk/lib/io/common.dart:114: value < 0 || 255 < value) {
I have changed this to truncate instead of throwing an exception.

I have commented out the two testOutofRange tests in
tests/standalone/io/file_typed_data_test.dart
tests/standalone/io/web_socket_typed_data_test.dart
which were testing for this condition


On 2013/06/28 23:10:13, Ivan Posva wrote:
> I was under the impression that we agree on this working at all times:
> 
> var l = rnd.nextBool() ? new Uint8List(1) : new List(1);
> l[0] = 260;
> file.writeFromSync(l, 0, 1);

[siva, 2013-06-29 18:19:39]

Committed patchset #5 manually as r24620 (presubmit successful).