Add the --steal-breakpoints flag.

Add the --steal-breakpoints flag.

Add a test.

Update debugging_test.dart to not use line number instead of token pos to make it modestly less fragile.

BUG=
R=rmacnak@google.com

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

[turnidge, 2015-04-30 20:38:02]

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

[rmacnak, 2015-04-30 21:48:55]

lgtm w/c

https://chromiumcodereview.appspot.com/1113243002/diff/20001/runtime/observat...
File runtime/observatory/tests/service/debugging_test.dart (right):

https://chromiumcodereview.appspot.com/1113243002/diff/20001/runtime/observat...
runtime/observatory/tests/service/debugging_test.dart:86: // We are at the
breakpoint on line 14.
15

https://chromiumcodereview.appspot.com/1113243002/diff/20001/runtime/observat...
runtime/observatory/tests/service/debugging_test.dart:110: await
isolate.stepInto();
stepOver()? This assumes Integer.+ is unsteppable.

https://chromiumcodereview.appspot.com/1113243002/diff/20001/runtime/vm/debug...
File runtime/vm/debugger.cc (right):

https://chromiumcodereview.appspot.com/1113243002/diff/20001/runtime/vm/debug...
runtime/vm/debugger.cc:39: "Intercept breakpoints and other pause events before
they "
Should probably indicate they go to the vm service/observatory instead.

[hausner, 2015-04-30 23:17:30]

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

[hausner, 2015-04-30 23:17:31]

DBC

https://codereview.chromium.org/1113243002/diff/20001/runtime/observatory/tes...
File runtime/observatory/tests/service/debugging_test.dart (right):

https://codereview.chromium.org/1113243002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/debugging_test.dart:29: (Isolate isolate)
async {
DBC: I'm not familiar with the harness and what it expects the functions in the
tests array to do. But adding the async modifier to the function makes it return
a future immediately. That future then completes with the return value of this
function. The return value is a future, too. Is that really what is intended
here?

[hausner, 2015-04-30 23:41:48]

More DBC.

https://codereview.chromium.org/1113243002/diff/20001/runtime/observatory/tes...
File runtime/observatory/tests/service/debugging_test.dart (right):

https://codereview.chromium.org/1113243002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/debugging_test.dart:29: (Isolate isolate)
async {
On 2015/04/30 23:17:31, hausner wrote:
> DBC: I'm not familiar with the harness and what it expects the functions in
the
> tests array to do. But adding the async modifier to the function makes it
return
> a future immediately. That future then completes with the return value of this
> function. The return value is a future, too. Is that really what is intended
> here?

Commenting on my own comment :)

After looking at runIsolateTests() to see what it does with the tests array, I
think marking the function literals as async is wrong. The harness iterates over
the array with Future.forEach(), which invokes each function when the previous
one has completed the future it returns.

Marking the function literal as async now means that the future it returns
completes with the "return completer.future" statement, not with the
"completer.complete()" call in the event handler. That is probably not what you
want.

[turnidge, 2015-05-01 16:54:16]

Matthias, can you take another look?

https://codereview.chromium.org/1113243002/diff/20001/runtime/observatory/tes...
File runtime/observatory/tests/service/debugging_test.dart (right):

https://codereview.chromium.org/1113243002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/debugging_test.dart:29: (Isolate isolate)
async {
On 2015/04/30 23:41:48, hausner wrote:
> On 2015/04/30 23:17:31, hausner wrote:
> > DBC: I'm not familiar with the harness and what it expects the functions in
> the
> > tests array to do. But adding the async modifier to the function makes it
> return
> > a future immediately. That future then completes with the return value of
this
> > function. The return value is a future, too. Is that really what is intended
> > here?
> 
> Commenting on my own comment :)
> 
> After looking at runIsolateTests() to see what it does with the tests array, I
> think marking the function literals as async is wrong. The harness iterates
over
> the array with Future.forEach(), which invokes each function when the previous
> one has completed the future it returns.
> 
> Marking the function literal as async now means that the future it returns
> completes with the "return completer.future" statement, not with the
> "completer.complete()" call in the event handler. That is probably not what
you
> want.

Thanks.  The old code was vastly wrong.  Edited the test as discussed with you
offline.

https://codereview.chromium.org/1113243002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/debugging_test.dart:86: // We are at the
breakpoint on line 14.
On 2015/04/30 21:48:55, rmacnak wrote:
> 15

Done.

https://codereview.chromium.org/1113243002/diff/20001/runtime/observatory/tes...
runtime/observatory/tests/service/debugging_test.dart:110: await
isolate.stepInto();
On 2015/04/30 21:48:55, rmacnak wrote:
> stepOver()? This assumes Integer.+ is unsteppable.

Done.

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

https://codereview.chromium.org/1113243002/diff/20001/runtime/vm/debugger.cc#...
runtime/vm/debugger.cc:39: "Intercept breakpoints and other pause events before
they "
On 2015/04/30 21:48:55, rmacnak wrote:
> Should probably indicate they go to the vm service/observatory instead.

Done.

[hausner, 2015-05-01 17:02:07]

Looks fine to me.

[turnidge, 2015-05-01 17:56:49]

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