Closed Bug 584240 Opened 12 years ago Closed 2 months ago

Make xpconnect/wrappers not morph slim wrappers


(Core :: JavaScript Engine, defect)

Other Branch
Not set





(Reporter: jorendorff, Unassigned)


(Blocks 1 open bug)


(Whiteboard: [compartments])

Trying to wrap a slim wrapper with any of the old-style security wrappers causes the slim wrapper to morph into a full XPCWrappedNative.

The new wrappers don't care--except the ones that have x-ray behavior. We must either:

1) make the new stuff work like the old stuff, and morph when we try to wrap a slim wrapper with any of the new-style security wrappers with x-ray behavior; or 

2) make slim wrappers never morph.
Blocks: domperf
Looking at XrayWrapper.cpp, if we want to avoid morphing:

- holder_get/holder_set
These use the XPCWrappedNative to get the XPCNativeScriptableInfo so that they can get flags and the scriptable callback. They'd need to check for slim wrappers (IS_SLIM_WRAPPER(obj)) and use |GetSlimWrapperProto(obj)->GetScriptableInfo()| to get the scriptable info from a slim wrapper and |static_cast<XPCWrappedNative *>(obj->getPrivate())->GetScriptableInfo()| from a XPCWrappedNative wrapper. The only annoying thing here is that until now we've always either had a slim wrapper or a XPCWrappedNative wrapper (security wrapped or not). The hooks thus assume that if they get a null XPCWrappedNative* the JSObject* they get is a slim wrapper. If we allow x-ray wrappers around slim wrapper they'd get a null XPCWrappedNative* and an x-ray wrapper and they would need to fish the slim wrapper out of the x-ray wrapper. We could another JSObject* argument to these hooks I guess (for the "unwrapped" wrapper), the API becomes slightly more complex but we wouldn't have to add code to all the hooks to unwrap security wrappers.

- ResolveNativeProperty
For getting the scriptable info see above.
It looks like the only other change would be to make XPCCallContext::SetName detect a slim wrapper and use |GetSlimWrapperProto(obj)->GetSet()->FindMember(name, &mMember, &mInterface)|.
Not sure how to change the |ccx.GetWrapper() != wn| check.

Just for reference, making hooks work with slim wrappers happens mostly in
Looking at AccessCheck.cpp, if we want to avoid morphing:

- IsFrameId
We could move the do_QueryWrappedNative from into XPConnect and use it here.

- AccessCheck::needsSystemOnlyWrapper
We only create XPCWrappedNative wrappers if NeedsSOW is true, so I think this is ok. Might need to null-check wn. It sure would be nice to store the flag for whether something needs a SOW in the XPCWrappedNativeProto, so we could have a SOW around a slim wrapper. Might be out of scope for this bug.
Looking at WrapperFactory.cpp, if we want to avoid morphing:

- WrapperFactory::Rewrap
Looks like all that's needed is s/IS_WN_WRAPPER_OBJECT(obj)/IS_WRAPPER_CLASS(obj->getClass())/.
blocking2.0: --- → beta5+
Whiteboard: [compartments]
We're going to do #1 to get this in Ff4.
blocking2.0: beta5+ → beta6+
No longer blocks: compartments
Blocks: compartments
Blocks: 594911
#1 is bug 594911. This bug is now about doing approach #2 (don't morph slim wrappers) which we might still want to do for perf, perhaps later.
No longer blocks: compartments, 594911
Summary: Make xpconnect/wrappers work with slim wrappers → Make xpconnect/wrappers not morph slim wrappers
Removing blocking flag.
blocking2.0: beta7+ → ---

The bug assignee didn't login in Bugzilla in the last 7 months.
:sdetar, could you have a look please?
For more information, please visit auto_nag documentation.

Assignee: jorendorff → nobody
Flags: needinfo?(sdetar)

I think some of these wrapper types no longer exist.

Closed: 2 months ago
Flags: needinfo?(sdetar)
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.