586192 - Fix up XrayWrapper

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

At a minimum, XrayWrappers can only wrap Holder objects; we don't currently create Holder objects anywhere. WrapperFactory must be changed to create them.

However I am worried about XrayWrapper<JSCrossCompartmentWrapper>. It seems like that combination wouldn't actually provide x-ray behavior for any of the methods that the XrayWrapper template doesn't explicitly override, like get() and set().

Also the fact that the two are merged in a single handler means the wrappedObject must be both a Holder and in another compartment. But XrayWrapper doesn't treat the holder as though it's in another compartment.

Maybe swapping the two would fix it: JSCrossCompartmentWrapper<XrayWrapper>. With that, the JSCrossCompartmentWrapper methods are called first. They handle rewrapping and then delegate to XrayWrapper methods. By the time XrayWrapper is called, all the arguments are same-compartment.

This blocks "throwing the switch".

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: WIP 1 - just refactor JSCrossCompartmentWrapper into a template

The template has to be instantiated from XPConnect (!) so all the methods go into a new jswrapperinlines.h header.

Comment 2

[Andreas Gal :gal]

Fixed in Blake's patch queue.

Comment 3

[Paul Wright]

(In reply to comment #2)
> Fixed in Blake's patch queue.

So this can be marked as FIXED, yes?

Comment 4

[Andreas Gal :gal]

Yup. Got obsoleted.