Stack traces now capture nested eval info if present. (issue 256790043 by erights@gmail.com)

9 views
Skip to first unread message

re...@codereview-hr.appspotmail.com

unread,
Jul 11, 2015, 11:04:23 PM7/11/15
to kpr...@google.com, eri...@gmail.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com
Reviewers: kpreid_google,

Description:
Chrome makes stacktrace info available in structured for through the
debugging API, including nested positions for nested eval positions,
except for https://code.google.com/p/v8/issues/detail?id=4268 . Before
this CL we were not extracting nested eval position info. Now we do,
while working around that bug.

FF makes stacktrace info available as a string, including nested eval
position info. Before this CL, we were scraping this string
imperfectly with RegExps, but missing the eval position info. Now we
scrape that as well.

Before this CL, we were using Causeway's JSON format for representing
stack trace structure. But Causeway's format did not anticipate nested
eval positions. This CL defines and uses an extended Causeway format
that includes room for this info.

Enhace the rendering of a Causeway stack into a v8-like stack trace
string to also render the nested eval positions of the extended
Causeway format.

Adds a confineLater to go with compileExprLater,
by analogy to confine and compileExpr.

Revise explicit.html to show these enhanced stacktraces, while also
showing more cases of eval strings that are and are not annotated with
source position info.

Please review this at https://codereview.appspot.com/256790043/

Affected files (+365, -218 lines):
M src/com/google/caja/ses/compileExprLater.js
M src/com/google/caja/ses/debug.js
M src/com/google/caja/ses/explicit.html
M src/com/google/caja/ses/useHTMLLogger.js


kpr...@google.com

unread,
Jul 20, 2015, 12:34:44 PM7/20/15
to eri...@gmail.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com

https://codereview.appspot.com/256790043/diff/1/src/com/google/caja/ses/debug.js
File src/com/google/caja/ses/debug.js (right):

https://codereview.appspot.com/256790043/diff/1/src/com/google/caja/ses/debug.js#newcode129
src/com/google/caja/ses/debug.js:129: var line2CWFrame = (function() {
Add a doc comment explaining the line2CWFrame function.

https://codereview.appspot.com/256790043/diff/1/src/com/google/caja/ses/debug.js#newcode146
src/com/google/caja/ses/debug.js:146: var FFFramePattern =
/^\s*([^:@(]*?)\s*(?:\(.*\))?@(.*?)$/;
remove extra whitespace (alignment is not helpful here).

https://codereview.appspot.com/256790043/diff/1/src/com/google/caja/ses/debug.js#newcode291
src/com/google/caja/ses/debug.js:291: UnsafeError.stackTraceLimit = 100;
Do we really want to do this ourselves?

https://codereview.appspot.com/256790043/diff/1/src/com/google/caja/ses/debug.js#newcode341
src/com/google/caja/ses/debug.js:341: * <p>The difference between
Causeway format and extended
Don't document the format primarily here. Document it at the point where
it is returned to outside users.

https://codereview.appspot.com/256790043/

eri...@gmail.com

unread,
Jul 20, 2015, 6:20:48 PM7/20/15
to kpr...@google.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com

https://codereview.appspot.com/256790043/diff/1/src/com/google/caja/ses/debug.js
File src/com/google/caja/ses/debug.js (right):

https://codereview.appspot.com/256790043/diff/1/src/com/google/caja/ses/debug.js#newcode129
src/com/google/caja/ses/debug.js:129: var line2CWFrame = (function() {
On 2015/07/20 16:34:43, kpreid_google wrote:
> Add a doc comment explaining the line2CWFrame function.

Done.

https://codereview.appspot.com/256790043/diff/1/src/com/google/caja/ses/debug.js#newcode146
src/com/google/caja/ses/debug.js:146: var FFFramePattern =
/^\s*([^:@(]*?)\s*(?:\(.*\))?@(.*?)$/;
On 2015/07/20 16:34:43, kpreid_google wrote:
> remove extra whitespace (alignment is not helpful here).

Done.

https://codereview.appspot.com/256790043/diff/1/src/com/google/caja/ses/debug.js#newcode291
src/com/google/caja/ses/debug.js:291: UnsafeError.stackTraceLimit = 100;
On 2015/07/20 16:34:43, kpreid_google wrote:
> Do we really want to do this ourselves?

Gone.

https://codereview.appspot.com/256790043/diff/1/src/com/google/caja/ses/debug.js#newcode341
src/com/google/caja/ses/debug.js:341: * <p>The difference between
Causeway format and extended
On 2015/07/20 16:34:43, kpreid_google wrote:
> Don't document the format primarily here. Document it at the point
where it is
> returned to outside users.

Done. I explained in the doc-comment for the file.

https://codereview.appspot.com/256790043/

kpr...@google.com

unread,
Jul 20, 2015, 6:43:33 PM7/20/15
to eri...@gmail.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com

eri...@gmail.com

unread,
Jul 23, 2015, 3:18:44 PM7/23/15
to kpr...@google.com, google-ca...@googlegroups.com, re...@codereview-hr.appspotmail.com
at 6774ff211039c6ca84e6c4111ed2384b43fc4abd

https://codereview.appspot.com/256790043/
Reply all
Reply to author
Forward
0 new messages