Use latent breakpoints list when looking up or removing breakpoints.

Use latent breakpoints list when looking up or removing breakpoints.

BUG=https://github.com/dart-lang/sdk/issues/29566
R=asiva@google.com

Committed: https://github.com/dart-lang/sdk/commit/5f9ec0abe8e9d5d268d5a6710da00018e0ccb193

[aam, 2017-05-07 03:20:41]

aam@google.com changed reviewers:
+ asiva@google.com

[aam, 2017-05-07 03:20:41]

Hi Siva,

  please take a look at the fix. It seems to do the job for the scenario
described in https://github.com/dart-lang/sdk/issues/29566.

Thank you.

[siva, 2017-05-08 18:28:19]

LGTM

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

https://codereview.chromium.org/2869573002/diff/1/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:4257: UnlinkCodeBreakpoints(curr_loc);
Not sure if this is an issue, latent breakpoints will not have any code
breakpoints to unlink, we would be doing a iteration through code breakpoints
and not find any. Maybe it is ok and not much of a performance issue.

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

https://codereview.chromium.org/2869573002/diff/1/runtime/vm/debugger.h#newco...
runtime/vm/debugger.h:494: Breakpoint* GetBreakpointByIdInTheList(intptr_t id,
BreakpointLocation* list);
Should these list variants be private methods as we don't expect them to be
called directly?

https://codereview.chromium.org/2869573002/diff/1/runtime/vm/debugger.h#newco...
runtime/vm/debugger.h:604: JSONArray* jsarr) const;
Ditto comment about making this a private method.

[aam, 2017-05-08 20:17:47]

PTAL

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

https://codereview.chromium.org/2869573002/diff/1/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:4257: UnlinkCodeBreakpoints(curr_loc);
On 2017/05/08 18:28:18, siva wrote:
> Not sure if this is an issue, latent breakpoints will not have any code
> breakpoints to unlink, we would be doing a iteration through code breakpoints
> and not find any. Maybe it is ok and not much of a performance issue.

Okay, makes sense. Added a check here to UnlinkCodeBreakpoints for non-latent
locations only.

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

https://codereview.chromium.org/2869573002/diff/1/runtime/vm/debugger.h#newco...
runtime/vm/debugger.h:494: Breakpoint* GetBreakpointByIdInTheList(intptr_t id,
BreakpointLocation* list);
On 2017/05/08 18:28:18, siva wrote:
> Should these list variants be private methods as we don't expect them to be
> called directly?

Done.

https://codereview.chromium.org/2869573002/diff/1/runtime/vm/debugger.h#newco...
runtime/vm/debugger.h:604: JSONArray* jsarr) const;
On 2017/05/08 18:28:18, siva wrote:
> Ditto comment about making this a private method.

Done.

[siva, 2017-05-08 22:55:19]

lgtm

[aam, 2017-05-09 12:58:42]

Description was changed from

==========
Use latent breakpoints list when looking up or removing breakpoints.

BUG=https://github.com/dart-lang/sdk/issues/29566
==========

to

==========
Use latent breakpoints list when looking up or removing breakpoints.

BUG=https://github.com/dart-lang/sdk/issues/29566
R=asiva@google.com

Committed:
https://github.com/dart-lang/sdk/commit/5f9ec0abe8e9d5d268d5a6710da00018e0ccb193
==========

[aam, 2017-05-09 12:58:43]

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