Rework how types work in the VM Service.

Rework how types work in the VM Service.

We now return a "type" property and an optional "_vmType" property for
each response.  We use this to "hide" vm implementation specifics in
the VM Service protocol.

For example, before we might have returned an integer reference with type
"@Smi".  Now it would have the type "@int" and the _vmType "@Smi".

This also allows us to hide funky vm internal types behind the generic
"Object" type.

Updated the protocol.md file somewhat to describe a notion of "private
properties".

Updated the Observatory and unit tests.

R=johnmccutchan@google.com

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

[turnidge, 2014-09-05 18:33:29]

turnidge@google.com changed reviewers:
+ koda@google.com

[koda, 2014-09-05 21:29:29]

Just minor comments; the general idea looks good.

https://codereview.chromium.org/547703002/diff/60001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/547703002/diff/60001/runtime/vm/object.cc#new...
runtime/vm/object.cc:6838: AddTypeProperties(&jsobj, "Function", JSONType(),
ref);
Might it not be useful to move the name string into a static method (e.g.,
Function::GetServiceTypeName returns "Function")? (We could put its
implementation just above this method, to keep things together.)

https://codereview.chromium.org/547703002/diff/60001/runtime/vm/service/proto...
File runtime/vm/service/protocol.md (right):

https://codereview.chromium.org/547703002/diff/60001/runtime/vm/service/proto...
runtime/vm/service/protocol.md:145: VM's implemention.  Private properties may
be added, removed, or
implemention -> implementation

https://codereview.chromium.org/547703002/diff/60001/runtime/vm/service/proto...
runtime/vm/service/protocol.md:151: property.  This provides the vm-internal
type name of an object, and
Be consistent w.r.t. vm/VM.

https://codereview.chromium.org/547703002/diff/60001/runtime/vm/service/proto...
runtime/vm/service/protocol.md:413: | _vmName? | String | Internal vm name. 
Provided only when different from 'name'.
VM-internal.

https://codereview.chromium.org/547703002/diff/60001/runtime/vm/service/proto...
runtime/vm/service/protocol.md:452: An [Object](#Object) is a supertype of any
response returned by the VM
An object is not a type. "Object" is a type. Maybe:

"Object is the supertype of all responses..."

[turnidge, 2014-09-08 16:20:19]

https://codereview.chromium.org/547703002/diff/60001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/547703002/diff/60001/runtime/vm/object.cc#new...
runtime/vm/object.cc:6838: AddTypeProperties(&jsobj, "Function", JSONType(),
ref);
On 2014/09/05 21:29:29, koda wrote:
> Might it not be useful to move the name string into a static method (e.g.,
> Function::GetServiceTypeName returns "Function")? (We could put its
> implementation just above this method, to keep things together.)

We will only use the function in this one location, but we'd have to add a ton
of definitions to support.  The current approach seems simpler to me.  Maybe we
should chat offline?

https://codereview.chromium.org/547703002/diff/60001/runtime/vm/service/proto...
File runtime/vm/service/protocol.md (right):

https://codereview.chromium.org/547703002/diff/60001/runtime/vm/service/proto...
runtime/vm/service/protocol.md:145: VM's implemention.  Private properties may
be added, removed, or
On 2014/09/05 21:29:29, koda wrote:
> implemention -> implementation

Done.

https://codereview.chromium.org/547703002/diff/60001/runtime/vm/service/proto...
runtime/vm/service/protocol.md:151: property.  This provides the vm-internal
type name of an object, and
On 2014/09/05 21:29:29, koda wrote:
> Be consistent w.r.t. vm/VM.

Done.

https://codereview.chromium.org/547703002/diff/60001/runtime/vm/service/proto...
runtime/vm/service/protocol.md:413: | _vmName? | String | Internal vm name. 
Provided only when different from 'name'.
On 2014/09/05 21:29:29, koda wrote:
> VM-internal.

Done.

https://codereview.chromium.org/547703002/diff/60001/runtime/vm/service/proto...
runtime/vm/service/protocol.md:452: An [Object](#Object) is a supertype of any
response returned by the VM
On 2014/09/05 21:29:29, koda wrote:
> An object is not a type. "Object" is a type. Maybe:
> 
> "Object is the supertype of all responses..."

Thanks.  I rewrote this a bunch of times and it got a little incoherent.  I used
your suggestions.

[Cutch, 2014-09-09 16:41:14]

johnmccutchan@google.com changed reviewers:
+ johnmccutchan@google.com

[Cutch, 2014-09-09 16:41:15]

lgtm with one small suggestion

https://codereview.chromium.org/547703002/diff/80001/runtime/bin/vmservice/ob...
File runtime/bin/vmservice/observatory/lib/src/elements/instance_ref.html
(right):

https://codereview.chromium.org/547703002/diff/80001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/instance_ref.html:41:
<template if="{{ ref.type == 'Instance' &&
Can we pull this logic (ref.type == 'Instance') into Dart code again and use
that from the template? Feels brittle to have a type string comparison in a
template.

[turnidge, 2014-09-09 17:02:22]

https://codereview.chromium.org/547703002/diff/80001/runtime/bin/vmservice/ob...
File runtime/bin/vmservice/observatory/lib/src/elements/instance_ref.html
(right):

https://codereview.chromium.org/547703002/diff/80001/runtime/bin/vmservice/ob...
runtime/bin/vmservice/observatory/lib/src/elements/instance_ref.html:41:
<template if="{{ ref.type == 'Instance' &&
On 2014/09/09 16:41:15, Cutch wrote:
> Can we pull this logic (ref.type == 'Instance') into Dart code again and use
> that from the template? Feels brittle to have a type string comparison in a
> template.

Added an isPlainInstance getter.

[turnidge, 2014-09-09 17:26:11]

Committed patchset #7 (id:120001) manually as 40044 (presubmit successful).