We leave the nsIDOMEventTarget* versions fallible for now, but this makes the
common case a lot simpler. Note that this means that pushing a null JSContext,
a bug, is no longer handled at runtime. But I think we should just assert
against it, since there are already callers that don't check the return value.
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.
We leave the nsIDOMEventTarget* versions fallible for now, but this makes the
common case a lot simpler. Note that this means that pushing a null JSContext,
a bug, is no longer handled at runtime. But I think we should just assert
against it, since there are already callers that don't check the return value.
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.
This bug happens when we take the !useSandbox path. Basically, when the code
throws, we can end up with garbage in *aRetValue while still returning true
from EvaluateString. It looks like the convention is for these kind of eval
functions to return success even for invalid code, so lets just make sure we
check things a bit better.
This crashtest is kind of half-baked in the sense that it doesn't actually
crash without the rest of the patch. But the testcase here involves a lot of
undefined behavior (what ends up getting left in *aRetValue) during a call
to window.open (which spins the event loop, etc). I already sunk about half
an hour into trying to make it crash, so I'm just going to go with this for
now.
