Bug #19166 --> pull a 180 and reverse the ref counting

model for url listeners. The manager now uses an
nsIsupportsArray to keep references to all of the url listeners.
However, in order to prevent circular references (which is
why I was using a nsVoidArray before), when we issue the on stop
call to a url listener, we automatically release our ref
to that object.
r=rhp

mailnews/base/src/nsUrlListenerManager.cpp

@@ -29,20 +29,24 @@ nsUrlListenerManager::nsUrlListenerManager() :
 {
 	NS_INIT_REFCNT();
 	// create a new isupports array to store our listeners in...
-	m_listeners = new nsVoidArray();
+  NS_NewISupportsArray(getter_AddRefs(m_listeners));
 }
 
 nsUrlListenerManager::~nsUrlListenerManager()
 {
-	if (m_listeners)
+  ReleaseListeners();
+}
+
+void nsUrlListenerManager::ReleaseListeners()
+{
+  if(m_listeners)
 	{
-		PRUint32 count = m_listeners->Count();
+		PRUint32 count;
+    nsresult rv = m_listeners->Count(&count);
 
 		for (int i = count - 1; i >= 0; i--)
 			m_listeners->RemoveElementAt(i);
 	}
-
-	delete m_listeners;
 }
 
 NS_IMPL_THREADSAFE_ISUPPORTS(nsUrlListenerManager, nsCOMTypeInfo<nsIUrlListenerManager>::GetIID());
@@ -50,7 +54,7 @@ NS_IMPL_THREADSAFE_ISUPPORTS(nsUrlListenerManager, nsCOMTypeInfo<nsIUrlListenerM
 nsresult nsUrlListenerManager::RegisterListener(nsIUrlListener * aUrlListener)
 {
 	if (m_listeners && aUrlListener)
-		m_listeners->AppendElement((void *) aUrlListener);
+		m_listeners->AppendElement(aUrlListener);
 
 	return NS_OK;
 }
@@ -58,7 +62,7 @@ nsresult nsUrlListenerManager::RegisterListener(nsIUrlListener * aUrlListener)
 nsresult nsUrlListenerManager::UnRegisterListener(nsIUrlListener * aUrlListener)
 {
 	if (m_listeners && aUrlListener)
-		m_listeners->RemoveElement((void *) aUrlListener);
+		m_listeners->RemoveElement(aUrlListener);
 	return NS_OK;
 }
 
@@ -70,11 +74,15 @@ nsresult nsUrlListenerManager::BroadcastChange(nsIURI * aUrl, nsUrlNotifyType no
 	if (m_listeners && aUrl)
 	{
 		// enumerate over all url listeners...(Start at the end and work our way down)
-		nsIUrlListener * listener = nsnull;
-		
-		for (PRUint32 i = m_listeners->Count(); i > 0; i--)
+		nsCOMPtr<nsIUrlListener> listener;
+    nsCOMPtr<nsISupports> aSupports;
+		PRUint32 index;
+    m_listeners->Count(&index);
+		for (; index > 0; index--)
 		{
-			listener = (nsIUrlListener *) m_listeners->ElementAt(i-1);
+      m_listeners->GetElementAt(index-1, getter_AddRefs(aSupports)); 
+			listener = do_QueryInterface(aSupports);
+
 			if (listener)
 			{
 				if (notification == nsUrlNotifyStartRunning)
@@ -96,6 +104,10 @@ nsresult nsUrlListenerManager::OnStartRunningUrl(nsIMsgMailNewsUrl * aUrl)
 
 nsresult nsUrlListenerManager::OnStopRunningUrl(nsIMsgMailNewsUrl * aUrl, nsresult aErrorCode)
 {
-	return BroadcastChange(aUrl, nsUrlNotifyStopRunning, aErrorCode);
+	nsresult rv = BroadcastChange(aUrl, nsUrlNotifyStopRunning, aErrorCode);
+  // in order to prevent circular references, after we issue on stop running url, 
+  // go through and release all of our listeners...
+  ReleaseListeners();
+  return rv;
 }
 

mailnews/base/src/nsUrlListenerManager.h

@@ -24,7 +24,7 @@
 #define nsUrlListenerManager_h___
 
 #include "nsIUrlListenerManager.h"
-#include "nsVoidArray.h"
+#include "nsISupportsArray.h"
 #include "nsCOMPtr.h"
 
 /********************************************************************************************
@@ -34,7 +34,7 @@
 	url listener register/unregister calls are forwarded to the listener manager. In addition,
 	the url listener manager handles broadcasting of event changes on the url.
 
-    mscott --> hmm now that I think about it this class is probably going to have to be made
+  mscott --> hmm now that I think about it this class is probably going to have to be made
 	thread safe. It might have to proxy notification calls into another thread....
  ********************************************************************************************/
 
@@ -51,10 +51,27 @@ public:
 	virtual ~nsUrlListenerManager();
     
 protected:
-	nsVoidArray * m_listeners;
+  // mscott --> for the longest time, I had m_listeners as a nsVoidArray to prevent
+  // circular references when the url listener owned the url which owned the manager
+  // which owned the url listener. By using a void array, were the manager didn't own
+  // the listeners, we got out of this problem. But this made the model very difficult
+  // to use for folks when it came to maintenance. Why? Because they would have a url
+  // listener which didn't own the url. And they had no way of knowing how to keep
+  // their object alive until the listener manager said the url was done. As a result,
+  // folks were using strange ref counting techniques on their objects such that they 
+  // would addref themselves before adding their listener to the url. Then, when they
+  // received a on stop running url, they would throw a random release in there.
+  // Needless to say, this is far from ideal so I've decided to change this to 
+  // an nsISupportsArray and cleanup the caller's ref counting hacks to get this to work.
+  // The danger is of course still the circular reference problem. In order to get around
+  // this, when we issue a on stop running url through the manager, I'm going to release
+  // all our url listeners. This should break the circle.
+	// nsVoidArray * m_listeners;
+  nsCOMPtr<nsISupportsArray> m_listeners;
 
 	// helper function used to enumerate ISupportsArray and broadcast the change
 	nsresult BroadcastChange(nsIURI * aUrl, nsUrlNotifyType notification, nsresult aErrorCode);
+  void ReleaseListeners();
 };
 
 #endif /* nsUrlListenerManager_h___ */