Provide list of step-in source positions in JS Debug API

Provide list of step-in source positions in JS Debug API

R=yangguo@chromium.org

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

[Peter.Rybin, 2013-06-09 21:46:57]

ping

[Yang, 2013-06-14 13:41:11]

LGTM with comments.

https://chromiumcodereview.appspot.com/15960016/diff/8001/src/mirror-debugger.js
File src/mirror-debugger.js (right):

https://chromiumcodereview.appspot.com/15960016/diff/8001/src/mirror-debugger...
src/mirror-debugger.js:1686:
serializeLocationFields(script.locationFromPosition(funcOffset + stepInRaw[i],
true), posStruct);
80 char limit.

https://chromiumcodereview.appspot.com/15960016/diff/8001/src/runtime.cc
File src/runtime.cc (right):

https://chromiumcodereview.appspot.com/15960016/diff/8001/src/runtime.cc#newc...
src/runtime.cc:11613: }
Am I understanding this correctly that you are looking for the break location at
current PC, and collecting all break locations after that in the current
function? Maybe add a comment to explain that. It's not too hard to understand,
but the debugger code is complicated enough and every hint to save time
understanding it is valuable.

I'm not entirely sure why you want to do that though, I mean, why do you ignore
the break locations before the current PC?

[Peter.Rybin, 2013-06-14 19:48:00]

https://chromiumcodereview.appspot.com/15960016/diff/8001/src/mirror-debugger.js
File src/mirror-debugger.js (right):

https://chromiumcodereview.appspot.com/15960016/diff/8001/src/mirror-debugger...
src/mirror-debugger.js:1686:
serializeLocationFields(script.locationFromPosition(funcOffset + stepInRaw[i],
true), posStruct);
On 2013/06/14 13:41:12, Yang wrote:
> 80 char limit.

Done.

https://chromiumcodereview.appspot.com/15960016/diff/8001/src/runtime.cc
File src/runtime.cc (right):

https://chromiumcodereview.appspot.com/15960016/diff/8001/src/runtime.cc#newc...
src/runtime.cc:11613: }
On 2013/06/14 13:41:12, Yang wrote:
> Am I understanding this correctly that you are looking for the break location
at
> current PC, and collecting all break locations after that in the current
> function? Maybe add a comment to explain that. It's not too hard to
understand,
> but the debugger code is complicated enough and every hint to save time
> understanding it is valuable.
> 
> I'm not entirely sure why you want to do that though, I mean, why do you
ignore
> the break locations before the current PC?

No, it's absolutely fine. The things is that in some teams comments are
officially discouraged, so it's a cultural shift with me.

The algorithm is trying to find all step-in positions starting from the PC till
the end of the current statement.

Done

[Yang, 2013-06-25 11:17:41]

On 2013/06/14 19:48:00, peter.rybin wrote:
>
https://chromiumcodereview.appspot.com/15960016/diff/8001/src/mirror-debugger.js
> File src/mirror-debugger.js (right):
> 
>
https://chromiumcodereview.appspot.com/15960016/diff/8001/src/mirror-debugger...
> src/mirror-debugger.js:1686:
> serializeLocationFields(script.locationFromPosition(funcOffset + stepInRaw[i],
> true), posStruct);
> On 2013/06/14 13:41:12, Yang wrote:
> > 80 char limit.
> 
> Done.
> 
> https://chromiumcodereview.appspot.com/15960016/diff/8001/src/runtime.cc
> File src/runtime.cc (right):
> 
>
https://chromiumcodereview.appspot.com/15960016/diff/8001/src/runtime.cc#newc...
> src/runtime.cc:11613: }
> On 2013/06/14 13:41:12, Yang wrote:
> > Am I understanding this correctly that you are looking for the break
location
> at
> > current PC, and collecting all break locations after that in the current
> > function? Maybe add a comment to explain that. It's not too hard to
> understand,
> > but the debugger code is complicated enough and every hint to save time
> > understanding it is valuable.
> > 
> > I'm not entirely sure why you want to do that though, I mean, why do you
> ignore
> > the break locations before the current PC?
> 
> No, it's absolutely fine. The things is that in some teams comments are
> officially discouraged, so it's a cultural shift with me.
> 
> The algorithm is trying to find all step-in positions starting from the PC
till
> the end of the current statement.
> 
> Done

LGTM btw.