From 5675270420f770f88d75db624f5e45e7c026a456 Mon Sep 17 00:00:00 2001 From: "peterv@propagandism.org" Date: Tue, 12 Feb 2008 08:02:41 -0800 Subject: [PATCH] Fix for bug 415028 (Startup assertions and crash compiling proto properties with gczeal == 2). r/sr=bz. --- content/xbl/src/nsXBLProtoImpl.cpp | 14 +-- content/xbl/src/nsXBLProtoImpl.h | 8 +- content/xbl/src/nsXBLProtoImplMember.h | 1 - content/xbl/src/nsXBLProtoImplMethod.cpp | 124 +++++++++------------ content/xbl/src/nsXBLProtoImplMethod.h | 21 +++- content/xbl/src/nsXBLProtoImplProperty.cpp | 25 +---- content/xbl/src/nsXBLProtoImplProperty.h | 1 - 7 files changed, 79 insertions(+), 115 deletions(-) diff --git a/content/xbl/src/nsXBLProtoImpl.cpp b/content/xbl/src/nsXBLProtoImpl.cpp index a39b3854871f..c1edd37e3916 100644 --- a/content/xbl/src/nsXBLProtoImpl.cpp +++ b/content/xbl/src/nsXBLProtoImpl.cpp @@ -192,7 +192,7 @@ nsXBLProtoImpl::CompilePrototypeMembers(nsXBLPrototypeBinding* aBinding) curr = curr->GetNext()) { nsresult rv = curr->CompileMember(context, mClassName, mClassObject); if (NS_FAILED(rv)) { - DestroyMembers(curr); + DestroyMembers(); return rv; } } @@ -219,7 +219,7 @@ void nsXBLProtoImpl::UnlinkJSObjects() { if (mClassObject) { - DestroyMembers(nsnull); + DestroyMembers(); } } @@ -272,18 +272,10 @@ nsXBLProtoImpl::UndefineFields(JSContext *cx, JSObject *obj) const } void -nsXBLProtoImpl::DestroyMembers(nsXBLProtoImplMember* aBrokenMember) +nsXBLProtoImpl::DestroyMembers() { NS_ASSERTION(mClassObject, "This should never be called when there is no class object"); - PRBool compiled = PR_TRUE; - for (nsXBLProtoImplMember* curr = mMembers; curr; curr = curr->GetNext()) { - if (curr == aBrokenMember) { - compiled = PR_FALSE; - } - curr->Destroy(compiled); - } - // Now clear out mMembers so we don't try to call Destroy() on them again delete mMembers; mMembers = nsnull; mConstructor = nsnull; diff --git a/content/xbl/src/nsXBLProtoImpl.h b/content/xbl/src/nsXBLProtoImpl.h index c62381584e04..a96bbe584942 100644 --- a/content/xbl/src/nsXBLProtoImpl.h +++ b/content/xbl/src/nsXBLProtoImpl.h @@ -65,8 +65,6 @@ public: MOZ_COUNT_DTOR(nsXBLProtoImpl); // Note: the constructor and destructor are in mMembers, so we'll // clean them up automatically. - for (nsXBLProtoImplMember* curr = mMembers; curr; curr=curr->GetNext()) - curr->Destroy(mClassObject != nsnull); delete mMembers; delete mFields; } @@ -108,11 +106,7 @@ public: } protected: - // Function to call if compilation of a member fails. When this is called, - // all members before aBrokenMember are compiled, compilation of - // aBrokenMember failed, and members after aBrokenMember are uncompiled. - // This function assumes that aBrokenMember is _not_ compiled. - void DestroyMembers(nsXBLProtoImplMember* aBrokenMember); + void DestroyMembers(); public: nsCString mClassName; // The name of the class. diff --git a/content/xbl/src/nsXBLProtoImplMember.h b/content/xbl/src/nsXBLProtoImplMember.h index d8e12cd3181f..e1563d84bff2 100644 --- a/content/xbl/src/nsXBLProtoImplMember.h +++ b/content/xbl/src/nsXBLProtoImplMember.h @@ -100,7 +100,6 @@ class nsXBLProtoImplMember public: nsXBLProtoImplMember(const PRUnichar* aName) :mNext(nsnull) { mName = ToNewUnicode(nsDependentString(aName)); } virtual ~nsXBLProtoImplMember() { nsMemory::Free(mName); delete mNext; } - virtual void Destroy(PRBool aIsCompiled)=0; nsXBLProtoImplMember* GetNext() { return mNext; } void SetNext(nsXBLProtoImplMember* aNext) { mNext = aNext; } diff --git a/content/xbl/src/nsXBLProtoImplMethod.cpp b/content/xbl/src/nsXBLProtoImplMethod.cpp index 3da1692c64ce..1efddf2597f3 100644 --- a/content/xbl/src/nsXBLProtoImplMethod.cpp +++ b/content/xbl/src/nsXBLProtoImplMethod.cpp @@ -53,10 +53,7 @@ nsXBLProtoImplMethod::nsXBLProtoImplMethod(const PRUnichar* aName) : nsXBLProtoImplMember(aName), - mUncompiledMethod(nsnull) -#ifdef DEBUG - , mIsCompiled(PR_FALSE) -#endif + mUncompiledMethod(BIT_UNCOMPILED) { MOZ_COUNT_CTOR(nsXBLProtoImplMethod); } @@ -64,62 +61,61 @@ nsXBLProtoImplMethod::nsXBLProtoImplMethod(const PRUnichar* aName) : nsXBLProtoImplMethod::~nsXBLProtoImplMethod() { MOZ_COUNT_DTOR(nsXBLProtoImplMethod); -} -void -nsXBLProtoImplMethod::Destroy(PRBool aIsCompiled) -{ - NS_PRECONDITION(aIsCompiled == mIsCompiled, - "Incorrect aIsCompiled in nsXBLProtoImplMethod::Destroy"); - if (aIsCompiled) { - mJSMethodObject = nsnull; - } - else { - delete mUncompiledMethod; - mUncompiledMethod = nsnull; + if (!IsCompiled()) { + delete GetUncompiledMethod(); } } void nsXBLProtoImplMethod::AppendBodyText(const nsAString& aText) { - NS_PRECONDITION(!mIsCompiled, + NS_PRECONDITION(!IsCompiled(), "Must not be compiled when accessing uncompiled method"); - if (!mUncompiledMethod) { - mUncompiledMethod = new nsXBLUncompiledMethod(); - if (!mUncompiledMethod) + + nsXBLUncompiledMethod* uncompiledMethod = GetUncompiledMethod(); + if (!uncompiledMethod) { + uncompiledMethod = new nsXBLUncompiledMethod(); + if (!uncompiledMethod) return; + SetUncompiledMethod(uncompiledMethod); } - mUncompiledMethod->AppendBodyText(aText); + uncompiledMethod->AppendBodyText(aText); } void nsXBLProtoImplMethod::AddParameter(const nsAString& aText) { - NS_PRECONDITION(!mIsCompiled, + NS_PRECONDITION(!IsCompiled(), "Must not be compiled when accessing uncompiled method"); - if (!mUncompiledMethod) { - mUncompiledMethod = new nsXBLUncompiledMethod(); - if (!mUncompiledMethod) + + nsXBLUncompiledMethod* uncompiledMethod = GetUncompiledMethod(); + if (!uncompiledMethod) { + uncompiledMethod = new nsXBLUncompiledMethod(); + if (!uncompiledMethod) return; + SetUncompiledMethod(uncompiledMethod); } - mUncompiledMethod->AddParameter(aText); + uncompiledMethod->AddParameter(aText); } void nsXBLProtoImplMethod::SetLineNumber(PRUint32 aLineNumber) { - NS_PRECONDITION(!mIsCompiled, + NS_PRECONDITION(!IsCompiled(), "Must not be compiled when accessing uncompiled method"); - if (!mUncompiledMethod) { - mUncompiledMethod = new nsXBLUncompiledMethod(); - if (!mUncompiledMethod) + + nsXBLUncompiledMethod* uncompiledMethod = GetUncompiledMethod(); + if (!uncompiledMethod) { + uncompiledMethod = new nsXBLUncompiledMethod(); + if (!uncompiledMethod) return; + SetUncompiledMethod(uncompiledMethod); } - mUncompiledMethod->SetLineNumber(aLineNumber); + uncompiledMethod->SetLineNumber(aLineNumber); } nsresult @@ -129,7 +125,7 @@ nsXBLProtoImplMethod::InstallMember(nsIScriptContext* aContext, void* aTargetClassObject, const nsCString& aClassStr) { - NS_PRECONDITION(mIsCompiled, + NS_PRECONDITION(IsCompiled(), "Should not be installing an uncompiled method"); JSContext* cx = (JSContext*) aContext->GetNativeContext(); @@ -177,36 +173,34 @@ nsresult nsXBLProtoImplMethod::CompileMember(nsIScriptContext* aContext, const nsCString& aClassStr, void* aClassObject) { - NS_PRECONDITION(!mIsCompiled, + NS_PRECONDITION(!IsCompiled(), "Trying to compile an already-compiled method"); NS_PRECONDITION(aClassObject, "Must have class object to compile"); -#ifdef DEBUG - // We have some "ok" early returns after which we consider ourselves compiled - mIsCompiled = PR_TRUE; -#endif - - // No parameters or body was supplied, so don't install method. - if (!mUncompiledMethod) - return NS_OK; + nsXBLUncompiledMethod* uncompiledMethod = GetUncompiledMethod(); + + // No parameters or body was supplied, so don't install method. + if (!uncompiledMethod) { + // Early return after which we consider ourselves compiled. + mJSMethodObject = nsnull; - // Don't install method if no name was supplied. - if (!mName) { - delete mUncompiledMethod; - mUncompiledMethod = nsnull; return NS_OK; } -#ifdef DEBUG - // OK, now we have some error early returns that mean we're not - // really compiled... - mIsCompiled = PR_FALSE; -#endif + // Don't install method if no name was supplied. + if (!mName) { + delete uncompiledMethod; + + // Early return after which we consider ourselves compiled. + mJSMethodObject = nsnull; + + return NS_OK; + } // We have a method. // Allocate an array for our arguments. - PRInt32 paramCount = mUncompiledMethod->GetParameterCount(); + PRInt32 paramCount = uncompiledMethod->GetParameterCount(); char** args = nsnull; if (paramCount > 0) { args = new char*[paramCount]; @@ -216,7 +210,7 @@ nsXBLProtoImplMethod::CompileMember(nsIScriptContext* aContext, const nsCString& // Add our parameters to our args array. PRInt32 argPos = 0; - for (nsXBLParameter* curr = mUncompiledMethod->mParameters; + for (nsXBLParameter* curr = uncompiledMethod->mParameters; curr; curr = curr->mNext) { args[argPos] = curr->mName; @@ -225,7 +219,7 @@ nsXBLProtoImplMethod::CompileMember(nsIScriptContext* aContext, const nsCString& // Get the body nsDependentString body; - PRUnichar *bodyText = mUncompiledMethod->mBodyText.GetText(); + PRUnichar *bodyText = uncompiledMethod->mBodyText.GetText(); if (bodyText) body.Rebind(bodyText); @@ -245,39 +239,27 @@ nsXBLProtoImplMethod::CompileMember(nsIScriptContext* aContext, const nsCString& (const char**)args, body, functionUri.get(), - mUncompiledMethod->mBodyText.GetLineNumber(), + uncompiledMethod->mBodyText.GetLineNumber(), PR_TRUE, (void **) &methodObject); // Destroy our uncompiled method and delete our arg list. - delete mUncompiledMethod; + delete uncompiledMethod; delete [] args; if (NS_FAILED(rv)) { - mUncompiledMethod = nsnull; + SetUncompiledMethod(nsnull); return rv; } mJSMethodObject = methodObject; - if (methodObject) { - // Root the compiled prototype script object. - if (NS_FAILED(rv)) { - mJSMethodObject = nsnull; - } - } - -#ifdef DEBUG - mIsCompiled = NS_SUCCEEDED(rv); -#endif - return rv; + return NS_OK; } void nsXBLProtoImplMethod::Trace(TraceCallback aCallback, void *aClosure) const { - NS_ASSERTION(mIsCompiled, "Shouldn't traverse uncompiled method"); - - if (mJSMethodObject) { + if (IsCompiled() && mJSMethodObject) { aCallback(nsIProgrammingLanguage::JAVASCRIPT, mJSMethodObject, aClosure); } } @@ -285,7 +267,7 @@ nsXBLProtoImplMethod::Trace(TraceCallback aCallback, void *aClosure) const nsresult nsXBLProtoImplAnonymousMethod::Execute(nsIContent* aBoundElement) { - NS_PRECONDITION(mIsCompiled, "Can't execute uncompiled method"); + NS_PRECONDITION(IsCompiled(), "Can't execute uncompiled method"); if (!mJSMethodObject) { // Nothing to do here diff --git a/content/xbl/src/nsXBLProtoImplMethod.h b/content/xbl/src/nsXBLProtoImplMethod.h index bac2927b56f2..f659c7ecbef3 100644 --- a/content/xbl/src/nsXBLProtoImplMethod.h +++ b/content/xbl/src/nsXBLProtoImplMethod.h @@ -113,7 +113,6 @@ class nsXBLProtoImplMethod: public nsXBLProtoImplMember public: nsXBLProtoImplMethod(const PRUnichar* aName); virtual ~nsXBLProtoImplMethod(); - virtual void Destroy(PRBool aIsCompiled); void AppendBodyText(const nsAString& aBody); void AddParameter(const nsAString& aName); @@ -131,10 +130,26 @@ public: virtual void Trace(TraceCallback aCallback, void *aClosure) const; + PRBool IsCompiled() const + { + return !(mUncompiledMethod & BIT_UNCOMPILED); + } + void SetUncompiledMethod(nsXBLUncompiledMethod* aUncompiledMethod) + { + mUncompiledMethod = PRUptrdiff(aUncompiledMethod) | BIT_UNCOMPILED; + } + nsXBLUncompiledMethod* GetUncompiledMethod() const + { + PRUptrdiff unmasked = mUncompiledMethod & ~BIT_UNCOMPILED; + return reinterpret_cast(unmasked); + } + protected: + enum { BIT_UNCOMPILED = 1 << 0 }; + union { - nsXBLUncompiledMethod* mUncompiledMethod; // An object that represents the method before being compiled. - JSObject * mJSMethodObject; // The JS object for the method (after compilation) + PRUptrdiff mUncompiledMethod; // An object that represents the method before being compiled. + JSObject* mJSMethodObject; // The JS object for the method (after compilation) }; #ifdef DEBUG diff --git a/content/xbl/src/nsXBLProtoImplProperty.cpp b/content/xbl/src/nsXBLProtoImplProperty.cpp index 407265f8acf7..61277a4b5bb8 100644 --- a/content/xbl/src/nsXBLProtoImplProperty.cpp +++ b/content/xbl/src/nsXBLProtoImplProperty.cpp @@ -78,29 +78,14 @@ nsXBLProtoImplProperty::nsXBLProtoImplProperty(const PRUnichar* aName, nsXBLProtoImplProperty::~nsXBLProtoImplProperty() { MOZ_COUNT_DTOR(nsXBLProtoImplProperty); -} -void -nsXBLProtoImplProperty::Destroy(PRBool aIsCompiled) -{ - NS_PRECONDITION(aIsCompiled == mIsCompiled, - "Incorrect aIsCompiled in nsXBLProtoImplProperty::Destroy"); - - if ((mJSAttributes & JSPROP_GETTER) && mJSGetterObject) { - mJSGetterObject = nsnull; - } - else { + if (!(mJSAttributes & JSPROP_GETTER)) { delete mGetterText; } - if ((mJSAttributes & JSPROP_SETTER) && mJSSetterObject) { - mJSSetterObject = nsnull; - } - else { + if (!(mJSAttributes & JSPROP_SETTER)) { delete mSetterText; } - - mGetterText = mSetterText = nsnull; } void @@ -341,13 +326,11 @@ nsXBLProtoImplProperty::CompileMember(nsIScriptContext* aContext, const nsCStrin void nsXBLProtoImplProperty::Trace(TraceCallback aCallback, void *aClosure) const { - NS_ASSERTION(mIsCompiled, "Shouldn't traverse uncompiled method"); - - if ((mJSAttributes & JSPROP_GETTER) && mJSGetterObject) { + if (mJSAttributes & JSPROP_GETTER) { aCallback(nsIProgrammingLanguage::JAVASCRIPT, mJSGetterObject, aClosure); } - if ((mJSAttributes & JSPROP_SETTER) && mJSSetterObject) { + if (mJSAttributes & JSPROP_SETTER) { aCallback(nsIProgrammingLanguage::JAVASCRIPT, mJSSetterObject, aClosure); } } diff --git a/content/xbl/src/nsXBLProtoImplProperty.h b/content/xbl/src/nsXBLProtoImplProperty.h index 9c10326c43de..97b7d8ec2125 100644 --- a/content/xbl/src/nsXBLProtoImplProperty.h +++ b/content/xbl/src/nsXBLProtoImplProperty.h @@ -55,7 +55,6 @@ public: const PRUnichar* aReadOnly); virtual ~nsXBLProtoImplProperty(); - virtual void Destroy(PRBool aIsCompiled); void AppendGetterText(const nsAString& aGetter); void AppendSetterText(const nsAString& aSetter);