Note: There are a few cases of duplicates in user autocompletion which are being worked on.

can we remove SetSafeJSContext and its single caller in nsAppShellService?

RESOLVED FIXED in mozilla8



6 years ago
6 years ago


(Reporter: luke, Assigned: luke)



Firefox Tracking Flags

(Not tracked)



(2 attachments)



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)

Comment 1

6 years ago
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!).

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 3

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

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

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.

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