Test for OscillatorNode.type=.

Test for OscillatorNode.type=. Fix bug in StringGetPeerHelper when passed an immediate object. Fix asserts when getting typed data address to allow for empty arrays.

BUG=http://dartbug.com/11984
R=asiva@google.com, vsm@google.com

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

[rmacnak, 2013-10-21 23:44:22]

Counterpart is
https://chromiumcodereview.appspot.com/29783004/

[vsm, 2013-10-21 23:52:47]

lgtm

[siva, 2013-10-22 00:08:53]

https://chromiumcodereview.appspot.com/28973003/diff/30001/runtime/vm/dart_ap...
File runtime/vm/dart_api_impl.cc (right):

https://chromiumcodereview.appspot.com/28973003/diff/30001/runtime/vm/dart_ap...
runtime/vm/dart_api_impl.cc:269: }
Why this is needed?

https://chromiumcodereview.appspot.com/28973003/diff/30001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://chromiumcodereview.appspot.com/28973003/diff/30001/runtime/vm/object....
runtime/vm/object.h:5619: ((byte_offset >= 0) && (byte_offset <
LengthInBytes())));
Not sure what the value of byte_offset is in this case?

[rmacnak, 2013-10-22 00:28:21]

https://chromiumcodereview.appspot.com/28973003/diff/30001/runtime/vm/dart_ap...
File runtime/vm/dart_api_impl.cc (right):

https://chromiumcodereview.appspot.com/28973003/diff/30001/runtime/vm/dart_ap...
runtime/vm/dart_api_impl.cc:269: }
On 2013/10/22 00:08:53, siva wrote:
> Why this is needed?

The assert in RawObject::GetClassId will fail when given a Smi.

Maybe we should be using an Object handle here instead, which does handle Smis.

https://chromiumcodereview.appspot.com/28973003/diff/30001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://chromiumcodereview.appspot.com/28973003/diff/30001/runtime/vm/object....
runtime/vm/object.h:5619: ((byte_offset >= 0) && (byte_offset <
LengthInBytes())));
On 2013/10/22 00:08:53, siva wrote:
> Not sure what the value of byte_offset is in this case?

byte_offset == 0 handles the case where the array is of length 0.

[siva, 2013-10-22 18:26:49]

lgtm

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

https://codereview.chromium.org/28973003/diff/30001/runtime/vm/dart_api_impl....
runtime/vm/dart_api_impl.cc:269: }
The function is supposed to be called only from
Dart_GetNativeStringArgument which expects a string object. Not
sure how a Smi was passed in, do we have a bug in the bindings code somewhere?

On 2013/10/22 00:28:21, Ryan Macnak wrote:
> On 2013/10/22 00:08:53, siva wrote:
> > Why this is needed?
> 
> The assert in RawObject::GetClassId will fail when given a Smi.
> 
> Maybe we should be using an Object handle here instead, which does handle
Smis.

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

https://codereview.chromium.org/28973003/diff/30001/runtime/vm/object.h#newco...
runtime/vm/object.h:5619: ((byte_offset >= 0) && (byte_offset <
LengthInBytes())));
On 2013/10/22 00:28:21, Ryan Macnak wrote:
> On 2013/10/22 00:08:53, siva wrote:
> > Not sure what the value of byte_offset is in this case?
> 
> byte_offset == 0 handles the case where the array is of length 0.

In that case should the assert be changed to
ASSERT((byte_offset == 0) || ((byte_offset > 0) && (byte_offset <
LengthInBytes()));

https://codereview.chromium.org/28973003/diff/30001/runtime/vm/object.h#newco...
runtime/vm/object.h:5752: ((byte_offset >= 0) && (byte_offset <
LengthInBytes())));
Ditto.

[rmacnak, 2013-10-22 18:54:13]

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

https://codereview.chromium.org/28973003/diff/30001/runtime/vm/dart_api_impl....
runtime/vm/dart_api_impl.cc:269: }
On 2013/10/22 18:26:49, siva wrote:
> The function is supposed to be called only from
> Dart_GetNativeStringArgument which expects a string object. Not
> sure how a Smi was passed in, do we have a bug in the bindings code somewhere?

DartUtilities::dartToString and its callers do not check the argument's type.

Dart_GetNativeStringArgument calls StringGetPeerHelper before checking the
argument is a String.

For all non-String, heap objects, StringGetPeerHelper would answer false and
Dart_GetNativeStringArgument would fall through and generate an error handle.
Now we fall through for immediate objects as well.

[rmacnak, 2013-10-23 00:12:21]

Committed patchset #4 manually as r29049 (presubmit successful).