From f3f44cfa74223f6da282396b458d11bae443bc03 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 24 Apr 2013 14:59:15 -0400 Subject: [PATCH] Bug 862629 part 1. Stop playing compartment games with WebIDL callbacks and just use the given object as-is. r=peterv --- content/base/src/nsINode.cpp | 11 +----- content/events/src/nsDOMEventTargetHelper.cpp | 11 +----- content/events/src/nsEventListenerManager.cpp | 27 ++----------- content/html/content/src/HTMLBodyElement.cpp | 11 +----- .../html/content/src/HTMLFrameSetElement.cpp | 11 +----- content/xbl/src/nsXBLPrototypeHandler.cpp | 6 +-- dom/base/nsGlobalWindow.cpp | 31 ++------------- dom/base/nsJSTimeoutHandler.cpp | 7 +--- dom/bindings/CallbackFunction.h | 14 ++----- dom/bindings/CallbackInterface.h | 18 --------- dom/bindings/CallbackObject.h | 38 +------------------ dom/bindings/Codegen.py | 31 +++------------ 12 files changed, 22 insertions(+), 194 deletions(-) diff --git a/content/base/src/nsINode.cpp b/content/base/src/nsINode.cpp index da34dba9d701..802f7a04ed0e 100644 --- a/content/base/src/nsINode.cpp +++ b/content/base/src/nsINode.cpp @@ -2102,20 +2102,11 @@ nsINode::SizeOfExcludingThis(nsMallocSizeOfFun aMallocSizeOf) const return NS_OK; \ } \ NS_IMETHODIMP nsINode::SetOn##name_(JSContext *cx, const JS::Value &v) { \ - JSObject *obj = GetWrapper(); \ - if (!obj) { \ - /* Just silently do nothing */ \ - return NS_OK; \ - } \ nsRefPtr handler; \ JSObject *callable; \ if (v.isObject() && \ JS_ObjectIsCallable(cx, callable = &v.toObject())) { \ - bool ok; \ - handler = new EventHandlerNonNull(cx, obj, callable, &ok); \ - if (!ok) { \ - return NS_ERROR_OUT_OF_MEMORY; \ - } \ + handler = new EventHandlerNonNull(callable); \ } \ ErrorResult rv; \ SetOn##name_(handler, rv); \ diff --git a/content/events/src/nsDOMEventTargetHelper.cpp b/content/events/src/nsDOMEventTargetHelper.cpp index af48a5a347d0..be677fa65ce2 100644 --- a/content/events/src/nsDOMEventTargetHelper.cpp +++ b/content/events/src/nsDOMEventTargetHelper.cpp @@ -283,20 +283,11 @@ nsDOMEventTargetHelper::SetEventHandler(nsIAtom* aType, JSContext* aCx, const JS::Value& aValue) { - JSObject* obj = GetWrapper(); - if (!obj) { - return NS_OK; - } - nsRefPtr handler; JSObject* callable; if (aValue.isObject() && JS_ObjectIsCallable(aCx, callable = &aValue.toObject())) { - bool ok; - handler = new EventHandlerNonNull(aCx, obj, callable, &ok); - if (!ok) { - return NS_ERROR_OUT_OF_MEMORY; - } + handler = new EventHandlerNonNull(callable); } ErrorResult rv; SetEventHandler(aType, handler, rv); diff --git a/content/events/src/nsEventListenerManager.cpp b/content/events/src/nsEventListenerManager.cpp index 6c87a7797c04..b7d08dabf762 100644 --- a/content/events/src/nsEventListenerManager.cpp +++ b/content/events/src/nsEventListenerManager.cpp @@ -874,37 +874,16 @@ nsEventListenerManager::CompileEventHandlerInternal(nsListenerStruct *aListenerS context->BindCompiledEventHandler(mTarget, listener->GetEventScope(), handler, &boundHandler); if (listener->EventName() == nsGkAtoms::onerror && win) { - bool ok; - JSAutoRequest ar(cx); nsRefPtr handlerCallback = - new OnErrorEventHandlerNonNull(cx, listener->GetEventScope(), - boundHandler, &ok); - if (!ok) { - // JS_WrapObject failed, which means OOM allocating the JSObject. - return NS_ERROR_OUT_OF_MEMORY; - } + new OnErrorEventHandlerNonNull(boundHandler); listener->SetHandler(handlerCallback); } else if (listener->EventName() == nsGkAtoms::onbeforeunload && win) { - bool ok; - JSAutoRequest ar(cx); nsRefPtr handlerCallback = - new BeforeUnloadEventHandlerNonNull(cx, listener->GetEventScope(), - boundHandler, &ok); - if (!ok) { - // JS_WrapObject failed, which means OOM allocating the JSObject. - return NS_ERROR_OUT_OF_MEMORY; - } + new BeforeUnloadEventHandlerNonNull(boundHandler); listener->SetHandler(handlerCallback); } else { - bool ok; - JSAutoRequest ar(cx); nsRefPtr handlerCallback = - new EventHandlerNonNull(cx, listener->GetEventScope(), - boundHandler, &ok); - if (!ok) { - // JS_WrapObject failed, which means OOM allocating the JSObject. - return NS_ERROR_OUT_OF_MEMORY; - } + new EventHandlerNonNull(boundHandler); listener->SetHandler(handlerCallback); } } diff --git a/content/html/content/src/HTMLBodyElement.cpp b/content/html/content/src/HTMLBodyElement.cpp index d6506f6bcd89..6f691ae5bd94 100644 --- a/content/html/content/src/HTMLBodyElement.cpp +++ b/content/html/content/src/HTMLBodyElement.cpp @@ -506,20 +506,11 @@ HTMLBodyElement::IsEventAttributeName(nsIAtom *aName) NS_IMETHODIMP \ HTMLBodyElement::SetOn##name_(JSContext *cx, const JS::Value &v) \ { \ - JSObject *obj = GetWrapper(); \ - if (!obj) { \ - /* Just silently do nothing */ \ - return NS_OK; \ - } \ nsRefPtr handler; \ JSObject *callable; \ if (v.isObject() && \ JS_ObjectIsCallable(cx, callable = &v.toObject())) { \ - bool ok; \ - handler = new type_(cx, obj, callable, &ok); \ - if (!ok) { \ - return NS_ERROR_OUT_OF_MEMORY; \ - } \ + handler = new type_(callable); \ } \ ErrorResult rv; \ forwardto_::SetOn##name_(handler, rv); \ diff --git a/content/html/content/src/HTMLFrameSetElement.cpp b/content/html/content/src/HTMLFrameSetElement.cpp index 87c48b425543..e68f34210ded 100644 --- a/content/html/content/src/HTMLFrameSetElement.cpp +++ b/content/html/content/src/HTMLFrameSetElement.cpp @@ -371,20 +371,11 @@ HTMLFrameSetElement::IsEventAttributeName(nsIAtom *aName) NS_IMETHODIMP \ HTMLFrameSetElement::SetOn##name_(JSContext *cx, const JS::Value &v) \ { \ - JSObject *obj = GetWrapper(); \ - if (!obj) { \ - /* Just silently do nothing */ \ - return NS_OK; \ - } \ nsRefPtr handler; \ JSObject *callable; \ if (v.isObject() && \ JS_ObjectIsCallable(cx, callable = &v.toObject())) { \ - bool ok; \ - handler = new type_(cx, obj, callable, &ok); \ - if (!ok) { \ - return NS_ERROR_OUT_OF_MEMORY; \ - } \ + handler = new type_(callable); \ } \ ErrorResult rv; \ forwardto_::SetOn##name_(handler, rv); \ diff --git a/content/xbl/src/nsXBLPrototypeHandler.cpp b/content/xbl/src/nsXBLPrototypeHandler.cpp index 67a8f3a21e3a..6b13d668b529 100644 --- a/content/xbl/src/nsXBLPrototypeHandler.cpp +++ b/content/xbl/src/nsXBLPrototypeHandler.cpp @@ -325,13 +325,9 @@ nsXBLPrototypeHandler::ExecuteHandler(EventTarget* aTarget, if (!JS_WrapObject(cx, bound.address())) { return NS_ERROR_FAILURE; } - JS::Rooted boundHandler(cx, bound); nsRefPtr handlerCallback = - new EventHandlerNonNull(cx, globalObject, boundHandler, &ok); - if (!ok) { - return NS_ERROR_OUT_OF_MEMORY; - } + new EventHandlerNonNull(bound); nsEventHandler eventHandler(handlerCallback); diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index 9d62e6c7346c..2ced52ae6bb3 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -11799,20 +11799,11 @@ nsGlobalWindow::DisableNetworkEvent(uint32_t aType) } \ NS_IMETHODIMP nsGlobalWindow::SetOn##name_(JSContext *cx, \ const JS::Value &v) { \ - JSObject *obj = mJSObject; \ - if (!obj) { \ - /* Just silently do nothing */ \ - return NS_OK; \ - } \ nsRefPtr handler; \ JSObject *callable; \ if (v.isObject() && \ JS_ObjectIsCallable(cx, callable = &v.toObject())) { \ - bool ok; \ - handler = new EventHandlerNonNull(cx, obj, callable, &ok); \ - if (!ok) { \ - return NS_ERROR_OUT_OF_MEMORY; \ - } \ + handler = new EventHandlerNonNull(callable); \ } \ ErrorResult rv; \ SetOn##name_(handler, rv); \ @@ -11839,19 +11830,11 @@ nsGlobalWindow::DisableNetworkEvent(uint32_t aType) return NS_ERROR_OUT_OF_MEMORY; \ } \ \ - JSObject *obj = mJSObject; \ - if (!obj) { \ - return NS_ERROR_UNEXPECTED; \ - } \ nsRefPtr handler; \ JSObject *callable; \ if (v.isObject() && \ JS_ObjectIsCallable(cx, callable = &v.toObject())) { \ - bool ok; \ - handler = new OnErrorEventHandlerNonNull(cx, obj, callable, &ok); \ - if (!ok) { \ - return NS_ERROR_OUT_OF_MEMORY; \ - } \ + handler = new OnErrorEventHandlerNonNull(callable); \ } \ return elm->SetEventHandler(handler); \ } @@ -11877,19 +11860,11 @@ nsGlobalWindow::DisableNetworkEvent(uint32_t aType) return NS_ERROR_OUT_OF_MEMORY; \ } \ \ - JSObject *obj = mJSObject; \ - if (!obj) { \ - return NS_ERROR_UNEXPECTED; \ - } \ nsRefPtr handler; \ JSObject *callable; \ if (v.isObject() && \ JS_ObjectIsCallable(cx, callable = &v.toObject())) { \ - bool ok; \ - handler = new BeforeUnloadEventHandlerNonNull(cx, obj, callable, &ok); \ - if (!ok) { \ - return NS_ERROR_OUT_OF_MEMORY; \ - } \ + handler = new BeforeUnloadEventHandlerNonNull(callable); \ } \ return elm->SetEventHandler(handler); \ } diff --git a/dom/base/nsJSTimeoutHandler.cpp b/dom/base/nsJSTimeoutHandler.cpp index d5d4147639f6..602555a98919 100644 --- a/dom/base/nsJSTimeoutHandler.cpp +++ b/dom/base/nsJSTimeoutHandler.cpp @@ -293,12 +293,7 @@ nsJSScriptTimeoutHandler::Init(nsGlobalWindow *aWindow, bool *aIsInterval, } else if (funobj) { NS_HOLD_JS_OBJECTS(this, nsJSScriptTimeoutHandler); - bool ok; - mFunction = new Function(cx, aWindow->FastGetGlobalJSObject(), funobj, &ok); - if (!ok) { - NS_DROP_JS_OBJECTS(this, nsJSScriptTimeoutHandler); - return NS_ERROR_OUT_OF_MEMORY; - } + mFunction = new Function(funobj); // Create our arg array. argc is the number of arguments passed // to setTimeout or setInterval; the first two are our callback diff --git a/dom/bindings/CallbackFunction.h b/dom/bindings/CallbackFunction.h index a6ab59c6e2e7..1071f2943a14 100644 --- a/dom/bindings/CallbackFunction.h +++ b/dom/bindings/CallbackFunction.h @@ -25,18 +25,10 @@ namespace dom { class CallbackFunction : public CallbackObject { public: - /** - * Create a CallbackFunction. aCallable is the callable we're wrapping. - * aOwner is the object that will be receiving this CallbackFunction as a - * method argument, if any. We need this so we can store our callable in the - * same compartment as our owner. If *aInited is set to false, an exception - * has been thrown. - */ - CallbackFunction(JSContext* cx, JSObject* aOwner, JSObject* aCallable, - bool* aInited) - : CallbackObject(cx, aOwner, aCallable, aInited) + explicit CallbackFunction(JSObject* aCallable) + : CallbackObject(aCallable) { - MOZ_ASSERT(JS_ObjectIsCallable(cx, aCallable)); + MOZ_ASSERT(JS_ObjectIsCallable(nullptr, aCallable)); } JSObject* Callable() const diff --git a/dom/bindings/CallbackInterface.h b/dom/bindings/CallbackInterface.h index 2cf51ecf9d2c..6a060eabbdc9 100644 --- a/dom/bindings/CallbackInterface.h +++ b/dom/bindings/CallbackInterface.h @@ -24,24 +24,6 @@ namespace dom { class CallbackInterface : public CallbackObject { public: - /** - * Create a CallbackInterface. aCallback is the callback object we're - * wrapping. aOwner is the object that will be receiving this - * CallbackInterface as a method argument, if any. We need this so we can - * store our callable in the same compartment as our owner. If *aInited is - * set to false, an exception has been thrown. - */ - CallbackInterface(JSContext* cx, JSObject* aOwner, JSObject* aCallback, - bool* aInited) - : CallbackObject(cx, aOwner, aCallback, aInited) - { - } - - /* - * Create a CallbackInterface without any sort of interesting games with - * compartments, for cases when you want to just use the existing object - * as-is. This constructor can never fail. - */ explicit CallbackInterface(JSObject* aCallback) : CallbackObject(aCallback) { diff --git a/dom/bindings/CallbackObject.h b/dom/bindings/CallbackObject.h index a7399ff9156e..841a6120d6ae 100644 --- a/dom/bindings/CallbackObject.h +++ b/dom/bindings/CallbackObject.h @@ -46,37 +46,6 @@ public: NS_DECL_CYCLE_COLLECTING_ISUPPORTS NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(CallbackObject) - /** - * Create a CallbackObject. aCallback is the callback object we're wrapping. - * aOwner is the object that will be receiving this CallbackObject as a method - * argument, if any. We need this so we can store our callback object in the - * same compartment as our owner. If *aInited is set to false, an exception - * has been thrown. - */ - CallbackObject(JSContext* cx, JSObject* aOwner, JSObject* aCallback, - bool* aInited) - : mCallback(nullptr) - { - // If aOwner is not null, enter the compartment of aOwner's - // underlying object. - if (aOwner) { - aOwner = js::UncheckedUnwrap(aOwner); - JSAutoCompartment ac(cx, aOwner); - if (!JS_WrapObject(cx, &aCallback)) { - *aInited = false; - return; - } - } - - Init(aCallback); - *aInited = true; - } - - /* - * Create a CallbackObject without any sort of interesting games with - * compartments, for cases when you want to just use the existing object - * as-is. This constructor can never fail. - */ explicit CallbackObject(JSObject* aCallback) { Init(aCallback); @@ -377,12 +346,7 @@ public: SafeAutoJSContext cx; JSAutoCompartment ac(cx, obj); - bool inited; - nsRefPtr newCallback = - new WebIDLCallbackT(cx, nullptr, obj, &inited); - if (!inited) { - return nullptr; - } + nsRefPtr newCallback = new WebIDLCallbackT(obj); return newCallback.forget(); } diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index 3d82bd165e6b..57905024d647 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -2793,11 +2793,8 @@ for (uint32_t i = 0; i < length; ++i) { else: declType = CGGeneric("OwningNonNull<%s>" % name) conversion = ( - " bool inited;\n" - " ${declName} = new %s(cx, ${obj}, &${val}.toObject(), &inited);\n" - " if (!inited) {\n" - "%s\n" - " }\n" % (name, CGIndenter(exceptionCodeIndented).define())) + " ${declName} = new %s(&${val}.toObject());\n" % name) + template = wrapObjectTemplate(conversion, type, "${declName} = nullptr", failureCode) @@ -3112,11 +3109,7 @@ for (uint32_t i = 0; i < length; ++i) { else: declType = CGGeneric("OwningNonNull<%s>" % name) conversion = ( - " bool inited;\n" - " ${declName} = new %s(cx, ${obj}, &${val}.toObject(), &inited);\n" - " if (!inited) {\n" - "%s\n" - " }\n" % (name, CGIndenter(exceptionCodeIndented).define())) + " ${declName} = new %s(&${val}.toObject());\n" % name) if allowTreatNonCallableAsNull and type.treatNonCallableAsNull(): haveCallable = "JS_ObjectIsCallable(cx, &${val}.toObject())" @@ -8508,14 +8501,12 @@ class CGCallback(CGClass): def getConstructors(self): return [ClassConstructor( - [Argument("JSContext*", "cx"), - Argument("JSObject*", "aOwner"), - Argument("JSObject*", "aCallback"), - Argument("bool*", "aInited")], + [Argument("JSObject*", "aCallback")], bodyInHeader=True, visibility="public", + explicit=True, baseConstructors=[ - "%s(cx, aOwner, aCallback, aInited)" % self.baseName + "%s(aCallback)" % self.baseName ])] def getMethodImpls(self, method): @@ -8608,16 +8599,6 @@ class CGCallbackInterface(CGCallback): CGCallback.__init__(self, iface, descriptor, "CallbackInterface", methods, getters=getters, setters=setters) - def getConstructors(self): - return CGCallback.getConstructors(self) + [ - ClassConstructor( - [Argument("JSObject*", "aCallback")], - bodyInHeader=True, - visibility="public", - explicit=True, - baseConstructors=["CallbackInterface(aCallback)"]) - ] - class FakeMember(): def __init__(self): self.treatUndefinedAs = self.treatNullAs = "Default"