The goal here is to get rid of this crap entirely, and make nsCxPusher always
push. But that's a scary change, so we do it in chunks. This patch, in particular,
should have zero behavioral change. This means preserving some very wrong behavior.
For instance, currently SafeAutoJSContext never pushes a damn thing, because the
safe JSContext doesn't have an associated nsIScriptContext. We preserve this
behavior, and in fact convert various similarly-buggy consumers to
SafeAutoJSContext, so that we can hoist the behavioral change into a subsequent
patch.
InstallXBLField knows how to handle cross-compartment |callee|. However,
The current value will always be wrapped. We need to unwrap said wrappers,
otherwise we'll end up with objects that aren't functions.
Right now, DoInitJSClass only returns non-null if the class object is new
(and thus needs to have members installed on it). The code then goes and
unconditionally tries to install the members, null-checking and aborting
each time.
Instead, let's use an explicit boolean and one early return. This lets us
get a reference to our JSClass no matter what, which we need.
This confused me, because fields are, in fact, exposed via the prototype
via the complicated two-step solution defined in InstallField. As it turns
out, CompilePrototypeMembers throws if mClassObject ends up null.
We use XPCNativeWrapper.unwrap rather than .wrappedJSObject so that the tests
are agnostic to whether there's an Xray wrapper or not.
I converted test_tree_column_reorder.xul into a chrome test because it does
all sorts of crazy introspection on the binding, and it really should be
a chrome test anyway.
There are a few changes we make here:
1 - Having callers pass JS::CompileOptions directly.
2 - Removing aUndefined, which makes no sense and is unused by consumers.
3 - Making aScopeObject and aRetValue non-optional, via references.
3 - Adding an argument to optionally coerce the return value to a string.
Coercing jsvals to strings is the reason we currently have two nearly-identical
functions, EvaluateString and EvaluateStringWithValue, since the coercion can
trigger arbitrary script and thus needs to be bracketed by all the junk that
nsJSContext does. However, if callers can be guaranteed that the return value
will be a bonafide string, then they can easily extract the string themselves
if they so desire. This will allow us to combine the two functions.
Note that the three consumers were all XBL, and were all passing aShared = true,
which had the effect of passing null for the target object. So we actually want
to pass JS::NullPtr() (the HandleObject version of nullptr) instead of
aClassObject in order to maintain the old behavior (whatever that is).
This doesn't switch all of the users yet, but is a step in the right
direction.
--HG--
extra : rebase_source : 91b4fef3f67586179c119208d000cf7629e04963
Bobby Holley
Josh Matthews
Mounir Lamouri
Olli Pettay
Steve Fink
Trevor Saunders
Boris Zbarsky
Cameron McCormack
Peter Van der Beken
Bill McCloskey
Masayuki Nakano
Jeff Walden
Jeff Muizelaar
Ed Morley
Mats Palmgren