1069490 - Add column number to ConsoleEvent, ConsoleStackEntry, nsIStackFrame

Status: RESOLVED FIXED

Description

[Eric Rahm [:erahm] (ni? for phab reviews)]

In bug 1060161 we started printing out stack traces with console messages on b2g devices. It was noted that having a column number in addition to a line number would be useful (particularly for minified code).

My proposal is:
   #1 - add an |unsigned long columnNumber| to ConsoleEvent [1] and
        ConsoleStackEntry [2]
   #2 - add a |readonly attribute int32_t columnNumber| to nsIStackFrame [3]
   #3 - |StackFrameToStackEntry| [4] would set ConsoleStackEntry's column number
   #4 - add a |GetColumnNumber| to JSStackFrame [5] that queries "column" value
        of the JS stack object

[1] http://hg.mozilla.org/mozilla-central/annotate/426497473505/dom/webidl/Console.webidl#l35
[2] http://hg.mozilla.org/mozilla-central/annotate/426497473505/dom/webidl/Console.webidl#l67
[3] http://hg.mozilla.org/mozilla-central/annotate/426497473505/xpcom/base/nsIException.idl#l14
[4] http://hg.mozilla.org/mozilla-central/annotate/426497473505/dom/base/Console.cpp#l752
[5] http://hg.mozilla.org/mozilla-central/annotate/426497473505/dom/bindings/Exceptions.cpp#l264

Comment 1

[Eric Rahm [:erahm] (ni? for phab reviews)]

Created attachment 8491925 [details] [diff] [review]
Part 1: Add columnNumber to ConsoleEvent and ConsoleStackEntry

Comment 2

[Eric Rahm [:erahm] (ni? for phab reviews)]

Created attachment 8491928 [details] [diff] [review]
Part 2: Add columnNumber to nsIStackFrame and set in JSStackFrame

Comment 3

[Eric Rahm [:erahm] (ni? for phab reviews)]

Created attachment 8491929 [details] [diff] [review]
Part 3: Set columnNumber when processing console messages

Comment 4

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Comment on attachment 8491925 [details] [diff] [review]
Part 1: Add columnNumber to ConsoleEvent and ConsoleStackEntry

r=me

Comment 5

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Comment on attachment 8491928 [details] [diff] [review]
Part 2: Add columnNumber to nsIStackFrame and set in JSStackFrame

r=me

Comment 6

[Boris Zbarsky [:bzbarsky, bz on IRC]]

Comment on attachment 8491929 [details] [diff] [review]
Part 3: Set columnNumber when processing console messages

r=me.  Thank you for breaking this up into nice easy to digest chunks!

Comment 7

[Eric Rahm [:erahm] (ni? for phab reviews)]

(In reply to Boris Zbarsky [:bz] from comment #6)
> Comment on attachment 8491929 [details] [diff] [review]
> Part 3: Set columnNumber when processing console messages
> 
> r=me.  Thank you for breaking this up into nice easy to digest chunks!

Thanks for the quick reviews! Try is running, I expect that I'll need to fix a few tests when the results come in: https://tbpl.mozilla.org/?tree=Try&rev=d965bae7a732

Comment 8

[Eric Rahm [:erahm] (ni? for phab reviews)]

Created attachment 8492489 [details] [diff] [review]
Part 4: Update tests

Rob, can you take a look at these test updates that handle the new |columnNumber| field on stack traces?

Comment 9

[Eric Rahm [:erahm] (ni? for phab reviews)]

Rerunning try against bc and dt: https://tbpl.mozilla.org/?tree=Try&rev=d719537dfe85

Comment 10

[J. Ryan Stinnett [:jryans]]

Comment on attachment 8492489 [details] [diff] [review]
Part 4: Update tests

Reassigning since Rob is out.

Comment 11

[Eric Rahm [:erahm] (ni? for phab reviews)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/3dae455e39d2
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/b0dd65a5d40b
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/55363f62f7ad
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/31ff3ab5c143

Comment 12

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/3dae455e39d2
https://hg.mozilla.org/mozilla-central/rev/b0dd65a5d40b
https://hg.mozilla.org/mozilla-central/rev/55363f62f7ad
https://hg.mozilla.org/mozilla-central/rev/31ff3ab5c143