From a968a0250c468649d82824796d93c7b0dfefa9b8 Mon Sep 17 00:00:00 2001 From: "bzbarsky%mit.edu" Date: Sat, 24 Sep 2005 02:33:33 +0000 Subject: [PATCH] Make sure to root the function object we compile in nsXBLPrototypeHandler::ExecuteHandler. r=jst, sr=brendan --- content/base/public/nsContentUtils.h | 81 +++++++++++++++++- content/base/src/nsContentUtils.cpp | 59 +++++++++++++ content/xbl/src/Makefile.in | 1 - content/xbl/src/nsXBLProtoImplField.cpp | 1 + content/xbl/src/nsXBLProtoImplMember.h | 36 -------- content/xbl/src/nsXBLProtoImplMethod.cpp | 5 +- content/xbl/src/nsXBLProtoImplProperty.cpp | 11 ++- content/xbl/src/nsXBLPrototypeHandler.cpp | 3 + content/xul/content/src/nsXULElement.cpp | 84 ++++++------------- content/xul/content/src/nsXULElement.h | 5 +- content/xul/document/src/nsXULContentSink.cpp | 7 +- 11 files changed, 188 insertions(+), 105 deletions(-) diff --git a/content/base/public/nsContentUtils.h b/content/base/public/nsContentUtils.h index a13cf1ef63a1..e8e095acf314 100644 --- a/content/base/public/nsContentUtils.h +++ b/content/base/public/nsContentUtils.h @@ -81,6 +81,8 @@ class nsIContentPolicy; class nsILineBreaker; class nsIWordBreaker; class nsIEventQueueService; +class nsIJSRuntimeService; +struct JSRuntime; #ifdef MOZ_XTF class nsIXTFService; #endif @@ -581,6 +583,42 @@ public: { return sEventQueueService; } + + /** + * Make sure that whatever value *aPtr contains at any given moment is + * protected from JS GC until we remove the GC root. A call to this that + * succeeds MUST be matched by a call to RemoveJSGCRoot to avoid leaking. + */ + static nsresult AddJSGCRoot(jsval* aPtr, const char* aName) { + return AddJSGCRoot((void*)aPtr, aName); + } + + /** + * Make sure that whatever object *aPtr is pointing to at any given moment is + * protected from JS GC until we remove the GC root. A call to this that + * succeeds MUST be matched by a call to RemoveJSGCRoot to avoid leaking. + */ + static nsresult AddJSGCRoot(JSObject** aPtr, const char* aName) { + return AddJSGCRoot((void*)aPtr, aName); + } + + /** + * Make sure that whatever object *aPtr is pointing to at any given moment is + * protected from JS GC until we remove the GC root. A call to this that + * succeeds MUST be matched by a call to RemoveJSGCRoot to avoid leaking. + */ + static nsresult AddJSGCRoot(void* aPtr, const char* aName); + + /** + * Remove aPtr as a JS GC root + */ + static nsresult RemoveJSGCRoot(jsval* aPtr) { + return RemoveJSGCRoot((void*)aPtr); + } + static nsresult RemoveJSGCRoot(JSObject** aPtr) { + return RemoveJSGCRoot((void*)aPtr); + } + static nsresult RemoveJSGCRoot(void* aPtr); private: static nsresult doReparentContentWrapper(nsIContent *aChild, @@ -591,7 +629,6 @@ private: static nsresult EnsureStringBundle(PropertiesFile aFile); - static nsIDOMScriptObjectFactory *sDOMScriptObjectFactory; static nsIXPConnect *sXPConnect; @@ -631,6 +668,12 @@ private: // Holds pointers to nsISupports* that should be released at shutdown static nsVoidArray* sPtrsToPtrsToRelease; + + // For now, we don't want to automatically clean this up in Shutdown(), since + // consumers might unfortunately end up wanting to use it after that + static nsIJSRuntimeService* sJSRuntimeService; + static JSRuntime* sScriptRuntime; + static PRInt32 sScriptRootCount; static PRBool sInitialized; }; @@ -661,6 +704,42 @@ private: PRBool mScriptIsRunning; }; +class nsAutoGCRoot { +public: + // aPtr should be the pointer to the jsval we want to protect + nsAutoGCRoot(jsval* aPtr, nsresult* aResult) : + mPtr(aPtr) + { + mResult = *aResult = + nsContentUtils::AddJSGCRoot(aPtr, "nsAutoGCRoot"); + } + + // aPtr should be the pointer to the JSObject* we want to protect + nsAutoGCRoot(JSObject** aPtr, nsresult* aResult) : + mPtr(aPtr) + { + mResult = *aResult = + nsContentUtils::AddJSGCRoot(aPtr, "nsAutoGCRoot"); + } + + // aPtr should be the pointer to the thing we want to protect + nsAutoGCRoot(void* aPtr, nsresult* aResult) : + mPtr(aPtr) + { + mResult = *aResult = + nsContentUtils::AddJSGCRoot(aPtr, "nsAutoGCRoot"); + } + + ~nsAutoGCRoot() { + if (NS_SUCCEEDED(mResult)) { + nsContentUtils::RemoveJSGCRoot(mPtr); + } + } + +private: + void* mPtr; + nsresult mResult; +}; #define NS_INTERFACE_MAP_ENTRY_CONTENT_CLASSINFO(_class) \ if (aIID.Equals(NS_GET_IID(nsIClassInfo))) { \ diff --git a/content/base/src/nsContentUtils.cpp b/content/base/src/nsContentUtils.cpp index 31844ad5dd77..e0e7b81aaf24 100644 --- a/content/base/src/nsContentUtils.cpp +++ b/content/base/src/nsContentUtils.cpp @@ -114,6 +114,7 @@ static NS_DEFINE_CID(kXTFServiceCID, NS_XTFSERVICE_CID); #include "nsIWordBreaker.h" #include "nsIEventQueueService.h" #include "jsdbgapi.h" +#include "nsIJSRuntimeService.h" // for ReportToConsole #include "nsIStringBundle.h" @@ -145,6 +146,9 @@ nsILineBreaker *nsContentUtils::sLineBreaker; nsIWordBreaker *nsContentUtils::sWordBreaker; nsIEventQueueService *nsContentUtils::sEventQueueService; nsVoidArray *nsContentUtils::sPtrsToPtrsToRelease; +nsIJSRuntimeService *nsContentUtils::sJSRuntimeService; +JSRuntime *nsContentUtils::sScriptRuntime; +PRInt32 nsContentUtils::sScriptRootCount = 0; PRBool nsContentUtils::sInitialized = PR_FALSE; @@ -2390,3 +2394,58 @@ nsContentUtils::GetContentPolicy() return sContentPolicyService; } + +// static +nsresult +nsContentUtils::AddJSGCRoot(void* aPtr, const char* aName) +{ + if (!sScriptRuntime) { + nsresult rv = CallGetService("@mozilla.org/js/xpc/RuntimeService;1", + &sJSRuntimeService); + NS_ENSURE_TRUE(sJSRuntimeService, rv); + + sJSRuntimeService->GetRuntime(&sScriptRuntime); + if (!sScriptRuntime) { + NS_RELEASE(sJSRuntimeService); + NS_WARNING("Unable to get JS runtime from JS runtime service"); + return NS_ERROR_FAILURE; + } + } + + PRBool ok; + ok = ::JS_AddNamedRootRT(sScriptRuntime, aPtr, aName); + if (!ok) { + if (sScriptRootCount == 0) { + // We just got the runtime... Just null things out, since no + // one's expecting us to have a runtime yet + NS_RELEASE(sJSRuntimeService); + sScriptRuntime = nsnull; + } + NS_WARNING("JS_AddNamedRootRT failed"); + return NS_ERROR_OUT_OF_MEMORY; + } + + // We now have one more root we added to the runtime + ++sScriptRootCount; + + return NS_OK; +} + +/* static */ +nsresult +nsContentUtils::RemoveJSGCRoot(void* aPtr) +{ + if (!sScriptRuntime) { + NS_NOTREACHED("Trying to remove a JS GC root when none were added"); + return NS_ERROR_UNEXPECTED; + } + + ::JS_RemoveRootRT(sScriptRuntime, aPtr); + + if (--sScriptRootCount == 0) { + NS_RELEASE(sJSRuntimeService); + sScriptRuntime = nsnull; + } + + return NS_OK; +} diff --git a/content/xbl/src/Makefile.in b/content/xbl/src/Makefile.in index c939ddbf7813..96dbffb08fb1 100644 --- a/content/xbl/src/Makefile.in +++ b/content/xbl/src/Makefile.in @@ -82,7 +82,6 @@ CPPSRCS = \ nsXBLResourceLoader.cpp \ nsXBLDocumentInfo.cpp \ nsXBLContentSink.cpp \ - nsXBLProtoImplMember.cpp \ nsXBLProtoImplProperty.cpp \ nsXBLProtoImplMethod.cpp \ nsXBLProtoImplField.cpp \ diff --git a/content/xbl/src/nsXBLProtoImplField.cpp b/content/xbl/src/nsXBLProtoImplField.cpp index c6330fa49095..b0fe3a81eb86 100644 --- a/content/xbl/src/nsXBLProtoImplField.cpp +++ b/content/xbl/src/nsXBLProtoImplField.cpp @@ -45,6 +45,7 @@ #include "nsReadableUtils.h" #include "nsXBLProtoImplField.h" #include "nsIScriptContext.h" +#include "nsContentUtils.h" MOZ_DECL_CTOR_COUNTER(nsXBLProtoImplField) diff --git a/content/xbl/src/nsXBLProtoImplMember.h b/content/xbl/src/nsXBLProtoImplMember.h index 5183ab9e8fb3..56e2de7c3c58 100644 --- a/content/xbl/src/nsXBLProtoImplMember.h +++ b/content/xbl/src/nsXBLProtoImplMember.h @@ -96,8 +96,6 @@ struct nsXBLTextWithLineNumber } }; -struct nsAutoGCRoot; - class nsXBLProtoImplMember { public: @@ -122,40 +120,6 @@ protected: nsXBLProtoImplMember* mNext; // The members of an implementation are chained. PRUnichar* mName; // The name of the field, method, or property. - static nsIJSRuntimeService* gJSRuntimeService; - static JSRuntime* gScriptRuntime; - static PRInt32 gScriptRuntimeRefcnt; - - static nsresult AddJSGCRoot(void* aScriptObjectRef, const char* aName); - static nsresult RemoveJSGCRoot(void* aScriptObjectRef); -}; - -// Struct to handle automatic rooting and unrooting -struct nsAutoGCRoot { - // aPtr should be the pointer to the jsval we want to protect - nsAutoGCRoot(jsval* aPtr, nsresult* aResult) : - mPtr(aPtr) - { - mResult = *aResult = - nsXBLProtoImplMember::AddJSGCRoot(mPtr, "nsAutoGCRoot"); - } - - // aPtr should be the pointer to the JSObject* we want to protect - nsAutoGCRoot(JSObject** aPtr, nsresult* aResult) : - mPtr(aPtr) - { - mResult = *aResult = - nsXBLProtoImplMember::AddJSGCRoot(mPtr, "nsAutoGCRoot"); - } - - ~nsAutoGCRoot() { - if (NS_SUCCEEDED(mResult)) { - nsXBLProtoImplMember::RemoveJSGCRoot(mPtr); - } - } - - void* mPtr; - nsresult mResult; }; #endif // nsXBLProtoImplMember_h__ diff --git a/content/xbl/src/nsXBLProtoImplMethod.cpp b/content/xbl/src/nsXBLProtoImplMethod.cpp index 6f386d6fbd06..482256763bc2 100644 --- a/content/xbl/src/nsXBLProtoImplMethod.cpp +++ b/content/xbl/src/nsXBLProtoImplMethod.cpp @@ -74,7 +74,7 @@ nsXBLProtoImplMethod::Destroy(PRBool aIsCompiled) "Incorrect aIsCompiled in nsXBLProtoImplMethod::Destroy"); if (aIsCompiled) { if (mJSMethodObject) - RemoveJSGCRoot(&mJSMethodObject); + nsContentUtils::RemoveJSGCRoot(&mJSMethodObject); mJSMethodObject = nsnull; } else { @@ -258,7 +258,8 @@ nsXBLProtoImplMethod::CompileMember(nsIScriptContext* aContext, const nsCString& JSContext* cx = NS_REINTERPRET_CAST(JSContext*, aContext->GetNativeContext()); rv = cx ? - AddJSGCRoot(&mJSMethodObject, "nsXBLProtoImplMethod::mJSMethodObject") : + nsContentUtils::AddJSGCRoot(&mJSMethodObject, + "nsXBLProtoImplMethod::mJSMethodObject") : NS_ERROR_UNEXPECTED; if (NS_FAILED(rv)) { mJSMethodObject = nsnull; diff --git a/content/xbl/src/nsXBLProtoImplProperty.cpp b/content/xbl/src/nsXBLProtoImplProperty.cpp index 4d72c7ffff72..4e6e5338fd26 100644 --- a/content/xbl/src/nsXBLProtoImplProperty.cpp +++ b/content/xbl/src/nsXBLProtoImplProperty.cpp @@ -47,6 +47,7 @@ #include "nsReadableUtils.h" #include "nsIScriptContext.h" #include "nsIScriptGlobalObject.h" +#include "nsContentUtils.h" MOZ_DECL_CTOR_COUNTER(nsXBLProtoImplProperty) @@ -88,14 +89,14 @@ nsXBLProtoImplProperty::Destroy(PRBool aIsCompiled) "Incorrect aIsCompiled in nsXBLProtoImplProperty::Destroy"); if ((mJSAttributes & JSPROP_GETTER) && mJSGetterObject) { - RemoveJSGCRoot(&mJSGetterObject); + nsContentUtils::RemoveJSGCRoot(&mJSGetterObject); } else { delete mGetterText; } if ((mJSAttributes & JSPROP_SETTER) && mJSSetterObject) { - RemoveJSGCRoot(&mJSSetterObject); + nsContentUtils::RemoveJSGCRoot(&mJSSetterObject); } else { delete mSetterText; @@ -269,7 +270,8 @@ nsXBLProtoImplProperty::CompileMember(nsIScriptContext* aContext, const nsCStrin JSContext* cx = NS_REINTERPRET_CAST(JSContext*, aContext->GetNativeContext()); rv = (cx) - ? AddJSGCRoot(&mJSGetterObject, "nsXBLProtoImplProperty::mJSGetterObject") + ? nsContentUtils::AddJSGCRoot(&mJSGetterObject, + "nsXBLProtoImplProperty::mJSGetterObject") : NS_ERROR_UNEXPECTED; } if (NS_FAILED(rv)) { @@ -324,7 +326,8 @@ nsXBLProtoImplProperty::CompileMember(nsIScriptContext* aContext, const nsCStrin JSContext* cx = NS_REINTERPRET_CAST(JSContext*, aContext->GetNativeContext()); rv = (cx) - ? AddJSGCRoot(&mJSSetterObject, "nsXBLProtoImplProperty::mJSSetterObject") + ? nsContentUtils::AddJSGCRoot(&mJSSetterObject, + "nsXBLProtoImplProperty::mJSSetterObject") : NS_ERROR_UNEXPECTED; } if (NS_FAILED(rv)) { diff --git a/content/xbl/src/nsXBLPrototypeHandler.cpp b/content/xbl/src/nsXBLPrototypeHandler.cpp index 12515d404df8..c1527844cbfe 100644 --- a/content/xbl/src/nsXBLPrototypeHandler.cpp +++ b/content/xbl/src/nsXBLPrototypeHandler.cpp @@ -486,6 +486,9 @@ nsXBLPrototypeHandler::ExecuteHandler(nsIDOMEventReceiver* aReceiver, } NS_ENSURE_SUCCESS(rv, rv); + nsAutoGCRoot root(&handler, &rv); + NS_ENSURE_SUCCESS(rv, rv); + // Temporarily bind it to the bound element boundContext->BindCompiledEventHandler(scriptObject, onEventAtom, handler); diff --git a/content/xul/content/src/nsXULElement.cpp b/content/xul/content/src/nsXULElement.cpp index 3f16739a7361..ba8035a3e4d1 100644 --- a/content/xul/content/src/nsXULElement.cpp +++ b/content/xul/content/src/nsXULElement.cpp @@ -204,57 +204,6 @@ static struct { } gFaults; #endif -#include "nsIJSRuntimeService.h" -static nsIJSRuntimeService* gJSRuntimeService = nsnull; -static JSRuntime* gScriptRuntime = nsnull; -static PRInt32 gScriptRuntimeRefcnt = 0; - -static nsresult -AddJSGCRoot(void* aScriptObjectRef, const char* aName) -{ - if (++gScriptRuntimeRefcnt == 1 || !gScriptRuntime) { - CallGetService("@mozilla.org/js/xpc/RuntimeService;1", - &gJSRuntimeService); - if (! gJSRuntimeService) { - NS_NOTREACHED("couldn't add GC root"); - return NS_ERROR_FAILURE; - } - - gJSRuntimeService->GetRuntime(&gScriptRuntime); - if (! gScriptRuntime) { - NS_NOTREACHED("couldn't add GC root"); - return NS_ERROR_FAILURE; - } - } - - PRBool ok; - ok = ::JS_AddNamedRootRT(gScriptRuntime, aScriptObjectRef, aName); - if (! ok) { - NS_NOTREACHED("couldn't add GC root"); - return NS_ERROR_OUT_OF_MEMORY; - } - - return NS_OK; -} - -static nsresult -RemoveJSGCRoot(void* aScriptObjectRef) -{ - if (! gScriptRuntime) { - NS_NOTREACHED("couldn't remove GC root"); - return NS_ERROR_FAILURE; - } - - ::JS_RemoveRootRT(gScriptRuntime, aScriptObjectRef); - - if (--gScriptRuntimeRefcnt == 0) { - NS_RELEASE(gJSRuntimeService); - gScriptRuntime = nsnull; - } - - return NS_OK; -} - //---------------------------------------------------------------------- @@ -794,9 +743,12 @@ nsXULElement::CompileEventHandler(nsIScriptContext* aContext, if (!cx) return NS_ERROR_UNEXPECTED; - rv = AddJSGCRoot(&attr->mEventHandler, - "nsXULPrototypeAttribute::mEventHandler"); - if (NS_FAILED(rv)) return rv; + rv = nsContentUtils::AddJSGCRoot(&attr->mEventHandler, + "nsXULPrototypeAttribute::mEventHandler"); + if (NS_FAILED(rv)) { + attr->mEventHandler = nsnull; + return rv; + } } } @@ -2922,7 +2874,7 @@ nsXULPrototypeAttribute::~nsXULPrototypeAttribute() { MOZ_COUNT_DTOR(nsXULPrototypeAttribute); if (mEventHandler) - RemoveJSGCRoot(&mEventHandler); + nsContentUtils::RemoveJSGCRoot(&mEventHandler); } @@ -3085,9 +3037,16 @@ nsXULPrototypeElement::Deserialize(nsIObjectInputStream* aStream, break; case eType_Script: { // language version/options obtained during deserialization. - nsXULPrototypeScript* script = new nsXULPrototypeScript(0, nsnull, PR_FALSE); + // Don't clobber rv here, since it might already be a failure! + nsresult result; + nsXULPrototypeScript* script = + new nsXULPrototypeScript(0, nsnull, PR_FALSE, &result); if (! script) return NS_ERROR_OUT_OF_MEMORY; + if (NS_FAILED(result)) { + delete script; + return result; + } child = script; child->mType = childType; @@ -3180,7 +3139,10 @@ nsXULPrototypeElement::SetAttrAt(PRUint32 aPos, const nsAString& aValue, // nsXULPrototypeScript // -nsXULPrototypeScript::nsXULPrototypeScript(PRUint32 aLineNo, const char *aVersion, PRBool aHasE4XOption) +nsXULPrototypeScript::nsXULPrototypeScript(PRUint32 aLineNo, + const char *aVersion, + PRBool aHasE4XOption, + nsresult* rv) : nsXULPrototypeNode(eType_Script), mLineNo(aLineNo), mSrcLoading(PR_FALSE), @@ -3191,13 +3153,17 @@ nsXULPrototypeScript::nsXULPrototypeScript(PRUint32 aLineNo, const char *aVersio mLangVersion(aVersion) { NS_LOG_ADDREF(this, 1, ClassName(), ClassSize()); - AddJSGCRoot(&mJSObject, "nsXULPrototypeScript::mJSObject"); + *rv = nsContentUtils::AddJSGCRoot(&mJSObject, + "nsXULPrototypeScript::mJSObject"); + mAddedGCRoot = NS_SUCCEEDED(*rv); } nsXULPrototypeScript::~nsXULPrototypeScript() { - RemoveJSGCRoot(&mJSObject); + if (mAddedGCRoot) { + nsContentUtils::RemoveJSGCRoot(&mJSObject); + } } diff --git a/content/xul/content/src/nsXULElement.h b/content/xul/content/src/nsXULElement.h index 34fb6c5dc402..d237aa615278 100644 --- a/content/xul/content/src/nsXULElement.h +++ b/content/xul/content/src/nsXULElement.h @@ -312,8 +312,10 @@ class nsXULDocument; class nsXULPrototypeScript : public nsXULPrototypeNode { public: + // Note: if *rv is failure after the script is constructed, delete + // it and return *rv. nsXULPrototypeScript(PRUint32 aLineNo, const char *aVersion, - PRBool aHasE4XOption); + PRBool aHasE4XOption, nsresult* rv); virtual ~nsXULPrototypeScript(); #ifdef NS_BUILD_REFCNT_LOGGING @@ -343,6 +345,7 @@ public: PRPackedBool mSrcLoading; PRPackedBool mOutOfLine; PRPackedBool mHasE4XOption; + PRPackedBool mAddedGCRoot; nsXULDocument* mSrcLoadWaiters; // [OWNER] but not COMPtr JSObject* mJSObject; const char* mLangVersion; diff --git a/content/xul/document/src/nsXULContentSink.cpp b/content/xul/document/src/nsXULContentSink.cpp index 64e761944904..248b8a88ec43 100644 --- a/content/xul/document/src/nsXULContentSink.cpp +++ b/content/xul/document/src/nsXULContentSink.cpp @@ -1256,9 +1256,14 @@ XULContentSinkImpl::OpenScript(const PRUnichar** aAttributes, // Don't process scripts that aren't JavaScript if (isJavaScript) { nsXULPrototypeScript* script = - new nsXULPrototypeScript(aLineNumber, jsVersionString, hasE4XOption); + new nsXULPrototypeScript(aLineNumber, jsVersionString, hasE4XOption, + &rv); if (! script) return NS_ERROR_OUT_OF_MEMORY; + if (NS_FAILED(rv)) { + delete script; + return rv; + } // If there is a SRC attribute... if (! src.IsEmpty()) {