From 053eaa92c1ffe2ec5e83d16754fabbc7ba27827f Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Tue, 29 Nov 2011 11:44:06 +0200 Subject: [PATCH] Bug 702036 - JSEventListener could have weak mTarget to reduce CC overhead, r=bz --- content/events/src/nsEventListenerManager.cpp | 2 +- content/events/src/nsEventListenerManager.h | 12 ++++++++-- content/xbl/src/nsXBLPrototypeHandler.cpp | 3 ++- dom/base/nsIJSEventListener.h | 22 ++++++++++++++----- dom/src/events/nsJSEventListener.cpp | 4 +--- 5 files changed, 30 insertions(+), 13 deletions(-) diff --git a/content/events/src/nsEventListenerManager.cpp b/content/events/src/nsEventListenerManager.cpp index 32ff49fd242..026f7a639f2 100644 --- a/content/events/src/nsEventListenerManager.cpp +++ b/content/events/src/nsEventListenerManager.cpp @@ -405,7 +405,7 @@ nsEventListenerManager::SetJSEventListener(nsIScriptContext *aContext, if (!ls) { // If we didn't find a script listener or no listeners existed // create and add a new one. - nsCOMPtr scriptListener; + nsCOMPtr scriptListener; rv = NS_NewJSEventListener(aContext, aScopeObject, mTarget, aName, aHandler, getter_AddRefs(scriptListener)); if (NS_SUCCEEDED(rv)) { diff --git a/content/events/src/nsEventListenerManager.h b/content/events/src/nsEventListenerManager.h index 8e54384ced4..f54c1bfffd4 100644 --- a/content/events/src/nsEventListenerManager.h +++ b/content/events/src/nsEventListenerManager.h @@ -63,7 +63,8 @@ class nsCxPusher; class nsIEventListenerInfo; class nsIDocument; -typedef struct { +struct nsListenerStruct +{ nsRefPtr mListener; PRUint32 mEventType; nsCOMPtr mTypeAtom; @@ -74,7 +75,14 @@ typedef struct { return (mFlags & NS_PRIV_EVENT_FLAG_SCRIPT) ? static_cast(mListener.get()) : nsnull; } -} nsListenerStruct; + + ~nsListenerStruct() + { + if ((mFlags & NS_PRIV_EVENT_FLAG_SCRIPT) && mListener) { + static_cast(mListener.get())->Disconnect(); + } + } +}; /* * Event listener manager diff --git a/content/xbl/src/nsXBLPrototypeHandler.cpp b/content/xbl/src/nsXBLPrototypeHandler.cpp index eaa0c6a1c10..0814b99c672 100644 --- a/content/xbl/src/nsXBLPrototypeHandler.cpp +++ b/content/xbl/src/nsXBLPrototypeHandler.cpp @@ -327,7 +327,7 @@ nsXBLPrototypeHandler::ExecuteHandler(nsIDOMEventTarget* aTarget, NS_ENSURE_SUCCESS(rv, rv); // Execute it. - nsCOMPtr eventListener; + nsCOMPtr eventListener; rv = NS_NewJSEventListener(boundContext, scope, scriptTarget, onEventAtom, boundHandler.getObject(), @@ -336,6 +336,7 @@ nsXBLPrototypeHandler::ExecuteHandler(nsIDOMEventTarget* aTarget, // Handle the event. eventListener->HandleEvent(aEvent); + eventListener->Disconnect(); return NS_OK; } diff --git a/dom/base/nsIJSEventListener.h b/dom/base/nsIJSEventListener.h index ef373326ec5..89bd9e979b9 100644 --- a/dom/base/nsIJSEventListener.h +++ b/dom/base/nsIJSEventListener.h @@ -46,13 +46,16 @@ class nsIScriptObjectOwner; class nsIAtom; #define NS_IJSEVENTLISTENER_IID \ -{ 0xafc5d047, 0xdb6b, 0x4076, \ - { 0xb3, 0xfa, 0x57, 0x96, 0x1e, 0x21, 0x48, 0x42 } } +{ 0x92f9212b, 0xa6aa, 0x4867, \ + { 0x93, 0x8a, 0x56, 0xbe, 0x17, 0x67, 0x4f, 0xd4 } } // Implemented by script event listeners. Used to retrieve the // script object corresponding to the event target and the handler itself. // (Note this interface is now used to store script objects for all // script languages, so is no longer JS specific) +// +// Note, mTarget is a raw pointer and the owner of the nsIJSEventListener object +// is expected to call Disconnect()! class nsIJSEventListener : public nsIDOMEventListener { public: @@ -60,9 +63,10 @@ public: nsIJSEventListener(nsIScriptContext* aContext, JSObject* aScopeObject, nsISupports *aTarget, JSObject *aHandler) - : mContext(aContext), mScopeObject(aScopeObject), - mTarget(do_QueryInterface(aTarget)), mHandler(aHandler) + : mContext(aContext), mScopeObject(aScopeObject), mHandler(aHandler) { + nsCOMPtr base = do_QueryInterface(aTarget); + mTarget = base.get(); } nsIScriptContext *GetEventContext() const @@ -75,6 +79,11 @@ public: return mTarget; } + void Disconnect() + { + mTarget = nsnull; + } + JSObject* GetEventScope() const { return mScopeObject; @@ -94,10 +103,11 @@ public: protected: virtual ~nsIJSEventListener() { + NS_ASSERTION(!mTarget, "Should have called Disconnect()!"); } nsCOMPtr mContext; JSObject* mScopeObject; - nsCOMPtr mTarget; + nsISupports* mTarget; JSObject *mHandler; }; @@ -107,6 +117,6 @@ NS_DEFINE_STATIC_IID_ACCESSOR(nsIJSEventListener, NS_IJSEVENTLISTENER_IID) nsresult NS_NewJSEventListener(nsIScriptContext *aContext, JSObject* aScopeObject, nsISupports* aTarget, nsIAtom* aType, JSObject* aHandler, - nsIDOMEventListener **aReturn); + nsIJSEventListener **aReturn); #endif // nsIJSEventListener_h__ diff --git a/dom/src/events/nsJSEventListener.cpp b/dom/src/events/nsJSEventListener.cpp index 778f0d101ce..c56e9f21f5e 100644 --- a/dom/src/events/nsJSEventListener.cpp +++ b/dom/src/events/nsJSEventListener.cpp @@ -103,7 +103,6 @@ nsJSEventListener::~nsJSEventListener() NS_IMPL_CYCLE_COLLECTION_CLASS(nsJSEventListener) NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsJSEventListener) - NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mTarget) if (tmp->mContext) { if (tmp->mContext->GetScriptTypeID() == nsIProgrammingLanguage::JAVASCRIPT) { NS_DROP_JS_OBJECTS(tmp, nsJSEventListener); @@ -117,7 +116,6 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsJSEventListener) } NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsJSEventListener) - NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mTarget) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mContext) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_SCRIPT_OBJECTS NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END @@ -274,7 +272,7 @@ nsJSEventListener::SetHandler(JSObject *aHandler) nsresult NS_NewJSEventListener(nsIScriptContext* aContext, JSObject* aScopeObject, nsISupports*aTarget, nsIAtom* aEventType, - JSObject* aHandler, nsIDOMEventListener ** aReturn) + JSObject* aHandler, nsIJSEventListener** aReturn) { NS_ENSURE_ARG(aEventType); nsJSEventListener* it =