Remove event listeners when dropping an event listener manager so that the weak references in DOMClassInfo's preserved wrapper table for event listeners are removed before the ELM's owner (the GC participant) goes away, and document why this needs to be done. Also consolidate the things that need to be done before letting go of an ELM into one method, and add some missing calls to that method. b=323807 r=mrbkap sr=jst

content/base/public/nsIDOMGCParticipant.h

@@ -52,6 +52,12 @@
  * reachability in the native object graphs to help prevent script ->
  * native -> script cyclical reference from causing leaks due to the
  * creation of garbage collection roots and native/script boundaries.
+ *
+ * Some implementations of nsIDOMGCParticipant may be responsible for
+ * enforcing the requirement that callers of
+ * |nsDOMClassInfo::PreserveWrapper| must call
+ * |nsDOMClassInfo::ReleaseWrapper| before the nsIDOMGCParticipant
+ * argument to the former is destroyed.
  */
 class nsIDOMGCParticipant : public nsISupports
 {

content/base/src/nsDocument.cpp

@@ -776,7 +776,7 @@ nsDocument::~nsDocument()
   }
 
   if (mListenerManager) {
-    mListenerManager->SetListenerTarget(nsnull);
+    mListenerManager->Disconnect();
   }
 
   if (mScriptLoader) {

content/base/src/nsGenericElement.h

@@ -210,7 +210,7 @@ public:
   ~EventListenerManagerMapEntry()
   {
     if (mListenerManager) {
-      mListenerManager->SetListenerTarget(nsnull);
+      mListenerManager->Disconnect();
     }
   }
 

content/events/public/nsIEventListenerManager.h

@@ -53,9 +53,10 @@ struct JSObject;
 /*
  * Event listener manager interface.
  */
+// dbb34a55-276d-4105-a26a-401bbb3e60c3
 #define NS_IEVENTLISTENERMANAGER_IID \
-{ 0xec4df50f, 0x3f1d, 0x479c, \
-  { 0x80, 0xad, 0x9c, 0x0e, 0x4b, 0x90, 0x61, 0x57 } }
+{ 0xdbb34a55, 0x276d, 0x4105, \
+  { 0xa2, 0x6a, 0x40, 0x1b, 0xbb, 0x3e, 0x60, 0xc3 } }
 
 
 class nsIEventListenerManager : public nsISupports {
@@ -166,14 +167,24 @@ public:
   NS_IMETHOD ReleaseEvent(PRInt32 aEventTypes) = 0;
 
   /**
-  * Removes all event listeners registered by this instance of the listener
-  * manager.
+  * Tells the event listener manager that its target (which owns it) is
+  * no longer using it (and could go away).
+  *
+  * This causes the removal of all event listeners registered by this
+  * instance of the listener manager.  This is important for Bug 323807,
+  * since nsDOMClassInfo::PreserveWrapper (and nsIDOMGCParticipant)
+  * require that we remove all event listeners to remove any weak
+  * references in the nsDOMClassInfo's preserved wrapper table to the
+  * target.
+  *
+  * It also clears the weak pointer set by the call to
+  * |SetListenerTarget|.
   */
-  NS_IMETHOD RemoveAllListeners() = 0;
+  NS_IMETHOD Disconnect() = 0;
 
   /**
-  * Removes all event listeners registered by this instance of the listener
-  * manager.
+  * Tells the event listener manager what its target is.  This must be
+  * followed by a call to |Disconnect| before the target is destroyed.
   */
   NS_IMETHOD SetListenerTarget(nsISupports* aTarget) = 0;
 

content/events/src/nsEventListenerManager.cpp

@@ -2013,10 +2013,24 @@ nsEventListenerManager::FlipCaptureBit(PRInt32 aEventTypes,
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsEventListenerManager::Disconnect()
+{
+  mTarget = nsnull;
+
+  // Bug 323807: nsDOMClassInfo::PreserveWrapper (and
+  // nsIDOMGCParticipant) require that we remove all event listeners now
+  // to remove any weak references in the nsDOMClassInfo's preserved
+  // wrapper table to the target.
+  RemoveAllListeners();
+}
+
 NS_IMETHODIMP
 nsEventListenerManager::SetListenerTarget(nsISupports* aTarget)
 {
-  //WEAK reference, must be set back to nsnull when done
+  NS_PRECONDITION(aTarget, "unexpected null pointer");
+
+  //WEAK reference, must be set back to nsnull when done by calling Disconnect
   mTarget = aTarget;
   return NS_OK;
 }

content/events/src/nsEventListenerManager.h

@@ -168,7 +168,7 @@ public:
                          const nsAString& aEventType,
                          nsIDOMEvent** aDOMEvent);
 
-  NS_IMETHOD RemoveAllListeners();
+  NS_IMETHOD Disconnect();
 
   NS_IMETHOD SetListenerTarget(nsISupports* aTarget);
 
@@ -236,6 +236,7 @@ protected:
                                PRInt32 aFlags,
                                nsIDOMEventGroup* aEvtGrp);
   void ReleaseListeners(nsVoidArray** aListeners);
+  nsresult RemoveAllListeners();
   nsresult FlipCaptureBit(PRInt32 aEventTypes, PRBool aInitCapture);
   nsVoidArray* GetListenersByType(EventArrayType aType, nsHashKey* aKey, PRBool aCreate);
   EventArrayType GetTypeForIID(const nsIID& aIID);

content/xul/content/src/nsXULElement.cpp

@@ -939,7 +939,7 @@ nsXULElement::UnbindFromTree(PRBool aDeep, PRBool aNullParent)
     // XXXbz why are we nuking our listener manager?  We can get events while
     // not in a document!
     if (mListenerManager) {
-        mListenerManager->SetListenerTarget(nsnull);
+        mListenerManager->Disconnect();
         mListenerManager = nsnull;
     }
 

dom/src/base/nsDOMClassInfo.h

@@ -180,6 +180,10 @@ public:
    *
    * A preservation with a given key overwrites any previous
    * preservation with that key.
+   *
+   * No strong references are held as a result of this function call, so
+   * the caller is responsible for calling |ReleaseWrapper| sometime
+   * before |aParticipant|'s destructor runs.
    */
   static nsresult PreserveWrapper(void* aKey,
                                   nsIXPConnectJSObjectHolder* (*aKeyToWrapperFunc)(void* aKey),
@@ -190,6 +194,9 @@ public:
    * Easier way to call the above just for DOM nodes (and better, since
    * we get the performance benefits of having the same identity function).
    * The call to |PreserveWrapper| is made with |aKey| == |aWrapper|.
+   *
+   * The caller need not call |ReleaseWrapper| since the node's
+   * wrapper's scriptable helper does so in its finalize callback.
    */
   static nsresult PreserveNodeWrapper(nsIXPConnectWrappedNative *aWrapper);
 

dom/src/base/nsGlobalWindow.cpp

@@ -377,6 +377,11 @@ nsGlobalWindow::~nsGlobalWindow()
       PR_REMOVE_AND_INIT_LINK(w);
     }
   } else {
+    if (mListenerManager) {
+      mListenerManager->Disconnect();
+      mListenerManager = nsnull;
+    }
+
     // An inner window is destroyed, pull it out of the outer window's
     // list if inner windows.
 
@@ -474,7 +479,7 @@ nsGlobalWindow::FreeInnerObjects(JSContext *cx)
   mChromeEventHandler = nsnull;
 
   if (mListenerManager) {
-    mListenerManager->RemoveAllListeners();
+    mListenerManager->Disconnect();
     mListenerManager = nsnull;
   }
 
@@ -1002,7 +1007,7 @@ nsGlobalWindow::SetNewDocument(nsIDocument* aDocument,
       }
 
       if (!reUseInnerWindow && currentInner->mListenerManager) {
-        currentInner->mListenerManager->RemoveAllListeners();
+        currentInner->mListenerManager->Disconnect();
         currentInner->mListenerManager = nsnull;
       }
 

dom/src/base/nsWindowRoot.cpp

@@ -72,6 +72,9 @@ nsWindowRoot::nsWindowRoot(nsIDOMWindow* aWindow)
 
 nsWindowRoot::~nsWindowRoot()
 {
+  if (mListenerManager) {
+    mListenerManager->Disconnect();
+  }
 }
 
 NS_INTERFACE_MAP_BEGIN(nsWindowRoot)