Expose JS code's column numbers and use them in more places

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(firefox43 affected)

Details

Attachments

(12 attachments, 1 obsolete attachment)

2.82 KB, patch
luke
: review+
Details | Diff | Splinter Review
2.62 KB, patch
mrbkap
: review+
Details | Diff | Splinter Review
8.42 KB, patch
baku
: review+
Details | Diff | Splinter Review
2.65 KB, patch
mrbkap
: review+
Details | Diff | Splinter Review
1.76 KB, patch
mrbkap
: review+
Details | Diff | Splinter Review
3.36 KB, patch
khuey
: review+
Details | Diff | Splinter Review
1.08 KB, patch
khuey
: review+
Details | Diff | Splinter Review
16.88 KB, patch
khuey
: review+
Details | Diff | Splinter Review
9.18 KB, patch
peterv
: review+
Details | Diff | Splinter Review
6.87 KB, patch
peterv
: review+
Details | Diff | Splinter Review
1.65 KB, patch
billm
: review+
Details | Diff | Splinter Review
1.74 KB, patch
billm
: review+
Details | Diff | Splinter Review
JS::DescribeScriptedCaller() can easily provide a column number as well as a line number. And the column number can be bubbled up in various places to where we currently use a 0 for the column number, or where we don't provide a column number but one would be useful.

(I came across this while doing some ad hoc instrumentation of JS timers, where I need the column number in order to be able to understand profiling of minified code. I.e. it's prompted by a real-world use case.)
The patch also makes the filename optional, to match the column, and to make
GetCallingLocation() more similar to JS::DescribeScriptedCaller().
Attachment #8655236 - Flags: review?(mrbkap)
Attachment #8655240 - Flags: review?(wmccloskey)
Because we currently set the source location of a nsJSScriptTimeoutHandler when
initializing from an expression, but not when initializing from a function,
which is an undesirable inconsistency. This requires plumbing through the
JSContext in a few places.
Attachment #8655245 - Flags: review?(peterv)
Attachment #8655240 - Attachment is obsolete: true
Attachment #8655240 - Flags: review?(wmccloskey)
Comment on attachment 8655235 [details] [diff] [review]
(part 3) - Use JS column numbers in WebSocket.cpp

Review of attachment 8655235 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/WebSocket.cpp
@@ +1090,5 @@
>      MOZ_ASSERT(NS_IsMainThread());
>      MOZ_ASSERT(aTopLevelWorkerPrivate && !aTopLevelWorkerPrivate->GetWindow());
>  
>      mImpl->Init(nullptr, aTopLevelWorkerPrivate->GetPrincipal(), mURL,
> +                mProtocolArray, mScriptFile, mScriptLine, mScriptColumn, mRv,

80chars
Attachment #8655235 - Flags: review?(amarchesini) → review+
Attachment #8655233 - Flags: review?(luke) → review+
Attachment #8655234 - Flags: review?(mrbkap) → review+
Attachment #8655236 - Flags: review?(mrbkap) → review+
Attachment #8655237 - Flags: review?(mrbkap) → review+
Attachment #8655248 - Flags: review?(wmccloskey) → review+
Attachment #8655247 - Flags: review?(wmccloskey) → review+
> >      mImpl->Init(nullptr, aTopLevelWorkerPrivate->GetPrincipal(), mURL,
> > +                mProtocolArray, mScriptFile, mScriptLine, mScriptColumn, mRv,
> 
> 80chars

It's 77.
I landed parts 1--10. Parts 11 and 12 are still awaiting review.
Keywords: leave-open
Attachment #8655245 - Flags: review?(peterv) → review+
Attachment #8655246 - Flags: review?(peterv) → review+
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.