From 225d119eed6078d728e9022234fb76c3a7bac926 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Fri, 8 Feb 2013 14:24:20 +0000 Subject: [PATCH] Bug 821850 - Make DoInitJSClass unconditionally fill in aClassObject. r=bz 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. --- content/xbl/src/nsXBLBinding.cpp | 7 ++++-- content/xbl/src/nsXBLBinding.h | 2 +- content/xbl/src/nsXBLProtoImpl.cpp | 28 +++++++++++++++------- content/xbl/src/nsXBLProtoImpl.h | 3 ++- content/xbl/src/nsXBLProtoImplMethod.cpp | 2 +- content/xbl/src/nsXBLProtoImplProperty.cpp | 2 +- content/xbl/src/nsXBLPrototypeBinding.cpp | 5 ++-- content/xbl/src/nsXBLPrototypeBinding.h | 2 +- 8 files changed, 34 insertions(+), 17 deletions(-) diff --git a/content/xbl/src/nsXBLBinding.cpp b/content/xbl/src/nsXBLBinding.cpp index 37c46ed343ff..9aa1333b757b 100644 --- a/content/xbl/src/nsXBLBinding.cpp +++ b/content/xbl/src/nsXBLBinding.cpp @@ -1352,7 +1352,7 @@ nsresult nsXBLBinding::DoInitJSClass(JSContext *cx, JSObject *global, JSObject *obj, const nsAFlatCString& aClassName, nsXBLPrototypeBinding* aProtoBinding, - JSObject** aClassObject) + JSObject** aClassObject, bool* aNew) { // First ensure our JS class is initialized. nsAutoCString className(aClassName); @@ -1408,6 +1408,7 @@ nsXBLBinding::DoInitJSClass(JSContext *cx, JSObject *global, JSObject *obj, if ((!::JS_LookupPropertyWithFlags(cx, global, className.get(), 0, &val)) || JSVAL_IS_PRIMITIVE(val)) { // We need to initialize the class. + *aNew = true; nsCStringKey key(xblKey); if (!c) { @@ -1485,12 +1486,14 @@ nsXBLBinding::DoInitJSClass(JSContext *cx, JSObject *global, JSObject *obj, ::JS_SetReservedSlot(proto, 0, PRIVATE_TO_JSVAL(aProtoBinding)); - *aClassObject = proto; } else { + *aNew = false; proto = JSVAL_TO_OBJECT(val); } + *aClassObject = proto; + if (obj) { // Set the prototype of our object to be the new class. if (!::JS_SetPrototype(cx, obj, proto)) { diff --git a/content/xbl/src/nsXBLBinding.h b/content/xbl/src/nsXBLBinding.h index a1009eda07a1..d53dfbc07a69 100644 --- a/content/xbl/src/nsXBLBinding.h +++ b/content/xbl/src/nsXBLBinding.h @@ -114,7 +114,7 @@ public: static nsresult DoInitJSClass(JSContext *cx, JSObject *global, JSObject *obj, const nsAFlatCString& aClassName, nsXBLPrototypeBinding* aProtoBinding, - JSObject** aClassObject); + JSObject** aClassObject, bool* aNew); bool AllowScripts(); // XXX make const diff --git a/content/xbl/src/nsXBLProtoImpl.cpp b/content/xbl/src/nsXBLProtoImpl.cpp index 9638bfcce570..34efde937f79 100644 --- a/content/xbl/src/nsXBLProtoImpl.cpp +++ b/content/xbl/src/nsXBLProtoImpl.cpp @@ -68,10 +68,17 @@ nsXBLProtoImpl::InstallImplementation(nsXBLPrototypeBinding* aPrototypeBinding, // not been built already. nsCOMPtr holder; JSObject* targetClassObject = nullptr; + bool targetObjectIsNew = false; nsresult rv = InitTargetObjects(aPrototypeBinding, context, aBinding->GetBoundElement(), - getter_AddRefs(holder), &targetClassObject); + getter_AddRefs(holder), &targetClassObject, + &targetObjectIsNew); NS_ENSURE_SUCCESS(rv, rv); // kick out if we were unable to properly intialize our target objects + MOZ_ASSERT(targetClassObject); + + // If the prototype already existed, we don't need to install anything. return early. + if (!targetObjectIsNew) + return NS_OK; JSObject * targetScriptObject; holder->GetJSObject(&targetScriptObject); @@ -95,7 +102,8 @@ nsXBLProtoImpl::InitTargetObjects(nsXBLPrototypeBinding* aBinding, nsIScriptContext* aContext, nsIContent* aBoundElement, nsIXPConnectJSObjectHolder** aScriptObjectHolder, - JSObject** aTargetClassObject) + JSObject** aTargetClassObject, + bool* aTargetIsNew) { nsresult rv = NS_OK; *aScriptObjectHolder = nullptr; @@ -132,7 +140,7 @@ nsXBLProtoImpl::InitTargetObjects(nsXBLPrototypeBinding* aBinding, // between the object and its base class. We become the new base class of the object, and the // object's old base class becomes the new class' base class. rv = aBinding->InitClass(mClassName, jscontext, global, JSVAL_TO_OBJECT(v), - aTargetClassObject); + aTargetClassObject, aTargetIsNew); if (NS_FAILED(rv)) { return rv; } @@ -163,14 +171,15 @@ nsXBLProtoImpl::CompilePrototypeMembers(nsXBLPrototypeBinding* aBinding) JSObject* classObject; + bool classObjectIsNew = false; nsresult rv = aBinding->InitClass(mClassName, cx, global, global, - &classObject); + &classObject, &classObjectIsNew); if (NS_FAILED(rv)) return rv; + MOZ_ASSERT(classObjectIsNew); + MOZ_ASSERT(classObject); mClassObject = classObject; - if (!mClassObject) - return NS_ERROR_FAILURE; AutoVersionChecker avc(cx); @@ -284,9 +293,12 @@ nsXBLProtoImpl::Read(nsIScriptContext* aContext, JSObject *global = aGlobal->GetGlobalJSObject(); JSObject* classObject; - nsresult rv = aBinding->InitClass(mClassName, cx, global, global, &classObject); + bool classObjectIsNew = false; + nsresult rv = aBinding->InitClass(mClassName, cx, global, global, &classObject, + &classObjectIsNew); NS_ENSURE_SUCCESS(rv, rv); - NS_ENSURE_TRUE(classObject, NS_ERROR_FAILURE); + MOZ_ASSERT(classObject); + MOZ_ASSERT(classObjectIsNew); mClassObject = classObject; diff --git a/content/xbl/src/nsXBLProtoImpl.h b/content/xbl/src/nsXBLProtoImpl.h index 4d6c73b27dd7..d3b712f050da 100644 --- a/content/xbl/src/nsXBLProtoImpl.h +++ b/content/xbl/src/nsXBLProtoImpl.h @@ -41,7 +41,8 @@ public: nsresult InitTargetObjects(nsXBLPrototypeBinding* aBinding, nsIScriptContext* aContext, nsIContent* aBoundElement, nsIXPConnectJSObjectHolder** aScriptObjectHolder, - JSObject** aTargetClassObject); + JSObject** aTargetClassObject, + bool* aTargetIsNew); nsresult CompilePrototypeMembers(nsXBLPrototypeBinding* aBinding); void SetMemberList(nsXBLProtoImplMember* aMemberList) diff --git a/content/xbl/src/nsXBLProtoImplMethod.cpp b/content/xbl/src/nsXBLProtoImplMethod.cpp index 2561046fc7d3..925121a6dc72 100644 --- a/content/xbl/src/nsXBLProtoImplMethod.cpp +++ b/content/xbl/src/nsXBLProtoImplMethod.cpp @@ -115,7 +115,7 @@ nsXBLProtoImplMethod::InstallMember(nsIScriptContext* aContext, JSObject* globalObject = sgo->GetGlobalJSObject(); // now we want to reevaluate our property using aContext and the script object for this window... - if (mJSMethodObject && aTargetClassObject) { + if (mJSMethodObject) { nsDependentString name(mName); JSAutoRequest ar(cx); JSAutoCompartment ac(cx, globalObject); diff --git a/content/xbl/src/nsXBLProtoImplProperty.cpp b/content/xbl/src/nsXBLProtoImplProperty.cpp index 05183793f791..27ed2463f331 100644 --- a/content/xbl/src/nsXBLProtoImplProperty.cpp +++ b/content/xbl/src/nsXBLProtoImplProperty.cpp @@ -158,7 +158,7 @@ nsXBLProtoImplProperty::InstallMember(nsIScriptContext* aContext, JSObject * globalObject = sgo->GetGlobalJSObject(); // now we want to reevaluate our property using aContext and the script object for this window... - if ((mJSGetterObject || mJSSetterObject) && aTargetClassObject) { + if (mJSGetterObject || mJSSetterObject) { JSObject * getter = nullptr; JSAutoRequest ar(cx); JSAutoCompartment ac(cx, globalObject); diff --git a/content/xbl/src/nsXBLPrototypeBinding.cpp b/content/xbl/src/nsXBLPrototypeBinding.cpp index d0a0b5219d2c..3112c526a910 100644 --- a/content/xbl/src/nsXBLPrototypeBinding.cpp +++ b/content/xbl/src/nsXBLPrototypeBinding.cpp @@ -841,14 +841,15 @@ nsresult nsXBLPrototypeBinding::InitClass(const nsCString& aClassName, JSContext * aContext, JSObject * aGlobal, JSObject * aScriptObject, - JSObject** aClassObject) + JSObject** aClassObject, + bool* aNew) { NS_ENSURE_ARG_POINTER(aClassObject); *aClassObject = nullptr; return nsXBLBinding::DoInitJSClass(aContext, aGlobal, aScriptObject, - aClassName, this, aClassObject); + aClassName, this, aClassObject, aNew); } nsIContent* diff --git a/content/xbl/src/nsXBLPrototypeBinding.h b/content/xbl/src/nsXBLPrototypeBinding.h index fc4857f6d220..6edab044f88f 100644 --- a/content/xbl/src/nsXBLPrototypeBinding.h +++ b/content/xbl/src/nsXBLPrototypeBinding.h @@ -121,7 +121,7 @@ public: nsresult InitClass(const nsCString& aClassName, JSContext * aContext, JSObject * aGlobal, JSObject * aScriptObject, - JSObject** aClassObject); + JSObject** aClassObject, bool* aNew); nsresult ConstructInterfaceTable(const nsAString& aImpls);