Open Bug 584240 Opened 11 years ago Updated 10 years ago
Make xpconnect/wrappers not morph slim wrappers
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.
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 http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/src/xpcwrappednativejsops.cpp#991
Looking at AccessCheck.cpp, if we want to avoid morphing: - IsFrameId We could move the do_QueryWrappedNative from http://mxr.mozilla.org/mozilla-central/source/dom/base/nsDOMClassInfo.h#371 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())/.
We're going to do #1 to get this in Ff4.
#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.
Removing blocking flag.
blocking2.0: beta7+ → ---
You need to log in before you can comment on or make changes to this bug.