[inspector] added targetCallFrames for continueToLocation

[inspector] added targetCallFrames for continueToLocation

By default we just break when we first time reach passed location, with current - we'll break at passed location only when it happens within the same stack frame.

BUG=v8:6397
R=dgozman@chromium.org

Review-Url: https://codereview.chromium.org/2879923003
Cr-Commit-Position: refs/heads/master@{#45354}
Committed: https://chromium.googlesource.com/v8/v8/+/d7e09f8fccbd11e10cd149a8ed6c21c50b6cedb2

[kozy, 2017-05-13 02:12:09]

Dmitry, please take a look.

[dgozman, 2017-05-15 17:01:27]

Please add a bug.

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-debugger-a...
File src/inspector/v8-debugger-agent-impl.h (right):

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-debugger-a...
src/inspector/v8-debugger-agent-impl.h:143: bool
shouldIgnoreContinueToLocation(const String16& breakpointId);
This should reside in V8Debugger. It's very similar to stepping and should be
implemented in a same way.

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-debugger.cc
File src/inspector/v8-debugger.cc (right):

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-debugger.c...
src/inspector/v8-debugger.cc:569: if (breakpointIds.size() == 1) {
Should we mark "continue to location" breakpoint somehow to avoid doing this all
the time?

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-stack-trac...
File src/inspector/v8-stack-trace-impl.cc (right):

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-stack-trac...
src/inspector/v8-stack-trace-impl.cc:131: if (scriptId() != frame->scriptId())
return false;
We always have scriptId, right? Then there is no need to compare functionName or
sourceURL.

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-stack-trac...
src/inspector/v8-stack-trace-impl.cc:273:
m_currentEnd(stackTrace->m_frames.end()),
If top stack is empty, iterator will be immediately done().

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-stack-trac...
src/inspector/v8-stack-trace-impl.cc:295: return m_currentIt != m_currentEnd ?
m_currentIt->get() : nullptr;
Just m_currentIt->get(), since nobody should access exhausted iterator.

[kozy, 2017-05-16 01:42:03]

Description was changed from

==========
[inspector] added strategy for continueToLocation

By default we just break when we first time reach passed location, with
inCurrentFrame position we'll break at passed location only when it happens
within same stack frame.

BUG=none
R=dgozman@chromium.org
==========

to

==========
[inspector] added strategy for continueToLocation

By default we just break when we first time reach passed location, with
inCurrentFrame position we'll break at passed location only when it happens
within same stack frame.

BUG=v8:6397
R=dgozman@chromium.org
==========

[kozy, 2017-05-16 01:43:48]

all done, please take another look.

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-debugger-a...
File src/inspector/v8-debugger-agent-impl.h (right):

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-debugger-a...
src/inspector/v8-debugger-agent-impl.h:143: bool
shouldIgnoreContinueToLocation(const String16& breakpointId);
On 2017/05/15 17:01:26, dgozman wrote:
> This should reside in V8Debugger. It's very similar to stepping and should be
> implemented in a same way.

Done in separate CL.

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-debugger.cc
File src/inspector/v8-debugger.cc (right):

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-debugger.c...
src/inspector/v8-debugger.cc:569: if (breakpointIds.size() == 1) {
On 2017/05/15 17:01:26, dgozman wrote:
> Should we mark "continue to location" breakpoint somehow to avoid doing this
all
> the time?

Done.

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-stack-trac...
File src/inspector/v8-stack-trace-impl.cc (right):

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-stack-trac...
src/inspector/v8-stack-trace-impl.cc:131: if (scriptId() != frame->scriptId())
return false;
On 2017/05/15 17:01:27, dgozman wrote:
> We always have scriptId, right? Then there is no need to compare functionName
or
> sourceURL.

Done.

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-stack-trac...
src/inspector/v8-stack-trace-impl.cc:273:
m_currentEnd(stackTrace->m_frames.end()),
On 2017/05/15 17:01:27, dgozman wrote:
> If top stack is empty, iterator will be immediately done().

currently it could not happen, removed DCHECK.

https://codereview.chromium.org/2879923003/diff/1/src/inspector/v8-stack-trac...
src/inspector/v8-stack-trace-impl.cc:295: return m_currentIt != m_currentEnd ?
m_currentIt->get() : nullptr;
On 2017/05/15 17:01:27, dgozman wrote:
> Just m_currentIt->get(), since nobody should access exhausted iterator.

Done.

[dgozman, 2017-05-16 20:19:25]

lgtm

https://codereview.chromium.org/2879923003/diff/40001/src/inspector/js_protoc...
File src/inspector/js_protocol.json (right):

https://codereview.chromium.org/2879923003/diff/40001/src/inspector/js_protoc...
src/inspector/js_protocol.json:561: { "name": "strategy", "type": "string",
"enum": ["default", "inCurrentFrame"], "optional": true, "experimental": true }
"targetCallFrame": "any", "current", "deeper"

https://codereview.chromium.org/2879923003/diff/40001/src/inspector/v8-debugg...
File src/inspector/v8-debugger.cc (right):

https://codereview.chromium.org/2879923003/diff/40001/src/inspector/v8-debugg...
src/inspector/v8-debugger.cc:435: if (!m_continueToLocationStack) {
DCHECK()

https://codereview.chromium.org/2879923003/diff/40001/src/inspector/v8-debugg...
src/inspector/v8-debugger.cc:447: bool
V8Debugger::shouldIgnoreContinueToLocation() {
shouldContinueToCurrentLocation

https://codereview.chromium.org/2879923003/diff/40001/src/inspector/v8-stack-...
File src/inspector/v8-stack-trace-impl.cc (right):

https://codereview.chromium.org/2879923003/diff/40001/src/inspector/v8-stack-...
src/inspector/v8-stack-trace-impl.cc:130: if (scriptId() != frame->scriptId())
return false;
return m_scriptId == frame->m_scriptId && ....;

[kozy, 2017-05-16 21:18:48]

thanks, all done.

https://codereview.chromium.org/2879923003/diff/40001/src/inspector/js_protoc...
File src/inspector/js_protocol.json (right):

https://codereview.chromium.org/2879923003/diff/40001/src/inspector/js_protoc...
src/inspector/js_protocol.json:561: { "name": "strategy", "type": "string",
"enum": ["default", "inCurrentFrame"], "optional": true, "experimental": true }
On 2017/05/16 20:19:24, dgozman wrote:
> "targetCallFrame": "any", "current", "deeper"

Done.

https://codereview.chromium.org/2879923003/diff/40001/src/inspector/v8-debugg...
File src/inspector/v8-debugger.cc (right):

https://codereview.chromium.org/2879923003/diff/40001/src/inspector/v8-debugg...
src/inspector/v8-debugger.cc:435: if (!m_continueToLocationStack) {
On 2017/05/16 20:19:24, dgozman wrote:
> DCHECK()

Done.

https://codereview.chromium.org/2879923003/diff/40001/src/inspector/v8-debugg...
src/inspector/v8-debugger.cc:447: bool
V8Debugger::shouldIgnoreContinueToLocation() {
On 2017/05/16 20:19:25, dgozman wrote:
> shouldContinueToCurrentLocation

Done.

https://codereview.chromium.org/2879923003/diff/40001/src/inspector/v8-stack-...
File src/inspector/v8-stack-trace-impl.cc (right):

https://codereview.chromium.org/2879923003/diff/40001/src/inspector/v8-stack-...
src/inspector/v8-stack-trace-impl.cc:130: if (scriptId() != frame->scriptId())
return false;
On 2017/05/16 20:19:25, dgozman wrote:
> return m_scriptId == frame->m_scriptId && ....;

Done.

[kozy, 2017-05-16 21:18:52]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2017-05-16 21:18:52]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2879923003/#ps60001
(title: "ac")

[commit-bot: I haz the power, 2017-05-16 21:18:54]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kozy, 2017-05-16 21:26:52]

Description was changed from

==========
[inspector] added strategy for continueToLocation

By default we just break when we first time reach passed location, with
inCurrentFrame position we'll break at passed location only when it happens
within same stack frame.

BUG=v8:6397
R=dgozman@chromium.org
==========

to

==========
[inspector] added targetCallFrames for continueToLocation

By default we just break when we first time reach passed location, with current
- we'll break at passed location only when it happens within the same stack
frame.

BUG=v8:6397
R=dgozman@chromium.org
==========

[commit-bot: I haz the power, 2017-05-16 21:52:49]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494969532128820,
"parent_rev": "b02bb408ef783ee9102d9d558f6602edc311cdcb", "commit_rev":
"d7e09f8fccbd11e10cd149a8ed6c21c50b6cedb2"}

[commit-bot: I haz the power, 2017-05-16 21:52:55]

Description was changed from

==========
[inspector] added targetCallFrames for continueToLocation

By default we just break when we first time reach passed location, with current
- we'll break at passed location only when it happens within the same stack
frame.

BUG=v8:6397
R=dgozman@chromium.org
==========

to

==========
[inspector] added targetCallFrames for continueToLocation

By default we just break when we first time reach passed location, with current
- we'll break at passed location only when it happens within the same stack
frame.

BUG=v8:6397
R=dgozman@chromium.org

Review-Url: https://codereview.chromium.org/2879923003
Cr-Commit-Position: refs/heads/master@{#45354}
Committed:
https://chromium.googlesource.com/v8/v8/+/d7e09f8fccbd11e10cd149a8ed6c21c50b6...
==========

[commit-bot: I haz the power, 2017-05-16 21:52:56]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/v8/v8/+/d7e09f8fccbd11e10cd149a8ed6c21c50b6...