Properly render FF 30 Nightly stack traces.

FF 30 Nightly stack traces are more informative for nested
evaluation. But the extra info does not correspond well to the
patterns we look for. Extend the lineColPattern into a prioritized
sequence of patterns, in the same way that framePatterns is a
prioritized sequence. Add a lineColPattern specialized for the FF30
Nightly nested eval pattern.

[Mark S. Miller, 2014-02-28 02:14:01]

The difference between the attached screenshots illustrates the issue.


On Thu, Feb 27, 2014 at 6:09 PM, <erights@gmail.com> wrote:

> Reviewers: kpreid2,
>
> Description:
> FF 30 Nightly stack traces are more informative for nested
> evaluation. But the extra info does not correspond well to the
> patterns we look for. Extend the lineColPattern into a prioritized
> sequence of patterns, in the same way that framePatterns is a
> prioritized sequence. Add a lineColPattern specialized for the FF30
> Nightly nested eval pattern.
>
> Please review this at https://codereview.appspot.com/69820043/
>
> Affected files (+29, -11 lines):
>   M     src/com/google/caja/ses/debug.js
>   M     src/com/google/caja/ses/startSES.js
>
>
> Index: src/com/google/caja/ses/debug.js
> ===================================================================
> --- src/com/google/caja/ses/debug.js    (revision 5671)
> +++ src/com/google/caja/ses/debug.js    (working copy)
> @@ -191,12 +191,22 @@
>         var framePatterns = [FFFramePattern, IEFramePattern,
>                              JSCFramePatt1, JSCFramePatt2];
>
> +       // Each of the LineColPatters should have the first capture
> +       // group be the source URL if any, the second by the line
> +       // number if any, and the third be the column number if any.
> +
> +       // Seen on FF Nightly 30 for execution in evaled strings
> +       var FFEvalLineColPatterns = (/^(?:.*?) line \d+ >
> eval():(\d+):(\d+)$/);
>         // If the source position ends in either one or two
>         // colon-digit-sequence suffixes, then the first of these are
>         // the line number, and the second, if present, is the column
>         // number.
> -       var lineColPattern = /^(.*?)(?::(\d+)(?::(\d+))?)?$/;
> +       var MainLineColPattern = /^(.*?)(?::(\d+)(?::(\d+))?)?$/;
>
> +       // List the above patterns in priority order, where the first
> +       // matching pattern is the one used for any one stack line.
> +       var lineColPatterns = [FFEvalLineColPatterns, MainLineColPattern];
> +
>         function getCWStack(err) {
>           if (!(err instanceof Error)) { return void 0; }
>           var stack = err.stack;
> @@ -216,15 +226,23 @@
>               if (match) {
>                 name = match[1] || '?';
>                 source = match[2] || '?';
> -               var sub = lineColPattern.exec(source);
> -               source = sub[1] || '?';
> -               if (sub[2]) {
> -                 if (sub[3]) {
> -                   span = [[+sub[2], +sub[3]]];
> -                 } else {
> -                   span = [[+sub[2]]];
> +               // Using .some here only because it gives us a way to
> escape
> +               // the loop early. We do not use the results of the .some.
> +               lineColPatterns.some(function(lineColPattern) {
> +                 var sub = lineColPattern.exec(source);
> +                 if (sub) {
> +                   source = sub[1] || '?';
> +                   if (sub[2]) {
> +                     if (sub[3]) {
> +                       span = [[+sub[2], +sub[3]]];
> +                     } else {
> +                       span = [[+sub[2]]];
> +                     }
> +                   }
> +                   return true;
>                   }
> -               }
> +                 return false;
> +               });
>                 return true;
>               }
>               return false;
> Index: src/com/google/caja/ses/startSES.js
> ===================================================================
> --- src/com/google/caja/ses/startSES.js (revision 5671)
> +++ src/com/google/caja/ses/startSES.js (working copy)
> @@ -287,7 +287,7 @@
>     * mitigate. Passing no {@code opt_mitigateOpts} performs all the
>     * default mitigations. Returns a well behaved options record.
>     *
> -   * <p>See {@code compileExpr} for documentation of the mitigation
> +   * <p>See {@code prepareExpr} for documentation of the mitigation
>     * options and their effects.
>     */
>    function resolveOptions(opt_mitigateOpts) {
> @@ -963,7 +963,7 @@
>       * any valid Program.
>       *
>       * For documentation on {@code opt_mitigateOpts} see the
> -     * corresponding parameter in compileExpr.
> +     * corresponding parameter in {@code prepareExpr}.
>       *
>       * <p>In addition, in case the module source happens to begin with
>       * a streotyped prelude of the CommonJS module system, the
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "Google Caja Discuss" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to google-caja-discuss+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>



-- 
    Cheers,
    --MarkM

[kpreid2, 2014-02-28 03:30:34]

LGTM

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

https://codereview.appspot.com/69820043/diff/1/src/com/google/caja/ses/debug....
src/com/google/caja/ses/debug.js:235: if (sub[2]) {
Perhaps a comment like "  // line number" here and on the next line, just for
skimmability.

[MarkM, 2014-02-28 03:48:32]

FF 30 Nightly stack traces are more informative for nested
evaluation. But the extra info does not correspond well to the
patterns we look for. Extend the lineColPattern into a prioritized
sequence of patterns, in the same way that framePatterns is a
prioritized sequence. Add a lineColPattern specialized for the FF30
Nightly nested eval pattern.

[MarkM, 2014-02-28 03:49:38]

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

https://codereview.appspot.com/69820043/diff/1/src/com/google/caja/ses/debug....
src/com/google/caja/ses/debug.js:235: if (sub[2]) {
On 2014/02/28 03:30:34, kpreid2 wrote:
> Perhaps a comment like "  // line number" here and on the next line, just for
> skimmability.

Done. (approximately)