vm-service: Add optional 'scope' parameter to 'evaluate' and 'evaluateInFrame'.

vm-service: Add optional 'scope' parameter to 'evaluate' and 'evaluateInFrame'.

Closes #29535

R=asiva@google.com

Committed: https://github.com/dart-lang/sdk/commit/03a2c24edf17a0b3aec930d88e3dcaaa6ccf8add

[rmacnak, 2017-05-08 22:19:24]

Description was changed from

==========
vm-service: Add optional 'scope' parameter to 'evaluate' and 'evaluateInFrame'.
==========

to

==========
vm-service: Add optional 'scope' parameter to 'evaluate' and 'evaluateInFrame'.

Closes #29535
==========

[rmacnak, 2017-05-08 22:31:24]

rmacnak@google.com changed reviewers:
+ asiva@google.com, gbracha@google.com

[rmacnak, 2017-05-09 21:33:51]

Description was changed from

==========
vm-service: Add optional 'scope' parameter to 'evaluate' and 'evaluateInFrame'.

Closes #29535
==========

to

==========
vm-service: Add optional 'scope' parameter to 'evaluate' and 'evaluateInFrame'.

Closes #29535
==========

[siva, 2017-05-18 20:56:49]

lgtm with comments.

https://codereview.chromium.org/2872503004/diff/20001/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

https://codereview.chromium.org/2872503004/diff/20001/runtime/vm/debugger.cc#...
runtime/vm/debugger.cc:1313: }
if there are conflicts what happens do we get an exception?

We should add a test case with conflicting parameter names.

https://codereview.chromium.org/2872503004/diff/20001/runtime/vm/service.cc
File runtime/vm/service.cc (right):

https://codereview.chromium.org/2872503004/diff/20001/runtime/vm/service.cc#n...
runtime/vm/service.cc:2231: return c <= ' ';
should this also include tab?

https://codereview.chromium.org/2872503004/diff/20001/runtime/vm/service.cc#n...
runtime/vm/service.cc:2259: str = String::FromUTF8(reinterpret_cast<const
uint8_t*>(name), c - name);
Is this really a UTF8 array or can you just use OneByteString::New(...) here or
maybe String::FromLatin1(...)

https://codereview.chromium.org/2872503004/diff/20001/runtime/vm/service.cc#n...
runtime/vm/service.cc:2277: str = String::FromUTF8(reinterpret_cast<const
uint8_t*>(id), c - id);
Ditto comment here.

Also why do we need to create a GrowableObjectArray for the values here, convert
the C string to a Dart string and again convert it back to a C string when doing
the lookup? Why not just store a GrowableArray<const char*> here and create the
GrowableObjectArray in BuildScope after the lookup is done.

[rmacnak, 2017-05-19 00:56:44]

https://codereview.chromium.org/2872503004/diff/20001/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

https://codereview.chromium.org/2872503004/diff/20001/runtime/vm/debugger.cc#...
runtime/vm/debugger.cc:1313: }
On 2017/05/18 20:56:48, siva wrote:
> if there are conflicts what happens do we get an exception?
> 
> We should add a test case with conflicting parameter names.

This code says if there is a local 'x' and value in the user-provided scope 'x',
the 'x' from the user provided scope wins. This case is included in
evaluate_in_frame_with_scope_test.dart. Added comment here.

https://codereview.chromium.org/2872503004/diff/20001/runtime/vm/service.cc
File runtime/vm/service.cc (right):

https://codereview.chromium.org/2872503004/diff/20001/runtime/vm/service.cc#n...
runtime/vm/service.cc:2231: return c <= ' ';
On 2017/05/18 20:56:48, siva wrote:
> should this also include tab?

This includes tab.

https://codereview.chromium.org/2872503004/diff/20001/runtime/vm/service.cc#n...
runtime/vm/service.cc:2259: str = String::FromUTF8(reinterpret_cast<const
uint8_t*>(name), c - name);
On 2017/05/18 20:56:48, siva wrote:
> Is this really a UTF8 array or can you just use OneByteString::New(...) here
or
> maybe String::FromLatin1(...)

The input came from a String::ToCString(). But switched to keeping C strings as
suggested below.

[rmacnak, 2017-05-19 01:02:53]

Description was changed from

==========
vm-service: Add optional 'scope' parameter to 'evaluate' and 'evaluateInFrame'.

Closes #29535
==========

to

==========
vm-service: Add optional 'scope' parameter to 'evaluate' and 'evaluateInFrame'.

Closes #29535

R=asiva@google.com

Committed:
https://github.com/dart-lang/sdk/commit/03a2c24edf17a0b3aec930d88e3dcaaa6ccf8add
==========

[rmacnak, 2017-05-19 01:02:54]

Committed patchset #3 (id:40001) manually as
03a2c24edf17a0b3aec930d88e3dcaaa6ccf8add (presubmit successful).