Never return a Dart_Handle on a dart::Class from the embedding API.

Never return a Dart_Handle on a dart::Class from the embedding API.

R=asiva@google.com

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

[rmacnak, 2013-09-19 00:46:04]

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

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/raw_object.h#ne...
runtime/vm/raw_object.h:350: bool IsRawClass() {
Will remove before commit, just here to demonstrate.

[siva, 2013-09-19 03:16:57]

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

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/dart_api_impl.c...
runtime/vm/dart_api_impl.cc:2826: result = Api::UnwrapHandle(type);
You should be able to use 
const Type& type_obj = Api::UnwrapTypeHandle(type);
(this will return a NULL if the object is not a type and the subsequent check
will report the error. You don't have to then again do the check
if (result.IsType()) further down.

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/dart_api_impl.c...
runtime/vm/dart_api_impl.cc:2835: cls = Type::Cast(result).type_class();
cls = type_obj.type_class();

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/dart_api_impl.c...
runtime/vm/dart_api_impl.cc:2944: const Object& result = Object::Handle(isolate,
Api::UnwrapHandle(type));
Ditto comment about
const Type& result = Api::UnwrapTypeHandle(type);

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/dart_api_impl.c...
runtime/vm/dart_api_impl.cc:2952: cls = Type::Cast(result).type_class();
const Class& cls = Class::Handle(isolate, result.type_class());

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/dart_api_impl_t...
File runtime/vm/dart_api_impl_test.cc (right):

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/dart_api_impl_t...
runtime/vm/dart_api_impl_test.cc:214: }
Why is this change in this CL, looks like it is part of the other CL you had.

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/debugger_api_im...
File runtime/vm/debugger_api_impl.cc (right):

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/debugger_api_im...
runtime/vm/debugger_api_impl.cc:462: const Object& obj = Object::Handle(isolate,
Api::UnwrapHandle(target));
Ditto comment about using
const Type& type_obj = Api::UnwrapTypeHandle(target);

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/mirrors_api_imp...
File runtime/vm/mirrors_api_impl.cc (right):

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/mirrors_api_imp...
runtime/vm/mirrors_api_impl.cc:25: DART_EXPORT Dart_Handle
Dart_ClassName(Dart_Handle object) {
We should change the name of this function to Dart_TypeName ?

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/mirrors_api_imp...
runtime/vm/mirrors_api_impl.cc:28: const Object& obj = Object::Handle(isolate,
Api::UnwrapHandle(object));
Ditto comment about using
const Type& type_obj = .....

[rmacnak, 2013-09-19 17:45:53]

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/dart_api_impl_t...
File runtime/vm/dart_api_impl_test.cc (right):

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/dart_api_impl_t...
runtime/vm/dart_api_impl_test.cc:214: }
On 2013/09/19 03:16:58, siva wrote:
> Why is this change in this CL, looks like it is part of the other CL you had.

Here we are using libraries instead of classes as the test for non-instance
handles to satisfy the assert that no Dart_Handle is created with a dart::Class.

The other IdentityEquals CL extends this test with a case where two objects at
different addresses are dart:core.identical().

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/mirrors_api_imp...
File runtime/vm/mirrors_api_impl.cc (right):

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/mirrors_api_imp...
runtime/vm/mirrors_api_impl.cc:25: DART_EXPORT Dart_Handle
Dart_ClassName(Dart_Handle object) {
On 2013/09/19 03:16:58, siva wrote:
> We should change the name of this function to Dart_TypeName ?

Makes sense. I have renamed this Dart_TypeName and added a Dart_ClassName that
forwards to it. (Thus begins the three-step dance to rename it without breaking
the Dartium build.)

https://codereview.chromium.org/24210003/diff/3001/runtime/vm/mirrors_api_imp...
runtime/vm/mirrors_api_impl.cc:28: const Object& obj = Object::Handle(isolate,
Api::UnwrapHandle(object));
On 2013/09/19 03:16:58, siva wrote:
> Ditto comment about using
> const Type& type_obj = .....

Got 'em.

[siva, 2013-09-19 20:15:00]

lgtm

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

https://codereview.chromium.org/24210003/diff/11001/runtime/vm/dart_api_impl....
runtime/vm/dart_api_impl.cc:1307: 
Dart_IsClass needs to be removed from dart_api.h

[rmacnak, 2013-09-19 23:53:18]

On 2013/09/19 20:15:00, siva wrote:
> lgtm
> 
>
https://codereview.chromium.org/24210003/diff/11001/runtime/vm/dart_api_impl.cc
> File runtime/vm/dart_api_impl.cc (right):
> 
>
https://codereview.chromium.org/24210003/diff/11001/runtime/vm/dart_api_impl....
> runtime/vm/dart_api_impl.cc:1307: 
> Dart_IsClass needs to be removed from dart_api.h

Done.

[rmacnak, 2013-09-20 18:30:25]

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