Make VM service id handling JSON-RPC 2 compliant.

Make VM service id handling JSON-RPC 2 compliant.

* Numeric ids will not be converted to strings.

* String ids will be escaped, so an id of '"' doesn't produce invalid
  JSON.

* List or map ids will be rejected.

* Null ids will produce no response.

R=johnmccutchan@google.com

Committed: https://github.com/dart-lang/sdk/commit/4153313dfd9293c49213798370bae3f75cd0ce0a

[nweiz, 2015-07-24 22:20:17]

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

[Cutch, 2015-07-27 14:00:51]

https://codereview.chromium.org/1255003003/diff/1/runtime/vm/json_stream.h
File runtime/vm/json_stream.h (right):

https://codereview.chromium.org/1255003003/diff/1/runtime/vm/json_stream.h#ne...
runtime/vm/json_stream.h:174: RawInstance* seq_;
You need to store seq_ as an Instance&.

[nweiz, 2015-07-27 18:16:02]

https://codereview.chromium.org/1255003003/diff/1/runtime/vm/json_stream.h
File runtime/vm/json_stream.h (right):

https://codereview.chromium.org/1255003003/diff/1/runtime/vm/json_stream.h#ne...
runtime/vm/json_stream.h:174: RawInstance* seq_;
On 2015/07/27 14:00:51, Cutch wrote:
> You need to store seq_ as an Instance&.

I'm pretty out of my depth here. If I do this, it seems like assigning to seq_
in Setup fails because Instance::operator= is private.

[Cutch, 2015-07-27 18:28:39]

On 2015/07/27 18:16:02, nweiz wrote:
> https://codereview.chromium.org/1255003003/diff/1/runtime/vm/json_stream.h
> File runtime/vm/json_stream.h (right):
> 
>
https://codereview.chromium.org/1255003003/diff/1/runtime/vm/json_stream.h#ne...
> runtime/vm/json_stream.h:174: RawInstance* seq_;
> On 2015/07/27 14:00:51, Cutch wrote:
> > You need to store seq_ as an Instance&.
> 
> I'm pretty out of my depth here. If I do this, it seems like assigning to seq_
> in Setup fails because Instance::operator= is private.

in json_stream.h:

Instance& seq_;

in json_stream.cc:

initialized list for construction:

seq_(Instance::Handle()),

assignment:

seq_ ^= seq;

[nweiz, 2015-07-27 21:07:06]

Code review changes

[Cutch, 2015-07-27 21:10:10]

One last handle usage change and then LGTM.

https://codereview.chromium.org/1255003003/diff/40001/runtime/vm/json_stream.cc
File runtime/vm/json_stream.cc (right):

https://codereview.chromium.org/1255003003/diff/40001/runtime/vm/json_stream....
runtime/vm/json_stream.cc:166: Instance& id = seq();
just use seq_ directly here OR:

Instance& id = Instance::Handle(seq());

https://codereview.chromium.org/1255003003/diff/40001/runtime/vm/json_stream.h
File runtime/vm/json_stream.h (right):

https://codereview.chromium.org/1255003003/diff/40001/runtime/vm/json_stream....
runtime/vm/json_stream.h:103: Instance& seq() const { return seq_; }
RawInstance* seq() const { return seq_.raw(); }

[nweiz, 2015-07-27 21:17:26]

Code review changes

[nweiz, 2015-07-27 21:17:35]

https://codereview.chromium.org/1255003003/diff/40001/runtime/vm/json_stream.cc
File runtime/vm/json_stream.cc (right):

https://codereview.chromium.org/1255003003/diff/40001/runtime/vm/json_stream....
runtime/vm/json_stream.cc:166: Instance& id = seq();
On 2015/07/27 21:10:10, Cutch wrote:
> just use seq_ directly here OR:
> 
> Instance& id = Instance::Handle(seq());

Done.

https://codereview.chromium.org/1255003003/diff/40001/runtime/vm/json_stream.h
File runtime/vm/json_stream.h (right):

https://codereview.chromium.org/1255003003/diff/40001/runtime/vm/json_stream....
runtime/vm/json_stream.h:103: Instance& seq() const { return seq_; }
On 2015/07/27 21:10:10, Cutch wrote:
> RawInstance* seq() const { return seq_.raw(); }

This causes an "invalid use of incomplete type ‘class dart::Instance’" error. Do
we need to have this method? I don't think any external classes use it.

[Cutch, 2015-07-27 22:15:30]

On 2015/07/27 21:17:35, nweiz wrote:
>
https://codereview.chromium.org/1255003003/diff/40001/runtime/vm/json_stream.cc
> File runtime/vm/json_stream.cc (right):
> 
>
https://codereview.chromium.org/1255003003/diff/40001/runtime/vm/json_stream....
> runtime/vm/json_stream.cc:166: Instance& id = seq();
> On 2015/07/27 21:10:10, Cutch wrote:
> > just use seq_ directly here OR:
> > 
> > Instance& id = Instance::Handle(seq());
> 
> Done.
> 
> https://codereview.chromium.org/1255003003/diff/40001/runtime/vm/json_stream.h
> File runtime/vm/json_stream.h (right):
> 
>
https://codereview.chromium.org/1255003003/diff/40001/runtime/vm/json_stream....
> runtime/vm/json_stream.h:103: Instance& seq() const { return seq_; }
> On 2015/07/27 21:10:10, Cutch wrote:
> > RawInstance* seq() const { return seq_.raw(); }
> 
> This causes an "invalid use of incomplete type ‘class dart::Instance’" error.
Do
> we need to have this method? I don't think any external classes use it.

Either move this into the .cc file:

RawInstance* JSONStream::seq() const {
  return seq_.raw();
}

Or drop it entirely because I don't think it is used anymore.

[nweiz, 2015-07-27 23:14:37]

Committed patchset #5 (id:80001) manually as
4153313dfd9293c49213798370bae3f75cd0ce0a (presubmit successful).