Closed Bug 1087851 Opened 5 years ago Closed 5 years ago

Get rid of the use of xpc::UnprivilegedJunkScope() in the worker XHR Proxy::HandleEvent

Categories

(Core :: DOM: Core & HTML, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(1 file)

I promised Bobby to do this as a followup for bug 1047483.

The basic issue is that we need a reflector for mXHR, and in fact we want to end up in its reflector's compartment.  But WrapNewBindingObject currently both produces the reflector _and_ wraps it into the caller compartment, and in particular will fail miserably if the caller is not in a compartment.

Need to think about how to refactor things here so we don't have to enter a random dummy compartment, basically.
Flags: needinfo?(bzbarsky)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Flags: needinfo?(bzbarsky)
Comment on attachment 8528096 [details] [diff] [review]
Add a GetOrCreateNewBindingObjectReflector API that can be used to do what WrapNewBindingObject does but without wrapping into the caller compartment

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

Thanks for doing this.

::: dom/bindings/BindingUtils.h
@@ +952,5 @@
> +// Like WrapNewBindingObject but doesn't wrap into the context compartment, and
> +// hence does not actually require cx to be in a compartment.
> +template <class T>
> +MOZ_ALWAYS_INLINE bool
> +GetOrCreateNewBindingObjectReflector(JSContext* cx, T* value,

I think it's important for the names of these two functions to be similar.

I've always been opposed to using the term 'wrap' to mean 'reflect', and cases like this are precisely why (WrapNewBindingObjectNoWrap? WTF?). Also, the phrase "NewBinding" is kind of confusing at this point. So my first choice would be to rename both of these functions to:

GetOrCreateDOMReflector and GetOrCreateDOMReflectorNoWrap
(I could also do s/DOM/WebIDL/, or something else).

It looks like a simple regexp could fix up the 47 existing calls to WrapNewBindingObject - I'd be fine with this happening in a followup patch.

If that's really objectionable for some reason, I could also do:

WrapNewBindingObject and WrapNewBindingObjectWithoutEntering

But I would much prefer the former.
Attachment #8528096 - Flags: review?(bobbyholley) → review+
Peter, any objections to GetOrCreateDOMReflector?  I'm ok with it myself.
Flags: needinfo?(peterv)
Fine by me.
Flags: needinfo?(peterv)
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.