Allow setting break-on-exceptions option over the service protocol.

Allow setting break-on-exceptions option over the service protocol. Add command to Observatory's debugger.

Decide whether to cache service objects based on the fixedId property.

Be more tolerant of eval scripts and functions.

BUG=
R=johnmccutchan@google.com

Committed: https://github.com/dart-lang/sdk/commit/392a83221ffd8405f671c9669a64bd950d6faf5e

[rmacnak, 2015-06-11 17:31:16]

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

[rmacnak, 2015-06-11 17:31:17]

https://codereview.chromium.org/1174313002/diff/100001/runtime/observatory/li...
File runtime/observatory/lib/src/elements/debugger.dart (right):

https://codereview.chromium.org/1174313002/diff/100001/runtime/observatory/li...
runtime/observatory/lib/src/elements/debugger.dart:371: : super(debugger,
'break-on-exceptions', []);
Don't really want to introduce the first hyphenated command...

https://codereview.chromium.org/1174313002/diff/100001/runtime/observatory/li...
File runtime/observatory/lib/src/service/object.dart (right):

https://codereview.chromium.org/1174313002/diff/100001/runtime/observatory/li...
runtime/observatory/lib/src/service/object.dart:2356: bool get canCache =>
!_id.startsWith(ServiceMap.objectIdRingPrefix);
This still doesn't pass the assert on 309.

[Cutch, 2015-06-11 21:20:12]

https://codereview.chromium.org/1174313002/diff/100001/runtime/observatory/li...
File runtime/observatory/lib/src/elements/debugger.dart (right):

https://codereview.chromium.org/1174313002/diff/100001/runtime/observatory/li...
runtime/observatory/lib/src/elements/debugger.dart:371: : super(debugger,
'break-on-exceptions', []);
On 2015/06/11 17:31:17, rmacnak wrote:
> Don't really want to introduce the first hyphenated command...

What about a 'set' command for configuring behaviour?

https://codereview.chromium.org/1174313002/diff/100001/runtime/observatory/li...
File runtime/observatory/lib/src/service/object.dart (right):

https://codereview.chromium.org/1174313002/diff/100001/runtime/observatory/li...
runtime/observatory/lib/src/service/object.dart:2356: bool get canCache =>
!_id.startsWith(ServiceMap.objectIdRingPrefix);
On 2015/06/11 17:31:17, rmacnak wrote:
> This still doesn't pass the assert on 309.

example of the assert on 309 failing?

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

https://codereview.chromium.org/1174313002/diff/100001/runtime/vm/service.cc#...
runtime/vm/service.cc:2676: isolate->debugger()->SetExceptionPauseInfo(info);
This state change affects all debuggers. This should trigger an event so other
debuggers connected can notify the user that break on exceptions is now
enabled/disabled/whatever.

[rmacnak, 2015-06-12 00:09:39]

https://codereview.chromium.org/1174313002/diff/100001/runtime/observatory/li...
File runtime/observatory/lib/src/elements/debugger.dart (right):

https://codereview.chromium.org/1174313002/diff/100001/runtime/observatory/li...
runtime/observatory/lib/src/elements/debugger.dart:371: : super(debugger,
'break-on-exceptions', []);
On 2015/06/11 21:20:12, Cutch wrote:
> On 2015/06/11 17:31:17, rmacnak wrote:
> > Don't really want to introduce the first hyphenated command...
> 
> What about a 'set' command for configuring behaviour?

Somewhat better.

https://codereview.chromium.org/1174313002/diff/100001/runtime/observatory/li...
File runtime/observatory/lib/src/service/object.dart (right):

https://codereview.chromium.org/1174313002/diff/100001/runtime/observatory/li...
runtime/observatory/lib/src/service/object.dart:2356: bool get canCache =>
!_id.startsWith(ServiceMap.objectIdRingPrefix);
On 2015/06/11 21:20:12, Cutch wrote:
> On 2015/06/11 17:31:17, rmacnak wrote:
> > This still doesn't pass the assert on 309.
> 
> example of the assert on 309 failing?

Nvm, I needed to do this for function as well (eval function).

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

https://codereview.chromium.org/1174313002/diff/100001/runtime/vm/service.cc#...
runtime/vm/service.cc:2676: isolate->debugger()->SetExceptionPauseInfo(info);
On 2015/06/11 21:20:12, Cutch wrote:
> This state change affects all debuggers. This should trigger an event so other
> debuggers connected can notify the user that break on exceptions is now
> enabled/disabled/whatever.

Rejiggered as a DebuggerChanged event.

[Cutch, 2015-06-12 13:10:47]

https://codereview.chromium.org/1174313002/diff/160001/runtime/observatory/li...
File runtime/observatory/lib/src/service/object.dart (right):

https://codereview.chromium.org/1174313002/diff/160001/runtime/observatory/li...
runtime/observatory/lib/src/service/object.dart:2362: bool get canCache =>
!_id.startsWith(ServiceMap.objectIdRingPrefix);
A better way to fix handle this:

Whenever an "id" is fixed (and can be cached) there is a "fixedId" bit sent:

AddProperty("fixedId", true);

I never hooked it up on the Dart side. The correct fix is use this field in the
service library

_canCache = map['fixedId'] == true;
bool get canCache => _canCache;

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

https://codereview.chromium.org/1174313002/diff/160001/runtime/vm/service.cc#...
runtime/vm/service.cc:2794: { "_getDebuggerUpdate", GetDebuggerUpdate,
What about including the debugger settings in stackTrace and getting rid of this
RPC?

https://codereview.chromium.org/1174313002/diff/160001/runtime/vm/service_eve...
File runtime/vm/service_event.cc (right):

https://codereview.chromium.org/1174313002/diff/160001/runtime/vm/service_eve...
runtime/vm/service_event.cc:87: case kDebuggerUpdate:
kDebuggerSettingsUpdate ?

[rmacnak, 2015-06-15 17:53:52]

https://codereview.chromium.org/1174313002/diff/160001/runtime/observatory/li...
File runtime/observatory/lib/src/service/object.dart (right):

https://codereview.chromium.org/1174313002/diff/160001/runtime/observatory/li...
runtime/observatory/lib/src/service/object.dart:2362: bool get canCache =>
!_id.startsWith(ServiceMap.objectIdRingPrefix);
On 2015/06/12 13:10:47, Cutch wrote:
> A better way to fix handle this:
> 
> Whenever an "id" is fixed (and can be cached) there is a "fixedId" bit sent:
> 
> AddProperty("fixedId", true);
> 
> I never hooked it up on the Dart side. The correct fix is use this field in
the
> service library
> 
> _canCache = map['fixedId'] == true;
> bool get canCache => _canCache;

Done.

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

https://codereview.chromium.org/1174313002/diff/160001/runtime/vm/service.cc#...
runtime/vm/service.cc:2794: { "_getDebuggerUpdate", GetDebuggerUpdate,
On 2015/06/12 13:10:47, Cutch wrote:
> What about including the debugger settings in stackTrace and getting rid of
this
> RPC?

Seems okay. A debugger will generally fetch the stack to get started anyway.

https://codereview.chromium.org/1174313002/diff/160001/runtime/vm/service_eve...
File runtime/vm/service_event.cc (right):

https://codereview.chromium.org/1174313002/diff/160001/runtime/vm/service_eve...
runtime/vm/service_event.cc:87: case kDebuggerUpdate:
On 2015/06/12 13:10:47, Cutch wrote:
> kDebuggerSettingsUpdate ?

Done.

[Cutch, 2015-06-15 18:01:10]

LGTM after moving the exception setting from the stack trace to the isolate.

[rmacnak, 2015-06-15 19:05:29]

Committed patchset #11 (id:200001) manually as
392a83221ffd8405f671c9669a64bd950d6faf5e (presubmit successful).