Move JSErrorReporter to the JSRuntime

RESOLVED FIXED in mozilla35

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

unspecified
mozilla35
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

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.
Depends on: 951991
Depends on: 1062077
Depends on: 1064550
Attachment #8486497 - Flags: review?(terrence)
Attachment #8486497 - Flags: review?(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.
Attachment #8486497 - Flags: review?(bzbarsky) → review+
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.
Attachment #8486496 - Flags: review?(terrence) → review+
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/
Attachment #8486497 - Flags: review?(terrence) → review+
https://hg.mozilla.org/mozilla-central/rev/3d78fa27054f
https://hg.mozilla.org/mozilla-central/rev/0eaa239b3bfd
Assignee: nobody → bobbyholley
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.