Debug: parameterize 'step over' action with a frame where the step must be performed

Debug: parameterize 'step over' action with a frame where the step must be performed

R=yangguo@chromium.org

Committed: http://code.google.com/p/v8/source/detail?r=16581

[Yang, 2013-09-05 15:19:54]

Not quite done yet. First round of comments. I'll continue tomorrow.

https://codereview.chromium.org/23533015/diff/1/src/debug-debugger.js
File src/debug-debugger.js (right):

https://codereview.chromium.org/23533015/diff/1/src/debug-debugger.js#newcode960
src/debug-debugger.js:960: ExecutionState.prototype.prepareStep =
function(opt_action, opt_count, opt_callframe) {
80 char limit.

https://codereview.chromium.org/23533015/diff/1/src/debug.cc
File src/debug.cc (right):

https://codereview.chromium.org/23533015/diff/1/src/debug.cc#newcode1380
src/debug.cc:1380: StackFrame::Id frame_id) {
same here, just put one argument per line.

https://codereview.chromium.org/23533015/diff/1/src/debug.h
File src/debug.h (right):

https://codereview.chromium.org/23533015/diff/1/src/debug.h#newcode263
src/debug.h:263: StackFrame::Id frame_id);
just put one argument per line please.

https://codereview.chromium.org/23533015/diff/1/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/23533015/diff/1/src/runtime.cc#newcode12583
src/runtime.cc:12583: if (frame_id != 0 && step_action != StepNext &&
step_action != StepMin &&
probably better to compare frame_id to StackFrame::NO_ID and not the constant
value behind it.

https://codereview.chromium.org/23533015/diff/1/src/runtime.cc#newcode12599
src/runtime.cc:12599: step_count, frame_id);
put frame_id on a new line or all arguments on the second line for consistent
formatting.

[Peter.Rybin, 2013-09-05 16:58:21]

Thank you. Please feel free to ask if you want any comments about the change.

https://codereview.chromium.org/23533015/diff/1/src/debug-debugger.js
File src/debug-debugger.js (right):

https://codereview.chromium.org/23533015/diff/1/src/debug-debugger.js#newcode960
src/debug-debugger.js:960: ExecutionState.prototype.prepareStep =
function(opt_action, opt_count, opt_callframe) {
On 2013/09/05 15:19:54, Yang wrote:
> 80 char limit.

Done.

https://codereview.chromium.org/23533015/diff/1/src/debug.cc
File src/debug.cc (right):

https://codereview.chromium.org/23533015/diff/1/src/debug.cc#newcode1380
src/debug.cc:1380: StackFrame::Id frame_id) {
On 2013/09/05 15:19:54, Yang wrote:
> same here, just put one argument per line.

Done.

https://codereview.chromium.org/23533015/diff/1/src/debug.h
File src/debug.h (right):

https://codereview.chromium.org/23533015/diff/1/src/debug.h#newcode263
src/debug.h:263: StackFrame::Id frame_id);
On 2013/09/05 15:19:54, Yang wrote:
> just put one argument per line please.

Done.

https://codereview.chromium.org/23533015/diff/1/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/23533015/diff/1/src/runtime.cc#newcode12583
src/runtime.cc:12583: if (frame_id != 0 && step_action != StepNext &&
step_action != StepMin &&
On 2013/09/05 15:19:54, Yang wrote:
> probably better to compare frame_id to StackFrame::NO_ID and not the constant
> value behind it.

Done.

https://codereview.chromium.org/23533015/diff/1/src/runtime.cc#newcode12599
src/runtime.cc:12599: step_count, frame_id);
On 2013/09/05 15:19:54, Yang wrote:
> put frame_id on a new line or all arguments on the second line for consistent
> formatting.

Done.

[Yang, 2013-09-06 11:59:12]

On 2013/09/05 16:58:21, Peter.Rybin wrote:
> Thank you. Please feel free to ask if you want any comments about the change.
> 
> https://codereview.chromium.org/23533015/diff/1/src/debug-debugger.js
> File src/debug-debugger.js (right):
> 
>
https://codereview.chromium.org/23533015/diff/1/src/debug-debugger.js#newcode960
> src/debug-debugger.js:960: ExecutionState.prototype.prepareStep =
> function(opt_action, opt_count, opt_callframe) {
> On 2013/09/05 15:19:54, Yang wrote:
> > 80 char limit.
> 
> Done.
> 
> https://codereview.chromium.org/23533015/diff/1/src/debug.cc
> File src/debug.cc (right):
> 
> https://codereview.chromium.org/23533015/diff/1/src/debug.cc#newcode1380
> src/debug.cc:1380: StackFrame::Id frame_id) {
> On 2013/09/05 15:19:54, Yang wrote:
> > same here, just put one argument per line.
> 
> Done.
> 
> https://codereview.chromium.org/23533015/diff/1/src/debug.h
> File src/debug.h (right):
> 
> https://codereview.chromium.org/23533015/diff/1/src/debug.h#newcode263
> src/debug.h:263: StackFrame::Id frame_id);
> On 2013/09/05 15:19:54, Yang wrote:
> > just put one argument per line please.
> 
> Done.
> 
> https://codereview.chromium.org/23533015/diff/1/src/runtime.cc
> File src/runtime.cc (right):
> 
> https://codereview.chromium.org/23533015/diff/1/src/runtime.cc#newcode12583
> src/runtime.cc:12583: if (frame_id != 0 && step_action != StepNext &&
> step_action != StepMin &&
> On 2013/09/05 15:19:54, Yang wrote:
> > probably better to compare frame_id to StackFrame::NO_ID and not the
constant
> > value behind it.
> 
> Done.
> 
> https://codereview.chromium.org/23533015/diff/1/src/runtime.cc#newcode12599
> src/runtime.cc:12599: step_count, frame_id);
> On 2013/09/05 15:19:54, Yang wrote:
> > put frame_id on a new line or all arguments on the second line for
consistent
> > formatting.
> 
> Done.

LGTM.

[Peter.Rybin, 2013-09-08 19:05:42]

Committed patchset #2 manually as r16581 (presubmit successful).