981198 - Move JSErrorReporter to the JSRuntime

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bobby Holley (:bholley)]

In order to take the JS engine out of the loop on exception reporting (bug 981187), we need to make it such that the error reporting logic itself is not dependent on which JSContext we use.

At present, we only really have multiple JSContexts on the main thread. So we can just unify them, and use the JSContextStack/ScriptSettingsStack to retrieve any state we need. In general, I'm guessing we just need the entry global, which we can QI to an nsPIDOMWindow. This effectively means unifying xpc::SystemErrorReporter and NS_ScriptErrorReporter, and dealing with any stragglers.

Note that there are some situations where we don't really have a reason to push an entry global, but currently push a JSContext to properly report errors (during event handler compilation, for example). In bug 978042, Bob added a typedef called AutoPushJSContextForErrorReporting, which will flag the places where we determine this to be the case. Once that's all sorted out, we can figure out if we need anything special here.

Anyway, so long as we don't mind the unified error reporter querying the JSContext stack to look for a DOM window for the time being, I think we can probably do this right now.

Comment 1

[Bobby Holley (:bholley)]

Attachment: Part 1 - Internally move the JSErrorReporter to the JSRuntime. v1

Comment 2

[Bobby Holley (:bholley)]

Attachment: Part 2 - Make JS_{Get,Set}ErrorReporter take a JSRuntime. v1

Comment 3

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=e2c5764d308f

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8486497 [details] [diff] [review]
Part 2 - Make JS_{Get,Set}ErrorReporter take a JSRuntime. v1

r=me on the dom/ipc/xpconnect/netwerk changes.  I didn't really look at the jseng bits.

Comment 5

[Terrence Cole [:terrence]]

Comment on attachment 8486496 [details] [diff] [review]
Part 1 - Internally move the JSErrorReporter to the JSRuntime. v1

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

Thanks, Bobby! I was actually planning to write this patch myself today.

Comment 6

[Terrence Cole [:terrence]]

Comment on attachment 8486497 [details] [diff] [review]
Part 2 - Make JS_{Get,Set}ErrorReporter take a JSRuntime. v1

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

\o/

Comment 7

[Bobby Holley (:bholley)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/3d78fa27054f
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/0eaa239b3bfd

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3d78fa27054f
https://hg.mozilla.org/mozilla-central/rev/0eaa239b3bfd