can we remove SetSafeJSContext and its single caller in nsAppShellService?

RESOLVED FIXED in mozilla8

Status

()

Core
XPConnect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: luke, Assigned: luke)

Tracking

unspecified
mozilla8
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
Created attachment 547278 [details] [diff] [review]
rm SetSafeJSContext

Currently, there is this really strange encapsulation-breaking method whereby nsAppShellService sets the safe JS context for the main thread to the hidden window's context.  mrbkap said this is stuff is from long ago and I should probably ask bz.  The attached patch (which just removes it) is green on try.

SetSafeJSContext isn't thing in the world, but it certainly confounds reasoning about the context stack and safe JS contexts.
Attachment #547278 - Flags: review?(bzbarsky)
Hrm...  I thought we used to have code that in fact did depend on the safe context being the hidden window (or more importantly having an associated nsIScriptContext and the like), but maybe that's gone now.

With this change, is the safe context just some synthetic jscontext?

ccing jst too, because he may know more about this than I do (which would not be hard!).
(Assignee)

Comment 2

6 years ago
(In reply to comment #1)
> With this change, is the safe context just some synthetic jscontext?

Yes, created by XPCJSContextStack::GetSafeJSContext.  It has null principals and some faked-up global object.
Comment on attachment 547278 [details] [diff] [review]
rm SetSafeJSContext

Well, let's give this a shot.
Attachment #547278 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 4

6 years ago
I'll have wait a bit, one of the other uses of SetSafeJSContext is dom/src/threads/nsDOMThreadService.cpp which seems to have been given reprieve for the time being.
(Assignee)

Comment 5

6 years ago
Created attachment 548956 [details] [diff] [review]
fix workers

Incidentally, this change did break new web workers.  Easy fix, though.  Over the shoulder r+ from Ben.
Attachment #548956 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/7487eb60b55c
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.