Added Uint8ClampedList. COmpielr optimziations to follow in next CL.

Added Uint8ClampedList. COmpielr optimziations to follow in next CL.
Committed: https://code.google.com/p/dart/source/detail?r=15806

[cshapiro, 2012-12-05 23:01:49]

https://codereview.chromium.org/11437028/diff/19/runtime/lib/byte_array.dart
File runtime/lib/byte_array.dart (right):

https://codereview.chromium.org/11437028/diff/19/runtime/lib/byte_array.dart#...
runtime/lib/byte_array.dart:47: [int start = 0, int length]) {
Is this indented correctly?  I would assume the opening square bracket should
line up with the opening parenthesis.

https://codereview.chromium.org/11437028/diff/19/runtime/lib/byte_array.dart#...
runtime/lib/byte_array.dart:539: 
extra vertical space?

https://codereview.chromium.org/11437028/diff/19/runtime/lib/byte_array.dart#...
runtime/lib/byte_array.dart:567: _setIndexed(index, _toUint8(value));
Why not replace lines 562-566 with a new function called "_toClampedUint8". 
This function would be used in place of the _toUint8 call which is mostly made
redundant by the range narrowing above.

Here is the clamp specification

http://dev.w3.org/2006/webapi/WebIDL/#Clamp

https://codereview.chromium.org/11437028/diff/19/runtime/lib/byte_array.dart#...
runtime/lib/byte_array.dart:582: if (from is _Uint8ClampedArray || from is
_ExternalUint8ClampedArray) {
I think _Uint8Array and _ExternalUint8Array are valid here.  By extension, the
setRange for _Uint8Array should accept the clamped arrays.

(The right thing to do might be to have this keyed off of the _BYTES_PER_ELEMENT
value of the from array rather than the type name.)

https://codereview.chromium.org/11437028/diff/19/runtime/vm/class_finalizer.cc
File runtime/vm/class_finalizer.cc (right):

https://codereview.chromium.org/11437028/diff/19/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:327: (cls.library() !=
Library::ScalarlistLibrary())) {
Why this new code?

https://codereview.chromium.org/11437028/diff/19/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/11437028/diff/19/runtime/vm/dart_api_impl.cc#...
runtime/vm/dart_api_impl.cc:2100: obj.IsUint8ClampedArray() ||
obj.IsExternalUint8ClampedArray()) {
Maybe add the Int8Array cases for symmetry like on line 2014?

https://codereview.chromium.org/11437028/diff/19/runtime/vm/object_test.cc
File runtime/vm/object_test.cc (right):

https://codereview.chromium.org/11437028/diff/19/runtime/vm/object_test.cc#ne...
runtime/vm/object_test.cc:1959: {
Rather than adding a nested scope, how about splitting this out into another
test?

https://codereview.chromium.org/11437028/diff/19/runtime/vm/object_test.cc#ne...
runtime/vm/object_test.cc:2186: {
ditto, here and below

https://codereview.chromium.org/11437028/diff/19/runtime/vm/raw_object.cc
File runtime/vm/raw_object.cc (right):

https://codereview.chromium.org/11437028/diff/19/runtime/vm/raw_object.cc#new...
runtime/vm/raw_object.cc:890: 
extra blank line

https://codereview.chromium.org/11437028/diff/19/tests/standalone/byte_array_...
File tests/standalone/byte_array_test.dart (right):

https://codereview.chromium.org/11437028/diff/19/tests/standalone/byte_array_...
tests/standalone/byte_array_test.dart:27: {
Create a new test?

https://codereview.chromium.org/11437028/diff/19/tests/standalone/byte_array_...
tests/standalone/byte_array_test.dart:63: {
Same here

[siva, 2012-12-06 01:31:13]

DBC

https://codereview.chromium.org/11437028/diff/19/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/11437028/diff/19/runtime/vm/object.h#newcode4668
runtime/vm/object.h:4668: class Uint8ClampedArray : public ByteArray {
If we make RawUint8ClampedArray extend RawUint8Array
then we could also have
Uint8ClampedArray extend Uint8Array
(it would be very similar to the ImmutableArray model)
Only the New methods would be special cased.

https://codereview.chromium.org/11437028/diff/19/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

https://codereview.chromium.org/11437028/diff/19/runtime/vm/raw_object.h#newc...
runtime/vm/raw_object.h:1279: class RawUint8ClampedArray : public RawByteArray {
I would make this a subclass of RawUint8Array
and not have the data_ field here.

https://codereview.chromium.org/11437028/diff/19/runtime/vm/raw_object.h#newc...
runtime/vm/raw_object.h:1404: class RawExternalUint8ClampedArray : public
RawByteArray {
Ditto.

https://codereview.chromium.org/11437028/diff/19/runtime/vm/raw_object.h#newc...
runtime/vm/raw_object.h:1410: friend class RawTokenStream;
Why are these friend class stuff needed here?

[srdjan, 2012-12-06 19:23:14]

https://codereview.chromium.org/11437028/diff/19/runtime/lib/byte_array.dart
File runtime/lib/byte_array.dart (right):

https://codereview.chromium.org/11437028/diff/19/runtime/lib/byte_array.dart#...
runtime/lib/byte_array.dart:47: [int start = 0, int length]) {
On 2012/12/05 23:01:49, cshapiro wrote:
> Is this indented correctly?  I would assume the opening square bracket should
> line up with the opening parenthesis.

Done.

https://codereview.chromium.org/11437028/diff/19/runtime/lib/byte_array.dart#...
runtime/lib/byte_array.dart:539: 
On 2012/12/05 23:01:49, cshapiro wrote:
> extra vertical space?

Done.

https://codereview.chromium.org/11437028/diff/19/runtime/lib/byte_array.dart#...
runtime/lib/byte_array.dart:567: _setIndexed(index, _toUint8(value));
On 2012/12/05 23:01:49, cshapiro wrote:
> Why not replace lines 562-566 with a new function called "_toClampedUint8". 
> This function would be used in place of the _toUint8 call which is mostly made
> redundant by the range narrowing above.
> 
> Here is the clamp specification
> 
> http://dev.w3.org/2006/webapi/WebIDL/#Clamp

Done.

https://codereview.chromium.org/11437028/diff/19/runtime/lib/byte_array.dart#...
runtime/lib/byte_array.dart:582: if (from is _Uint8ClampedArray || from is
_ExternalUint8ClampedArray) {
On 2012/12/05 23:01:49, cshapiro wrote:
> I think _Uint8Array and _ExternalUint8Array are valid here.  By extension, the
> setRange for _Uint8Array should accept the clamped arrays.
> 
> (The right thing to do might be to have this keyed off of the
_BYTES_PER_ELEMENT
> value of the from array rather than the type name.)

We need to talk more about that.

https://codereview.chromium.org/11437028/diff/19/runtime/vm/class_finalizer.cc
File runtime/vm/class_finalizer.cc (right):

https://codereview.chromium.org/11437028/diff/19/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:327: (cls.library() !=
Library::ScalarlistLibrary())) {
On 2012/12/05 23:01:49, cshapiro wrote:
> Why this new code?

Forgot to remove.

https://codereview.chromium.org/11437028/diff/19/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/11437028/diff/19/runtime/vm/dart_api_impl.cc#...
runtime/vm/dart_api_impl.cc:2100: obj.IsUint8ClampedArray() ||
obj.IsExternalUint8ClampedArray()) {
On 2012/12/05 23:01:49, cshapiro wrote:
> Maybe add the Int8Array cases for symmetry like on line 2014?

The incoming type is uint8_t not int8_t?

https://codereview.chromium.org/11437028/diff/19/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/11437028/diff/19/runtime/vm/object.h#newcode4668
runtime/vm/object.h:4668: class Uint8ClampedArray : public ByteArray {
On 2012/12/06 01:31:13, siva wrote:
> If we make RawUint8ClampedArray extend RawUint8Array
> then we could also have
> Uint8ClampedArray extend Uint8Array
> (it would be very similar to the ImmutableArray model)
> Only the New methods would be special cased.

Done.

https://codereview.chromium.org/11437028/diff/19/runtime/vm/object_test.cc
File runtime/vm/object_test.cc (right):

https://codereview.chromium.org/11437028/diff/19/runtime/vm/object_test.cc#ne...
runtime/vm/object_test.cc:1959: {
On 2012/12/05 23:01:49, cshapiro wrote:
> Rather than adding a nested scope, how about splitting this out into another
> test?

Done.

https://codereview.chromium.org/11437028/diff/19/runtime/vm/object_test.cc#ne...
runtime/vm/object_test.cc:2186: {
On 2012/12/05 23:01:49, cshapiro wrote:
> ditto, here and below

Done.

https://codereview.chromium.org/11437028/diff/19/runtime/vm/raw_object.cc
File runtime/vm/raw_object.cc (right):

https://codereview.chromium.org/11437028/diff/19/runtime/vm/raw_object.cc#new...
runtime/vm/raw_object.cc:890: 
On 2012/12/05 23:01:49, cshapiro wrote:
> extra blank line

Done.

https://codereview.chromium.org/11437028/diff/19/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

https://codereview.chromium.org/11437028/diff/19/runtime/vm/raw_object.h#newc...
runtime/vm/raw_object.h:1279: class RawUint8ClampedArray : public RawByteArray {
On 2012/12/06 01:31:13, siva wrote:
> I would make this a subclass of RawUint8Array
> and not have the data_ field here.

Done.

https://codereview.chromium.org/11437028/diff/19/runtime/vm/raw_object.h#newc...
runtime/vm/raw_object.h:1404: class RawExternalUint8ClampedArray : public
RawByteArray {
On 2012/12/06 01:31:13, siva wrote:
> Ditto.

Done.

https://codereview.chromium.org/11437028/diff/19/runtime/vm/raw_object.h#newc...
runtime/vm/raw_object.h:1410: friend class RawTokenStream;
On 2012/12/06 01:31:13, siva wrote:
> Why are these friend class stuff needed here?

Removed.

https://codereview.chromium.org/11437028/diff/19/tests/standalone/byte_array_...
File tests/standalone/byte_array_test.dart (right):

https://codereview.chromium.org/11437028/diff/19/tests/standalone/byte_array_...
tests/standalone/byte_array_test.dart:27: {
On 2012/12/05 23:01:49, cshapiro wrote:
> Create a new test?

Done.

https://codereview.chromium.org/11437028/diff/19/tests/standalone/byte_array_...
tests/standalone/byte_array_test.dart:63: {
On 2012/12/05 23:01:49, cshapiro wrote:
> Same here

Done.

[cshapiro, 2012-12-06 21:17:46]

lgtm