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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

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.)

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Expose the column number in JS::DescribeScriptedCaller()

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Use JS column numbers in ReportWrapperDenial()

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Use JS column numbers in WebSocket.cpp

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 4) - Expose the column number in nsJSUtils::GetCallingLocation()

The patch also makes the filename optional, to match the column, and to make
GetCallingLocation() more similar to JS::DescribeScriptedCaller().

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: (part 5) - Use JS column numbers in XPCNativeInterface::NewInstance()

Comment 6

[Nicholas Nethercote [inactive]]

Attachment: (part 7) - Use JS column numbers in GetParamsForMessage()

Comment 7

[Nicholas Nethercote [inactive]]

Attachment: (part 8) - Use JS column numbers in EventSource

Comment 8

[Nicholas Nethercote [inactive]]

Attachment: (part 9) - Use JS column numbers in nsContentUtils::ReportToConsoleNonLocalized

Comment 9

[Nicholas Nethercote [inactive]]

Attachment: (part 10) - Use JS column numbers in indexedDB

Comment 10

[Nicholas Nethercote [inactive]]

Attachment: (part 11) - Set source location when initializing an nsJSScriptTimeoutHandler from a Function

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.

Comment 11

[Nicholas Nethercote [inactive]]

Attachment: (part 12) - Use JS column numbers in nsJSTimeoutHandler

Comment 12

[Nicholas Nethercote [inactive]]

Attachment: (part 7) - Use JS column numbers in GetParamsForMessage()

Comment 13

[Nicholas Nethercote [inactive]]

Attachment: (part 6) - Use JS column numbers in ContentParent::OnChannelError()

Comment 14

[Andrea Marchesini [:baku]]

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

Comment 15

[Nicholas Nethercote [inactive]]

> >      mImpl->Init(nullptr, aTopLevelWorkerPrivate->GetPrincipal(), mURL,
> > +                mProtocolArray, mScriptFile, mScriptLine, mScriptColumn, mRv,
> 
> 80chars

It's 77.

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aa569d3c9c0c
https://hg.mozilla.org/integration/mozilla-inbound/rev/87b184107bb0
https://hg.mozilla.org/integration/mozilla-inbound/rev/1842e6a850a5
https://hg.mozilla.org/integration/mozilla-inbound/rev/e69ce64116c9
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b1b725f1f1d
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab12904f4689
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b6d99eaee3f
https://hg.mozilla.org/integration/mozilla-inbound/rev/4011cb8cbcae
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff59d437f225
https://hg.mozilla.org/integration/mozilla-inbound/rev/82aade52497a

Comment 17

[Nicholas Nethercote [inactive]]

I landed parts 1--10. Parts 11 and 12 are still awaiting review.

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/aa569d3c9c0c
https://hg.mozilla.org/mozilla-central/rev/87b184107bb0
https://hg.mozilla.org/mozilla-central/rev/1842e6a850a5
https://hg.mozilla.org/mozilla-central/rev/e69ce64116c9
https://hg.mozilla.org/mozilla-central/rev/7b1b725f1f1d
https://hg.mozilla.org/mozilla-central/rev/ab12904f4689
https://hg.mozilla.org/mozilla-central/rev/0b6d99eaee3f
https://hg.mozilla.org/mozilla-central/rev/4011cb8cbcae
https://hg.mozilla.org/mozilla-central/rev/ff59d437f225
https://hg.mozilla.org/mozilla-central/rev/82aade52497a

Comment 19

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/09a4daca3727
https://hg.mozilla.org/integration/mozilla-inbound/rev/b4883fe60278

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/09a4daca3727
https://hg.mozilla.org/mozilla-central/rev/b4883fe60278