Bug 1273251: Part 3 - Allow CallbackObject to contain a null callable. r?peterv
MozReview-Commit-ID: FCXVHouhG3I
--- a/dom/base/CustomElementRegistry.cpp
+++ b/dom/base/CustomElementRegistry.cpp
@@ -567,17 +567,17 @@ CustomElementRegistry::Define(const nsAS
AutoJSAPI jsapi;
if (NS_WARN_IF(!jsapi.Init(mWindow))) {
aRv.Throw(NS_ERROR_FAILURE);
return;
}
JSContext *cx = jsapi.cx();
- JS::Rooted<JSObject*> constructor(cx, aFunctionConstructor.Callable());
+ JS::Rooted<JSObject*> constructor(cx, aFunctionConstructor.CallableOrNull());
/**
* 1. If IsConstructor(constructor) is false, then throw a TypeError and abort
* these steps.
*/
// For now, all wrappers are constructable if they are callable. So we need to
// unwrap constructor to check it is really constructable.
JS::Rooted<JSObject*> constructorUnwrapped(cx, js::CheckedUnwrap(constructor));
@@ -855,9 +855,9 @@ CustomElementDefinition::CustomElementDe
mConstructor(aConstructor),
mPrototype(aPrototype),
mCallbacks(aCallbacks),
mDocOrder(aDocOrder)
{
}
} // namespace dom
-} // namespace mozilla
\ No newline at end of file
+} // namespace mozilla
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -1761,17 +1761,17 @@ GetOrCreateDOMReflectorNoWrap(JSContext*
return
GetOrCreateDOMReflectorNoWrapHelper<T>::GetOrCreate(cx, value, rval);
}
template <class T>
inline JSObject*
GetCallbackFromCallbackObject(T* aObj)
{
- return aObj->Callback();
+ return aObj->CallbackOrNull();
}
// Helper for getting the callback JSObject* of a smart ptr around a
// CallbackObject or a reference to a CallbackObject or something like
// that.
template <class T, bool isSmartPtr=IsSmartPtr<T>::value>
struct GetCallbackFromCallbackObjectHelper
{
--- a/dom/bindings/CallbackFunction.h
+++ b/dom/bindings/CallbackFunction.h
@@ -35,19 +35,19 @@ public:
// See CallbackObject for an explanation of the arguments.
explicit CallbackFunction(JS::Handle<JSObject*> aCallable,
JS::Handle<JSObject*> aAsyncStack,
nsIGlobalObject* aIncumbentGlobal)
: CallbackObject(aCallable, aAsyncStack, aIncumbentGlobal)
{
}
- JS::Handle<JSObject*> Callable() const
+ JS::Handle<JSObject*> CallableOrNull() const
{
- return Callback();
+ return CallbackOrNull();
}
JS::Handle<JSObject*> CallablePreserveColor() const
{
return CallbackPreserveColor();
}
bool HasGrayCallable() const
--- a/dom/bindings/CallbackObject.cpp
+++ b/dom/bindings/CallbackObject.cpp
@@ -85,18 +85,25 @@ CallbackObject::CallSetup::CallSetup(Cal
// Compute the caller's subject principal (if necessary) early, before we
// do anything that might perturb the relevant state.
nsIPrincipal* webIDLCallerPrincipal = nullptr;
if (aIsJSImplementedWebIDL) {
webIDLCallerPrincipal = nsContentUtils::SubjectPrincipalOrSystemIfNativeCaller();
}
+ JSObject* wrappedCallback = aCallback->CallbackPreserveColor();
+ if (!wrappedCallback) {
+ aRv.ThrowDOMException(NS_ERROR_DOM_NOT_SUPPORTED_ERR,
+ NS_LITERAL_CSTRING("Cannot execute callback from a nuked compartment."));
+ return;
+ }
+
// First, find the real underlying callback.
- JSObject* realCallback = js::UncheckedUnwrap(aCallback->CallbackPreserveColor());
+ JSObject* realCallback = js::UncheckedUnwrap(wrappedCallback);
nsIGlobalObject* globalObject = nullptr;
JSContext* cx;
{
// Bug 955660: we cannot do "proper" rooting here because we need the
// global to get a context. Everything here is simple getters that cannot
// GC, so just paper over the necessary dataflow inversion.
JS::AutoSuppressGCAnalysis nogc;
@@ -149,23 +156,24 @@ CallbackObject::CallSetup::CallSetup(Cal
"incumbent global is being torn down."));
return;
}
mAutoIncumbentScript.emplace(incumbent);
}
cx = mAutoEntryScript->cx();
- // Unmark the callable (by invoking Callback() and not the CallbackPreserveColor()
- // variant), and stick it in a Rooted before it can go gray again.
+ // Unmark the callable (by invoking CallbackOrNull() and not the
+ // CallbackPreserveColor() variant), and stick it in a Rooted before it can
+ // go gray again.
// Nothing before us in this function can trigger a CC, so it's safe to wait
// until here it do the unmark. This allows us to construct mRootedCallable
// with the cx from mAutoEntryScript, avoiding the cost of finding another
// JSContext. (Rooted<> does not care about requests or compartments.)
- mRootedCallable.emplace(cx, aCallback->Callback());
+ mRootedCallable.emplace(cx, aCallback->CallbackOrNull());
}
// JS-implemented WebIDL is always OK to run, since it runs with Chrome
// privileges anyway.
if (mIsMainThread && !aIsJSImplementedWebIDL) {
// Check that it's ok to run this callback at all.
// Make sure to use realCallback to get the global of the callback object,
// not the wrapper.
@@ -303,17 +311,17 @@ CallbackObjectHolderBase::ToXPCOMCallbac
}
// We don't init the AutoJSAPI with our callback because we don't want it
// reporting errors to its global's onerror handlers.
AutoJSAPI jsapi;
jsapi.Init();
JSContext* cx = jsapi.cx();
- JS::Rooted<JSObject*> callback(cx, aCallback->Callback());
+ JS::Rooted<JSObject*> callback(cx, aCallback->CallbackOrNull());
JSAutoCompartment ac(cx, callback);
RefPtr<nsXPCWrappedJS> wrappedJS;
nsresult rv =
nsXPCWrappedJS::GetNewOrUsed(callback, aIID, getter_AddRefs(wrappedJS));
if (NS_FAILED(rv) || !wrappedJS) {
return nullptr;
}
--- a/dom/bindings/CallbackObject.h
+++ b/dom/bindings/CallbackObject.h
@@ -74,17 +74,25 @@ public:
// for that purpose.
explicit CallbackObject(JS::Handle<JSObject*> aCallback,
JS::Handle<JSObject*> aAsyncStack,
nsIGlobalObject* aIncumbentGlobal)
{
Init(aCallback, aAsyncStack, aIncumbentGlobal);
}
- JS::Handle<JSObject*> Callback() const
+ // This is guaranteed to be non-null from the time the CallbackObject is
+ // created until JavaScript has had a chance to run. It will only return null
+ // after a JavaScript caller has called nukeSandbox on a Sandbox object, and
+ // the cycle collector has had a chance to run.
+ //
+ // This means that any native callee which receives a CallbackObject as an
+ // argument can safely rely on the callback being non-null so long as it
+ // doesn't trigger any scripts before it accesses it.
+ JS::Handle<JSObject*> CallbackOrNull() const
{
mCallback.exposeToActiveJS();
return CallbackPreserveColor();
}
JSObject* GetCreationStack() const
{
return mCreationStack;
@@ -108,17 +116,17 @@ public:
{
// Calling fromMarkedLocation() is safe because we trace our mCallback, and
// because the value of mCallback cannot change after if has been set.
return JS::Handle<JSObject*>::fromMarkedLocation(mCallback.address());
}
/*
* If the callback is known to be non-gray, then this method can be
- * used instead of Callback() to avoid the overhead of
+ * used instead of CallbackOrNull() to avoid the overhead of
* ExposeObjectToActiveJS().
*/
JS::Handle<JSObject*> CallbackKnownNotGray() const
{
MOZ_ASSERT(!JS::ObjectIsMarkedGray(mCallback));
return CallbackPreserveColor();
}
@@ -155,20 +163,24 @@ protected:
explicit CallbackObject(CallbackObject* aCallbackObject)
{
Init(aCallbackObject->mCallback, aCallbackObject->mCreationStack,
aCallbackObject->mIncumbentGlobal);
}
bool operator==(const CallbackObject& aOther) const
{
- JSObject* thisObj =
- js::UncheckedUnwrap(CallbackPreserveColor());
- JSObject* otherObj =
- js::UncheckedUnwrap(aOther.CallbackPreserveColor());
+ JSObject* wrappedThis = CallbackPreserveColor();
+ JSObject* wrappedOther = aOther.CallbackPreserveColor();
+ if (!wrappedThis || !wrappedOther) {
+ return this == &aOther;
+ }
+
+ JSObject* thisObj = js::UncheckedUnwrap(wrappedThis);
+ JSObject* otherObj = js::UncheckedUnwrap(wrappedOther);
return thisObj == otherObj;
}
private:
inline void InitNoHold(JSObject* aCallback, JSObject* aCreationStack,
nsIGlobalObject* aIncumbentGlobal)
{
MOZ_ASSERT(aCallback && !mCallback);
@@ -562,20 +574,20 @@ public:
// But nullptr can't use the above template, because it doesn't know which S
// to select. So we need a special overload for nullptr.
void operator=(decltype(nullptr) arg)
{
this->get().operator=(arg);
}
- // Codegen relies on being able to do Callback() on us.
- JS::Handle<JSObject*> Callback() const
+ // Codegen relies on being able to do CallbackOrNull() on us.
+ JS::Handle<JSObject*> CallbackOrNull() const
{
- return this->get()->Callback();
+ return this->get()->CallbackOrNull();
}
~RootedCallback()
{
// Ensure that our callback starts holding on to its own JS objects as
// needed. Having to null-check here when T is OwningNonNull is a bit
// silly, but it's simpler than creating two separate RootedCallback
// instantiations for OwningNonNull and RefPtr.
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -4152,17 +4152,18 @@ class CastableObjectUnwrapper():
self.substitution["codeOnFailure"] = fill(
"""
// Be careful to not wrap random DOM objects here, even if
// they're wrapped in opaque security wrappers for some reason.
// XXXbz Wish we could check for a JS-implemented object
// that already has a content reflection...
if (!IsDOMObject(js::UncheckedUnwrap(${source}))) {
nsCOMPtr<nsIGlobalObject> contentGlobal;
- if (!GetContentGlobalForJSImplementedObject(cx, Callback(), getter_AddRefs(contentGlobal))) {
+ JS::Handle<JSObject*> callback = CallbackOrNull();
+ if (!(callback && GetContentGlobalForJSImplementedObject(cx, callback, getter_AddRefs(contentGlobal)))) {
$*{exceptionCode}
}
JS::Rooted<JSObject*> jsImplSourceObj(cx, ${source});
${target} = new ${type}(jsImplSourceObj, contentGlobal);
} else {
$*{codeOnFailure}
}
""",
@@ -15013,21 +15014,21 @@ class CGJSImplClass(CGBindingImplClass):
return fill(
"""
JS::Rooted<JSObject*> obj(aCx, ${name}Binding::Wrap(aCx, this, aGivenProto));
if (!obj) {
return nullptr;
}
// Now define it on our chrome object
- JSAutoCompartment ac(aCx, mImpl->Callback());
+ JSAutoCompartment ac(aCx, mImpl->CallbackOrNull());
if (!JS_WrapObject(aCx, &obj)) {
return nullptr;
}
- if (!JS_DefineProperty(aCx, mImpl->Callback(), "__DOM_IMPL__", obj, 0)) {
+ if (!JS_DefineProperty(aCx, mImpl->CallbackOrNull(), "__DOM_IMPL__", obj, 0)) {
return nullptr;
}
return obj;
""",
name=self.descriptor.name)
def getGetParentObjectReturnType(self):
return "nsISupports*"
--- a/dom/bindings/ToJSValue.h
+++ b/dom/bindings/ToJSValue.h
@@ -127,17 +127,17 @@ ToJSValue(JSContext* aCx,
MOZ_MUST_USE inline bool
ToJSValue(JSContext* aCx,
CallbackObject& aArgument,
JS::MutableHandle<JS::Value> aValue)
{
// Make sure we're called in a compartment
MOZ_ASSERT(JS::CurrentGlobalOrNull(aCx));
- aValue.setObject(*aArgument.Callback());
+ aValue.setObjectOrNull(aArgument.CallbackOrNull());
return MaybeWrapValue(aCx, aValue);
}
// Accept objects that inherit from nsWrapperCache (e.g. most
// DOM objects).
template <class T>
MOZ_MUST_USE
--- a/dom/console/Console.cpp
+++ b/dom/console/Console.cpp
@@ -2277,21 +2277,26 @@ Console::NotifyHandler(JSContext* aCx, c
MOZ_ASSERT(aCallData);
if (!mConsoleEventNotifier) {
return;
}
JS::Rooted<JS::Value> value(aCx);
+ JS::Rooted<JSObject*> callable(aCx, mConsoleEventNotifier->CallableOrNull());
+ if (NS_WARN_IF(!callable)) {
+ return;
+ }
+
// aCx and aArguments are in the same compartment because this method is
// called directly when a Console.something() runs.
// mConsoleEventNotifier->Callable() is the scope where value will be sent to.
if (NS_WARN_IF(!PopulateConsoleNotificationInTheTargetScope(aCx, aArguments,
- mConsoleEventNotifier->Callable(),
+ callable,
&value,
aCallData))) {
return;
}
JS::Rooted<JS::Value> ignored(aCx);
mConsoleEventNotifier->Call(value, &ignored);
}
--- a/dom/events/DOMEventTargetHelper.cpp
+++ b/dom/events/DOMEventTargetHelper.cpp
@@ -317,17 +317,17 @@ DOMEventTargetHelper::SetEventHandler(ns
void
DOMEventTargetHelper::GetEventHandler(nsIAtom* aType,
JSContext* aCx,
JS::Value* aValue)
{
EventHandlerNonNull* handler = GetEventHandler(aType, EmptyString());
if (handler) {
- *aValue = JS::ObjectValue(*handler->Callable());
+ *aValue = JS::ObjectOrNullValue(handler->CallableOrNull());
} else {
*aValue = JS::NullValue();
}
}
nsresult
DOMEventTargetHelper::PreHandleEvent(EventChainPreVisitor& aVisitor)
{
--- a/dom/events/EventListenerService.cpp
+++ b/dom/events/EventListenerService.cpp
@@ -133,17 +133,17 @@ EventListenerInfo::GetJSVal(JSContext* a
aAc.emplace(aCx, object);
aJSVal.setObject(*object);
return true;
}
nsCOMPtr<JSEventHandler> jsHandler = do_QueryInterface(mListener);
if (jsHandler && jsHandler->GetTypedEventHandler().HasEventHandler()) {
JS::Handle<JSObject*> handler =
- jsHandler->GetTypedEventHandler().Ptr()->Callable();
+ jsHandler->GetTypedEventHandler().Ptr()->CallableOrNull();
if (handler) {
aAc.emplace(aCx, handler);
aJSVal.setObject(*handler);
return true;
}
}
return false;
}
--- a/dom/promise/Promise.cpp
+++ b/dom/promise/Promise.cpp
@@ -620,26 +620,26 @@ Promise::Then(JSContext* aCx,
JS::Rooted<JSObject*> promise(aCx, PromiseObj());
if (!JS_WrapObject(aCx, &promise)) {
aRv.NoteJSContextException(aCx);
return;
}
JS::Rooted<JSObject*> resolveCallback(aCx);
if (aResolveCallback) {
- resolveCallback = aResolveCallback->Callback();
+ resolveCallback = aResolveCallback->CallbackOrNull();
if (!JS_WrapObject(aCx, &resolveCallback)) {
aRv.NoteJSContextException(aCx);
return;
}
}
JS::Rooted<JSObject*> rejectCallback(aCx);
if (aRejectCallback) {
- rejectCallback = aRejectCallback->Callback();
+ rejectCallback = aRejectCallback->CallbackOrNull();
if (!JS_WrapObject(aCx, &rejectCallback)) {
aRv.NoteJSContextException(aCx);
return;
}
}
JS::Rooted<JSObject*> retval(aCx);
retval = JS::CallOriginalPromiseThen(aCx, promise, resolveCallback,
--- a/dom/promise/PromiseCallback.cpp
+++ b/dom/promise/PromiseCallback.cpp
@@ -384,18 +384,20 @@ WrapperPromiseCallback::Call(JSContext*
}
// XXXbz shouldn't this check be over in ResolveInternal anyway?
if (valueObj == nextPromiseObj) {
const char* fileName = nullptr;
uint32_t lineNumber = 0;
// Try to get some information about the callback to report a sane error,
// but don't try too hard (only deals with scripted functions).
- JS::Rooted<JSObject*> unwrapped(aCx,
- js::CheckedUnwrap(mCallback->Callback()));
+ JS::Rooted<JSObject*> unwrapped(aCx, mCallback->CallbackOrNull());
+ if (unwrapped) {
+ js::CheckedUnwrap(&unwrapped);
+ }
if (unwrapped) {
JSAutoCompartment ac(aCx, unwrapped);
if (JS_ObjectIsFunction(aCx, unwrapped)) {
JS::Rooted<JS::Value> asValue(aCx, JS::ObjectValue(*unwrapped));
JS::Rooted<JSFunction*> func(aCx, JS_ValueToFunction(aCx, asValue));
MOZ_ASSERT(func);
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -1283,17 +1283,17 @@ private:
{
// Silence bad assertions.
}
virtual bool
WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
{
JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
- JS::Rooted<JS::Value> callable(aCx, JS::ObjectValue(*mHandler->Callable()));
+ JS::Rooted<JS::Value> callable(aCx, JS::ObjectOrNullValue(mHandler->CallableOrNull()));
JS::HandleValueArray args = JS::HandleValueArray::empty();
JS::Rooted<JS::Value> rval(aCx);
if (!JS_CallFunctionValue(aCx, global, callable, args, &rval)) {
// Just return false; WorkerRunnable::Run will report the exception.
return false;
}
return true;
--- a/xpcom/base/CycleCollectedJSContext.cpp
+++ b/xpcom/base/CycleCollectedJSContext.cpp
@@ -933,17 +933,18 @@ public:
virtual ~PromiseJobRunnable()
{
}
protected:
NS_IMETHOD
Run() override
{
- nsIGlobalObject* global = xpc::NativeGlobal(mCallback->CallbackPreserveColor());
+ JSObject* callback = mCallback->CallbackPreserveColor();
+ nsIGlobalObject* global = callback ? xpc::NativeGlobal(callback) : nullptr;
if (global && !global->IsDying()) {
mCallback->Call("promise callback");
}
return NS_OK;
}
private:
RefPtr<PromiseJobCallback> mCallback;