Thanks for the review. I will need a committer to land this for me.
http://codereview.chromium.org/1694011/diff/55001/56002 File include/v8.h (right): http://codereview.chromium.org/1694011/diff/55001/56002#newcode697 include/v8.h:697: static const int kNoLineNumberInfo = 0; On 2010/05/05 07:24:19, Søren Gjesse wrote:
Please remove empty line.
Done. http://codereview.chromium.org/1694011/diff/55001/56002#newcode704 include/v8.h:704: * A JavaScript stack trace. This will only include JavaScript associated On 2010/05/05 07:24:19, Søren Gjesse wrote:
Please revise this comment into soething like this
"Representation of a JavaScript stack trace. The information
collected is a
snapshot of the execution stack and the information remains valid
after
execution continues."
I don't think we should mention stuff about V8's native runtime.js
here as
embedders should not worry about that.
Done. But keep in mind that the stack trace stuff in messages.js and runtime.cc does in fact look at frames from native.js I was thinking that we would, in a subsequent patch, make this API take in a flag that determines whether or not to look at frames from native.js so that we can support that use case. The default ofcourse would remain without that flag. For now though, I just removed the reference to it. http://codereview.chromium.org/1694011/diff/55001/56002#newcode741 include/v8.h:741: * Grabs the current JavaScript StackTrace. On 2010/05/05 07:24:19, Søren Gjesse wrote:
Grabs the current JavaScript StackTrace. -> Grab a snapshot of the the
current
JavaScript execution stack.
Done. http://codereview.chromium.org/1694011/diff/55001/56002#newcode760 include/v8.h:760: * This method will return kNotFound if it is unable to retrieve the line On 2010/05/05 07:24:19, Søren Gjesse wrote:
kNotFound -> Message::kNoLineNumberInfo
Done. http://codereview.chromium.org/1694011/diff/55001/56002#newcode761 include/v8.h:761: * number, or if LINE_NUMBER was not passed as an option when capturing the On 2010/05/05 07:24:19, Søren Gjesse wrote:
LINE_NUMBER -> kLineNumber
Done. http://codereview.chromium.org/1694011/diff/55001/56002#newcode769 include/v8.h:769: * This method will return kNotFound if it is unable to retrieve the column On 2010/05/05 07:24:19, Søren Gjesse wrote:
kNotFound -> Message::kNoLineNumberInfo
You mean Message::kNoColumnInfo http://codereview.chromium.org/1694011/diff/55001/56002#newcode770 include/v8.h:770: * number, or if COLUMN_OFFSET was not passed as an option when capturing the On 2010/05/05 07:24:19, Søren Gjesse wrote:
COLUMN_OFFSET -> kColumnOffset
Done. http://codereview.chromium.org/1694011/diff/55001/56002#newcode787 include/v8.h:787: * Returns whether or not the associated function was run via a call to On 2010/05/05 07:24:19, Søren Gjesse wrote:
was run -> is compiled
Done. http://codereview.chromium.org/1694011/diff/55001/56002#newcode793 include/v8.h:793: * Returns whther or not the associated function is being run as a On 2010/05/05 07:24:19, Søren Gjesse wrote:
being run -> called
Done. http://codereview.chromium.org/1694011/diff/55001/56008 File src/messages.js (right): http://codereview.chromium.org/1694011/diff/55001/56008#newcode205 src/messages.js:205: function GetLineNumber(message) { On 2010/05/05 07:24:19, Søren Gjesse wrote:
Please add the constant kNoLineNumberInfo to JavaScript somewhere.
Done. http://codereview.chromium.org/1694011/diff/55001/56004 File src/top.cc (right): http://codereview.chromium.org/1694011/diff/55001/56004#newcode399 src/top.cc:399: SetProperty(stackFrame, Factory::LookupAsciiSymbol("column"), On 2010/05/05 07:24:19, Søren Gjesse wrote:
You might consider moving the calls to Factory::LookupAsciiSymbol(xxx)
to before
the loop so there will only be one lookup for each.
Done. http://codereview.chromium.org/1694011/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
