Debugger wire protocol: setting breakpoints

Debugger wire protocol: setting breakpoints

- Adding commands to set breakpoints and showing current stack trace.
- Adding a callback hook to signal when a breakpoint gets resolved to
  and actual line number. 
- Adding ids for breakpoints and new breakpoint API functions, deprecating
  the existing ones. 
- Adding one helper function to the JSON class

Anton may want to take a look at the JSON addition.
Committed: https://code.google.com/p/dart/source/detail?r=7628

[srdjan, 2012-05-14 20:48:16]

LGTM with comments and questions

http://codereview.chromium.org/10383164/diff/1/runtime/bin/dbg_connection.cc
File runtime/bin/dbg_connection.cc (right):

http://codereview.chromium.org/10383164/diff/1/runtime/bin/dbg_connection.cc#...
runtime/bin/dbg_connection.cc:186: queued_messages_.Clear();
Is using a static queued_messages_  multi-isolate safe or do you need to put it
into individual isolates?

http://codereview.chromium.org/10383164/diff/1/runtime/vm/debugger.h
File runtime/vm/debugger.h (right):

http://codereview.chromium.org/10383164/diff/1/runtime/vm/debugger.h#newcode32
runtime/vm/debugger.h:32: intptr_t LineNumber();
Can the three functions above be const?

http://codereview.chromium.org/10383164/diff/1/runtime/vm/debugger.h#newcode45
runtime/vm/debugger.h:45: intptr_t id_;
Can it be const? Same for token_index, line_number?

http://codereview.chromium.org/10383164/diff/1/tools/ddbg.dart
File tools/ddbg.dart (right):

http://codereview.chromium.org/10383164/diff/1/tools/ddbg.dart#newcode17
tools/ddbg.dart:17: bool verbose = true;
Do you really want it to be true?

[hausner, 2012-05-14 21:07:27]

Thanks for the quick review!

http://codereview.chromium.org/10383164/diff/1/runtime/bin/dbg_connection.cc
File runtime/bin/dbg_connection.cc (right):

http://codereview.chromium.org/10383164/diff/1/runtime/bin/dbg_connection.cc#...
runtime/bin/dbg_connection.cc:186: queued_messages_.Clear();
On 2012/05/14 20:48:16, srdjan wrote:
> Is using a static queued_messages_  multi-isolate safe or do you need to put
it
> into individual isolates?
Good point. Generally static variables are not ok, but here I think we're fine
because we only ever have one instance of the wire protocol handler. Only one
isolate at a time will be able to talk to the debugger. If that assumption
should change in the future, then we need a per-isolate copy of the protocol
handler.

http://codereview.chromium.org/10383164/diff/1/runtime/vm/debugger.h
File runtime/vm/debugger.h (right):

http://codereview.chromium.org/10383164/diff/1/runtime/vm/debugger.h#newcode32
runtime/vm/debugger.h:32: intptr_t LineNumber();
On 2012/05/14 20:48:16, srdjan wrote:
> Can the three functions above be const?
No, because they evaluate the result lazily. For example, getting the line
number of a breakpoint causes the script to be re-tokenized. I don't want to do
that eagerly.

http://codereview.chromium.org/10383164/diff/1/runtime/vm/debugger.h#newcode45
runtime/vm/debugger.h:45: intptr_t id_;
On 2012/05/14 20:48:16, srdjan wrote:
> Can it be const? Same for token_index, line_number?
id_ and token_index_ can be const, line_number_ can't, for the reason mentioned
above.

Done.

http://codereview.chromium.org/10383164/diff/1/tools/ddbg.dart
File tools/ddbg.dart (right):

http://codereview.chromium.org/10383164/diff/1/tools/ddbg.dart#newcode17
tools/ddbg.dart:17: bool verbose = true;
On 2012/05/14 20:48:16, srdjan wrote:
> Do you really want it to be true?
Changed to false.

[Anton Muhin, 2012-05-15 15:17:56]

http://codereview.chromium.org/10383164/diff/9001/lib/json/json.dart
File lib/json/json.dart (right):

http://codereview.chromium.org/10383164/diff/9001/lib/json/json.dart#newcode111
lib/json/json.dart:111: p._parseObject();
I am not sure it's valid implementation: _parseObject will unconditionally skip
first char as it's used under dispatch in _parseValue, so it will probably
accept something like [ 'foo': 1 }

[hausner, 2012-05-15 15:32:56]

http://codereview.chromium.org/10383164/diff/9001/lib/json/json.dart
File lib/json/json.dart (right):

http://codereview.chromium.org/10383164/diff/9001/lib/json/json.dart#newcode111
lib/json/json.dart:111: p._parseObject();
Nice catch! Thank you for taking a look. I'll fix it in a separate checkin.

On 2012/05/15 15:17:56, antonmuhin wrote:
> I am not sure it's valid implementation: _parseObject will unconditionally
skip
> first char as it's used under dispatch in _parseValue, so it will probably
> accept something like [ 'foo': 1 }