[debugger] tuned StepNext and StepOut at return position

[debugger] tuned StepNext and StepOut at return position

Proposed behaviour:
- StepNext at return position go into next function call (no changes with current behavior, but implemented in v8::Debug instead of hack on inspector side);
- StepOut at return position go into next non-current function call.

We need this to have better stepping in cases with native functions, blackboxed functions and/or different embedder calls (e.g. event listeners).

New behavior could be illustrated with two examples (for more see stepping-with-natives-and-frameworks test):
- let's assume that we've blackboxed callAll function, this function just takes its arguments and call one after another:
var foo = () => 1;
callAll(foo, foo, () => 2);
If we break inside of first call of function foo. Then on..
..StepNext - we're able to reach second call of function foo,
..StepOut - we're able to reach () => 2 call.

- let's consider case with native function:
[1,2,3].map(x => x * 2)
If we break inside of first callback call, then with StepNext we can iterate through all calls of callback, with StepOut we go to next statement after .map call.

Implementation details:
- when we request break we schedule step-in function call for any step action at return position and for step-in at any position,
- when we request StepOut at return position - we mark current function as needed-to-be-ignored inside of PrepareStepIn(function) call,
- when we request StepOut at not return position - we set break at return position and ask debugger to just repeat last step action on next stepping-related break.

Design doc: https://docs.google.com/document/d/1ihXHOIhP_q-fJCA0e2EiXz_Zr3B08KMjaPifcaqZ60Q/edit

BUG=v8:6118, chromium:583193
R=dgozman@chromium.org,yangguo@chromium.org

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

[kozy, 2017-03-16 07:19:01]

Yang, please take a look. This is implementation of alternative solution from
https://docs.google.com/document/d/1ihXHOIhP_q-fJCA0e2EiXz_Zr3B08KMjaPifcaqZ6...

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

https://codereview.chromium.org/2758483002/diff/1/src/inspector/v8-debugger-a...
src/inspector/v8-debugger-agent-impl.cc:734: // JavaScriptCallFrame* frame =
I can remove it in this CL since we use to step between embedder call, I'll be
able to remove it in followup:
https://codereview.chromium.org/2748503002/

[kozy, 2017-03-16 07:20:10]

Description was changed from

==========
[debugger] StepOut and StepNext support blackboxed code

It's PoC alternative idea from:
https://docs.google.com/document/d/1ihXHOIhP_q-fJCA0e2EiXz_Zr3B08KMjaPifcaqZ6...
On all step actions we enable DebugOnFunctionCall callback. On StepNext and
StepOut we additionally store current frame id and will actually break
OnFunctionCall iff stored frame is not predented on current frame.

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

to

==========
[debugger] StepOut and StepNext support blackboxed code better

It's PoC alternative idea from:
https://docs.google.com/document/d/1ihXHOIhP_q-fJCA0e2EiXz_Zr3B08KMjaPifcaqZ6...
On all step actions we enable DebugOnFunctionCall callback. On StepNext and
StepOut we additionally store current frame id and will actually break
OnFunctionCall iff stored frame is not predented on current frame.

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

[kozy, 2017-03-18 00:39:14]

Description was changed from

==========
[debugger] StepOut and StepNext support blackboxed code better

It's PoC alternative idea from:
https://docs.google.com/document/d/1ihXHOIhP_q-fJCA0e2EiXz_Zr3B08KMjaPifcaqZ6...
On all step actions we enable DebugOnFunctionCall callback. On StepNext and
StepOut we additionally store current frame id and will actually break
OnFunctionCall iff stored frame is not predented on current frame.

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

to

==========
[debugger] StepNext and StepOut at return position is converted to StepInto

Proposed behaviour:
- StepNext at return position go into next function call (no changes with
current behavior, but implemented in v8::Debug instead of hack on inspector
side);
- StepOut at return position go into next non-current function call.

We need this to have better stepping in cases with native functions, blackboxed
functions and/or different embedder calls (e.g. event listeners).

New behavior could be illustrated with two examples:
- let's assume that we've blackboxed callAll function, this function just takes
its arguments and call one after another:
var foo = () => 1;
callAll(foo, foo, () => 2);
If we break inside of first call of function foo. Then on..
..StepNext - we're able to reach second call of function foo,
..StepOut - we're able to reach () => 2 call.

- let's consider case with native function:
[1,2,3].map(x => x * 2)
If we break inside of first callback call, then with StepNext we can iterate
through all calls of callback, with StepOut we go to next statement after .map
call.

Implementation details:
- when we request break we schedule step-in function call for any step action at
return position and for step-in at any position,
- when we request StepOut at return position - we mark current function as
needed-to-be-ignored inside of PrepareStepIn(function) call,
- when we request StepOut at not return position - we set break at return
position and ask debugger to just repeat last step action on next
stepping-related break.

Design doc:
https://docs.google.com/document/d/1ihXHOIhP_q-fJCA0e2EiXz_Zr3B08KMjaPifcaqZ6...

BUG=v8:6118
R=dgozman@chromium.org,yangguo@chromium.org
==========

[kozy, 2017-03-18 00:39:43]

Description was changed from

==========
[debugger] StepNext and StepOut at return position is converted to StepInto

Proposed behaviour:
- StepNext at return position go into next function call (no changes with
current behavior, but implemented in v8::Debug instead of hack on inspector
side);
- StepOut at return position go into next non-current function call.

We need this to have better stepping in cases with native functions, blackboxed
functions and/or different embedder calls (e.g. event listeners).

New behavior could be illustrated with two examples:
- let's assume that we've blackboxed callAll function, this function just takes
its arguments and call one after another:
var foo = () => 1;
callAll(foo, foo, () => 2);
If we break inside of first call of function foo. Then on..
..StepNext - we're able to reach second call of function foo,
..StepOut - we're able to reach () => 2 call.

- let's consider case with native function:
[1,2,3].map(x => x * 2)
If we break inside of first callback call, then with StepNext we can iterate
through all calls of callback, with StepOut we go to next statement after .map
call.

Implementation details:
- when we request break we schedule step-in function call for any step action at
return position and for step-in at any position,
- when we request StepOut at return position - we mark current function as
needed-to-be-ignored inside of PrepareStepIn(function) call,
- when we request StepOut at not return position - we set break at return
position and ask debugger to just repeat last step action on next
stepping-related break.

Design doc:
https://docs.google.com/document/d/1ihXHOIhP_q-fJCA0e2EiXz_Zr3B08KMjaPifcaqZ6...

BUG=v8:6118
R=dgozman@chromium.org,yangguo@chromium.org
==========

to

==========
[debugger] tuned StepNext and StepOut at return position

Proposed behaviour:
- StepNext at return position go into next function call (no changes with
current behavior, but implemented in v8::Debug instead of hack on inspector
side);
- StepOut at return position go into next non-current function call.

We need this to have better stepping in cases with native functions, blackboxed
functions and/or different embedder calls (e.g. event listeners).

New behavior could be illustrated with two examples:
- let's assume that we've blackboxed callAll function, this function just takes
its arguments and call one after another:
var foo = () => 1;
callAll(foo, foo, () => 2);
If we break inside of first call of function foo. Then on..
..StepNext - we're able to reach second call of function foo,
..StepOut - we're able to reach () => 2 call.

- let's consider case with native function:
[1,2,3].map(x => x * 2)
If we break inside of first callback call, then with StepNext we can iterate
through all calls of callback, with StepOut we go to next statement after .map
call.

Implementation details:
- when we request break we schedule step-in function call for any step action at
return position and for step-in at any position,
- when we request StepOut at return position - we mark current function as
needed-to-be-ignored inside of PrepareStepIn(function) call,
- when we request StepOut at not return position - we set break at return
position and ask debugger to just repeat last step action on next
stepping-related break.

Design doc:
https://docs.google.com/document/d/1ihXHOIhP_q-fJCA0e2EiXz_Zr3B08KMjaPifcaqZ6...

BUG=v8:6118
R=dgozman@chromium.org,yangguo@chromium.org
==========

[kozy, 2017-03-18 00:44:23]

Description was changed from

==========
[debugger] tuned StepNext and StepOut at return position

Proposed behaviour:
- StepNext at return position go into next function call (no changes with
current behavior, but implemented in v8::Debug instead of hack on inspector
side);
- StepOut at return position go into next non-current function call.

We need this to have better stepping in cases with native functions, blackboxed
functions and/or different embedder calls (e.g. event listeners).

New behavior could be illustrated with two examples:
- let's assume that we've blackboxed callAll function, this function just takes
its arguments and call one after another:
var foo = () => 1;
callAll(foo, foo, () => 2);
If we break inside of first call of function foo. Then on..
..StepNext - we're able to reach second call of function foo,
..StepOut - we're able to reach () => 2 call.

- let's consider case with native function:
[1,2,3].map(x => x * 2)
If we break inside of first callback call, then with StepNext we can iterate
through all calls of callback, with StepOut we go to next statement after .map
call.

Implementation details:
- when we request break we schedule step-in function call for any step action at
return position and for step-in at any position,
- when we request StepOut at return position - we mark current function as
needed-to-be-ignored inside of PrepareStepIn(function) call,
- when we request StepOut at not return position - we set break at return
position and ask debugger to just repeat last step action on next
stepping-related break.

Design doc:
https://docs.google.com/document/d/1ihXHOIhP_q-fJCA0e2EiXz_Zr3B08KMjaPifcaqZ6...

BUG=v8:6118
R=dgozman@chromium.org,yangguo@chromium.org
==========

to

==========
[debugger] tuned StepNext and StepOut at return position

Proposed behaviour:
- StepNext at return position go into next function call (no changes with
current behavior, but implemented in v8::Debug instead of hack on inspector
side);
- StepOut at return position go into next non-current function call.

We need this to have better stepping in cases with native functions, blackboxed
functions and/or different embedder calls (e.g. event listeners).

New behavior could be illustrated with two examples (for more see
stepping-with-natives-and-frameworks test):
- let's assume that we've blackboxed callAll function, this function just takes
its arguments and call one after another:
var foo = () => 1;
callAll(foo, foo, () => 2);
If we break inside of first call of function foo. Then on..
..StepNext - we're able to reach second call of function foo,
..StepOut - we're able to reach () => 2 call.

- let's consider case with native function:
[1,2,3].map(x => x * 2)
If we break inside of first callback call, then with StepNext we can iterate
through all calls of callback, with StepOut we go to next statement after .map
call.

Implementation details:
- when we request break we schedule step-in function call for any step action at
return position and for step-in at any position,
- when we request StepOut at return position - we mark current function as
needed-to-be-ignored inside of PrepareStepIn(function) call,
- when we request StepOut at not return position - we set break at return
position and ask debugger to just repeat last step action on next
stepping-related break.

Design doc:
https://docs.google.com/document/d/1ihXHOIhP_q-fJCA0e2EiXz_Zr3B08KMjaPifcaqZ6...

BUG=v8:6118
R=dgozman@chromium.org,yangguo@chromium.org
==========

[kozy, 2017-03-18 00:45:55]

The CQ bit was checked by kozyatinskiy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-18 00:45:56]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kozy, 2017-03-18 00:48:06]

Yang and Dmitry, please take a look before meeting on Monday, this CL shows that
we can implement proposed in Universal stepping doc logic by 40 lines of code
and behavior looks and feels good.

https://codereview.chromium.org/2758483002/diff/80001/test/cctest/test-debug.cc
File test/cctest/test-debug.cc (right):

https://codereview.chromium.org/2758483002/diff/80001/test/cctest/test-debug....
test/cctest/test-debug.cc:6708: // With turbofan we generate one return location
per return statement,
I found this behavior during work on this CL and can extract into separate CL if
needed.

[commit-bot: I haz the power, 2017-03-18 01:11:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-18 01:11:56]

Dry run: This issue passed the CQ dry run.

[kozy, 2017-03-20 00:55:58]

Description was changed from

==========
[debugger] tuned StepNext and StepOut at return position

Proposed behaviour:
- StepNext at return position go into next function call (no changes with
current behavior, but implemented in v8::Debug instead of hack on inspector
side);
- StepOut at return position go into next non-current function call.

We need this to have better stepping in cases with native functions, blackboxed
functions and/or different embedder calls (e.g. event listeners).

New behavior could be illustrated with two examples (for more see
stepping-with-natives-and-frameworks test):
- let's assume that we've blackboxed callAll function, this function just takes
its arguments and call one after another:
var foo = () => 1;
callAll(foo, foo, () => 2);
If we break inside of first call of function foo. Then on..
..StepNext - we're able to reach second call of function foo,
..StepOut - we're able to reach () => 2 call.

- let's consider case with native function:
[1,2,3].map(x => x * 2)
If we break inside of first callback call, then with StepNext we can iterate
through all calls of callback, with StepOut we go to next statement after .map
call.

Implementation details:
- when we request break we schedule step-in function call for any step action at
return position and for step-in at any position,
- when we request StepOut at return position - we mark current function as
needed-to-be-ignored inside of PrepareStepIn(function) call,
- when we request StepOut at not return position - we set break at return
position and ask debugger to just repeat last step action on next
stepping-related break.

Design doc:
https://docs.google.com/document/d/1ihXHOIhP_q-fJCA0e2EiXz_Zr3B08KMjaPifcaqZ6...

BUG=v8:6118
R=dgozman@chromium.org,yangguo@chromium.org
==========

to

==========
[debugger] tuned StepNext and StepOut at return position

Proposed behaviour:
- StepNext at return position go into next function call (no changes with
current behavior, but implemented in v8::Debug instead of hack on inspector
side);
- StepOut at return position go into next non-current function call.

We need this to have better stepping in cases with native functions, blackboxed
functions and/or different embedder calls (e.g. event listeners).

New behavior could be illustrated with two examples (for more see
stepping-with-natives-and-frameworks test):
- let's assume that we've blackboxed callAll function, this function just takes
its arguments and call one after another:
var foo = () => 1;
callAll(foo, foo, () => 2);
If we break inside of first call of function foo. Then on..
..StepNext - we're able to reach second call of function foo,
..StepOut - we're able to reach () => 2 call.

- let's consider case with native function:
[1,2,3].map(x => x * 2)
If we break inside of first callback call, then with StepNext we can iterate
through all calls of callback, with StepOut we go to next statement after .map
call.

Implementation details:
- when we request break we schedule step-in function call for any step action at
return position and for step-in at any position,
- when we request StepOut at return position - we mark current function as
needed-to-be-ignored inside of PrepareStepIn(function) call,
- when we request StepOut at not return position - we set break at return
position and ask debugger to just repeat last step action on next
stepping-related break.

Design doc:
https://docs.google.com/document/d/1ihXHOIhP_q-fJCA0e2EiXz_Zr3B08KMjaPifcaqZ6...

BUG=v8:6118,chromium:583193
R=dgozman@chromium.org,yangguo@chromium.org
==========

[Yang, 2017-03-20 14:29:40]

https://codereview.chromium.org/2758483002/diff/80001/src/debug/debug.cc
File src/debug/debug.cc (right):

https://codereview.chromium.org/2758483002/diff/80001/src/debug/debug.cc#newc...
src/debug/debug.cc:891: thread_local_.last_step_action_ = StepIn;
We should clear thread_local_.ignore_step_into_function_ here. Otherwise it may
lead to memory leak. The function would hold onto the entire native context.

[kozy, 2017-03-20 15:52:54]

all done.

https://codereview.chromium.org/2758483002/diff/80001/src/debug/debug.cc
File src/debug/debug.cc (right):

https://codereview.chromium.org/2758483002/diff/80001/src/debug/debug.cc#newc...
src/debug/debug.cc:891: thread_local_.last_step_action_ = StepIn;
On 2017/03/20 14:29:40, Yang wrote:
> We should clear thread_local_.ignore_step_into_function_ here. Otherwise it
may
> lead to memory leak. The function would hold onto the entire native context.

Done.

[kozy, 2017-03-20 15:53:04]

The CQ bit was checked by kozyatinskiy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-20 15:53:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-20 16:20:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-20 16:20:22]

Dry run: This issue passed the CQ dry run.

[Yang, 2017-03-21 13:13:52]

https://codereview.chromium.org/2758483002/diff/100001/src/debug/debug.cc
File src/debug/debug.cc (right):

https://codereview.chromium.org/2758483002/diff/100001/src/debug/debug.cc#new...
src/debug/debug.cc:423: v->VisitPointer(&thread_local_.suspended_generator_);
You need to iterate ignore_step_into_function_, or otherwise GC will not update
the pointer correctly.

https://codereview.chromium.org/2758483002/diff/100001/src/debug/debug.cc#new...
src/debug/debug.cc:1027: if (location.IsReturn()) {
Please add comments!

How about this slight change:
- If we perform StepOut, and we are not at a return, we set a
fast_forward_to_return_ flag, and instrument all returns of the current
function. (yes, let's rename ignore_next_stepping_break_).
- In Debug::Break, if the fast_forward_to_return flag is set, we simply DCHECK
that we are indeed at a return, and the target frame count is the same as the
current frame count (to prevent breaking in a recursive call), and call
PrepareStep to StepOut.

- If we perform StepOut, and we are at a return, we store StepIn to
last_step_action_, remember ignore_step_into_function_, and also instrument the
caller. When then UpdateHookInFunctionCall.
- Afterwards, we could break either in the caller, or we could break through
DebugOnFunctionCall. There, we check that we are not stepping into
ignore_step_info_function_.

This way, I don't think we have to introduce a
step_into_function_call_requested_ flag.

https://codereview.chromium.org/2758483002/diff/100001/src/debug/debug.cc#new...
src/debug/debug.cc:1037: if (location.IsReturn()) step_action = StepOut;
We already have this here.

https://codereview.chromium.org/2758483002/diff/100001/src/debug/debug.cc#new...
src/debug/debug.cc:1058: if (!location.IsReturn() && !IsBlackboxed(shared)) {
How can IsBlackboxed return true?

[kozy, 2017-03-21 16:03:12]

all done, please take another look.

https://codereview.chromium.org/2758483002/diff/100001/src/debug/debug.cc
File src/debug/debug.cc (right):

https://codereview.chromium.org/2758483002/diff/100001/src/debug/debug.cc#new...
src/debug/debug.cc:423: v->VisitPointer(&thread_local_.suspended_generator_);
On 2017/03/21 13:13:52, Yang wrote:
> You need to iterate ignore_step_into_function_, or otherwise GC will not
update
> the pointer correctly.

Thanks! done.

https://codereview.chromium.org/2758483002/diff/100001/src/debug/debug.cc#new...
src/debug/debug.cc:1027: if (location.IsReturn()) {
On 2017/03/21 13:13:52, Yang wrote:
> Please add comments!
> 
> How about this slight change:
> - If we perform StepOut, and we are not at a return, we set a
> fast_forward_to_return_ flag, and instrument all returns of the current
> function. (yes, let's rename ignore_next_stepping_break_).
> - In Debug::Break, if the fast_forward_to_return flag is set, we simply DCHECK
> that we are indeed at a return, and the target frame count is the same as the
> current frame count (to prevent breaking in a recursive call), and call
> PrepareStep to StepOut.
> 
> - If we perform StepOut, and we are at a return, we store StepIn to
> last_step_action_, remember ignore_step_into_function_, and also instrument
the
> caller. When then UpdateHookInFunctionCall.
> - Afterwards, we could break either in the caller, or we could break through
> DebugOnFunctionCall. There, we check that we are not stepping into
> ignore_step_info_function_.
> 
> This way, I don't think we have to introduce a
> step_into_function_call_requested_ flag.

Done! My goals with current solutions were: doesn't change last_step_action_
since I think about this as a last user requested step action and have minimum
changes in Break. But at the same time I like idea to introduce less additional
state.

https://codereview.chromium.org/2758483002/diff/100001/src/debug/debug.cc#new...
src/debug/debug.cc:1037: if (location.IsReturn()) step_action = StepOut;
On 2017/03/21 13:13:52, Yang wrote:
> We already have this here.

removed.

https://codereview.chromium.org/2758483002/diff/100001/src/debug/debug.cc#new...
src/debug/debug.cc:1058: if (!location.IsReturn() && !IsBlackboxed(shared)) {
On 2017/03/21 13:13:52, Yang wrote:
> How can IsBlackboxed return true?

we can break in blackboxed frame in case when we have not blackboxed frames (not
top one) and call V8Inspector::breakProgram, in line 1041 we do:
if (step_action == StepNext && IsBlackboxed(shared)) step_action = StepOut;

[dgozman, 2017-03-21 23:33:22]

tests and behavior lgtm!

https://codereview.chromium.org/2758483002/diff/120001/src/debug/debug.h
File src/debug/debug.h (right):

https://codereview.chromium.org/2758483002/diff/120001/src/debug/debug.h#newc...
src/debug/debug.h:569: // If set then this function will be ignored in
PrepareStepIn call.
If set, next PrepareStepIn will ignore this function until stepped into another
function, at which point this will be cleared.

https://codereview.chromium.org/2758483002/diff/120001/test/inspector/protoco...
File test/inspector/protocol-test.js (right):

https://codereview.chromium.org/2758483002/diff/120001/test/inspector/protoco...
test/inspector/protocol-test.js:10: InspectorTest._commandToLog = new Set();
_commandsForLogging

https://codereview.chromium.org/2758483002/diff/120001/test/inspector/protoco...
test/inspector/protocol-test.js:34: InspectorTest.dumpProtocolCommand =
(command) => InspectorTest._commandToLog.add(command);
logProtocolCommandCalls

[Yang, 2017-03-22 11:56:20]

lgtm. thanks!

[kozy, 2017-03-22 13:50:41]

The CQ bit was checked by kozyatinskiy@chromium.org

[commit-bot: I haz the power, 2017-03-22 13:50:51]

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

[commit-bot: I haz the power, 2017-03-22 14:16:19]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1490190641909540,
"parent_rev": "ee6467481154e46f5429698a8fceca22827e2484", "commit_rev":
"e27d18c94337f908d75fdccb4c5263180dbb1511"}

[commit-bot: I haz the power, 2017-03-22 14:16:24]

Description was changed from

==========
[debugger] tuned StepNext and StepOut at return position

Proposed behaviour:
- StepNext at return position go into next function call (no changes with
current behavior, but implemented in v8::Debug instead of hack on inspector
side);
- StepOut at return position go into next non-current function call.

We need this to have better stepping in cases with native functions, blackboxed
functions and/or different embedder calls (e.g. event listeners).

New behavior could be illustrated with two examples (for more see
stepping-with-natives-and-frameworks test):
- let's assume that we've blackboxed callAll function, this function just takes
its arguments and call one after another:
var foo = () => 1;
callAll(foo, foo, () => 2);
If we break inside of first call of function foo. Then on..
..StepNext - we're able to reach second call of function foo,
..StepOut - we're able to reach () => 2 call.

- let's consider case with native function:
[1,2,3].map(x => x * 2)
If we break inside of first callback call, then with StepNext we can iterate
through all calls of callback, with StepOut we go to next statement after .map
call.

Implementation details:
- when we request break we schedule step-in function call for any step action at
return position and for step-in at any position,
- when we request StepOut at return position - we mark current function as
needed-to-be-ignored inside of PrepareStepIn(function) call,
- when we request StepOut at not return position - we set break at return
position and ask debugger to just repeat last step action on next
stepping-related break.

Design doc:
https://docs.google.com/document/d/1ihXHOIhP_q-fJCA0e2EiXz_Zr3B08KMjaPifcaqZ6...

BUG=v8:6118,chromium:583193
R=dgozman@chromium.org,yangguo@chromium.org
==========

to

==========
[debugger] tuned StepNext and StepOut at return position

Proposed behaviour:
- StepNext at return position go into next function call (no changes with
current behavior, but implemented in v8::Debug instead of hack on inspector
side);
- StepOut at return position go into next non-current function call.

We need this to have better stepping in cases with native functions, blackboxed
functions and/or different embedder calls (e.g. event listeners).

New behavior could be illustrated with two examples (for more see
stepping-with-natives-and-frameworks test):
- let's assume that we've blackboxed callAll function, this function just takes
its arguments and call one after another:
var foo = () => 1;
callAll(foo, foo, () => 2);
If we break inside of first call of function foo. Then on..
..StepNext - we're able to reach second call of function foo,
..StepOut - we're able to reach () => 2 call.

- let's consider case with native function:
[1,2,3].map(x => x * 2)
If we break inside of first callback call, then with StepNext we can iterate
through all calls of callback, with StepOut we go to next statement after .map
call.

Implementation details:
- when we request break we schedule step-in function call for any step action at
return position and for step-in at any position,
- when we request StepOut at return position - we mark current function as
needed-to-be-ignored inside of PrepareStepIn(function) call,
- when we request StepOut at not return position - we set break at return
position and ask debugger to just repeat last step action on next
stepping-related break.

Design doc:
https://docs.google.com/document/d/1ihXHOIhP_q-fJCA0e2EiXz_Zr3B08KMjaPifcaqZ6...

BUG=v8:6118,chromium:583193
R=dgozman@chromium.org,yangguo@chromium.org

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

[commit-bot: I haz the power, 2017-03-22 14:16:25]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/v8/v8/+/e27d18c94337f908d75fdccb4c5263180db...

[kozy, 2017-03-22 17:09:46]

oops, addressed reset-related comments in separate CL:
https://codereview.chromium.org/2767873002/

https://codereview.chromium.org/2758483002/diff/120001/test/inspector/protoco...
File test/inspector/protocol-test.js (right):

https://codereview.chromium.org/2758483002/diff/120001/test/inspector/protoco...
test/inspector/protocol-test.js:10: InspectorTest._commandToLog = new Set();
On 2017/03/21 23:33:21, dgozman wrote:
> _commandsForLogging

Done.

https://codereview.chromium.org/2758483002/diff/120001/test/inspector/protoco...
test/inspector/protocol-test.js:34: InspectorTest.dumpProtocolCommand =
(command) => InspectorTest._commandToLog.add(command);
On 2017/03/21 23:33:21, dgozman wrote:
> logProtocolCommandCalls

Done.

[kozy, 2017-03-22 17:10:10]

s/reset/tests/

[kozy, 2017-03-22 17:19:53]

correct link to followup CL: https://codereview.chromium.org/2766283002#

https://codereview.chromium.org/2758483002/diff/120001/src/debug/debug.h
File src/debug/debug.h (right):

https://codereview.chromium.org/2758483002/diff/120001/src/debug/debug.h#newc...
src/debug/debug.h:569: // If set then this function will be ignored in
PrepareStepIn call.
On 2017/03/21 23:33:21, dgozman wrote:
> If set, next PrepareStepIn will ignore this function until stepped into
another
> function, at which point this will be cleared.

Done.