relanding Bug 399022, Leak-prone code in nsCertTree r=bzbarsky, a=sayrer

security/manager/ssl/src/nsCertTree.cpp

@@ -291,11 +291,11 @@ nsCertTree::GetThreadDescAtIndex(PRInt32 index)
 //  GetCertAtIndex
 //
 //  If the row at index is a cert, return that cert.  Otherwise, return null.
-nsIX509Cert *
+already_AddRefed<nsIX509Cert>
 nsCertTree::GetCertAtIndex(PRInt32 index, PRInt32 *outAbsoluteCertOffset)
 {
   nsRefPtr<nsCertTreeDispInfo> certdi =
-    getter_AddRefs(GetDispInfoAtIndex(index, outAbsoluteCertOffset));
+    GetDispInfoAtIndex(index, outAbsoluteCertOffset);
   if (!certdi)
     return nsnull;
 
@@ -308,7 +308,7 @@ nsCertTree::GetCertAtIndex(PRInt32 index, PRInt32 *outAbsoluteCertOffset)
 }
 
 //  If the row at index is a cert, return that cert.  Otherwise, return null.
-nsCertTreeDispInfo *
+already_AddRefed<nsCertTreeDispInfo>
 nsCertTree::GetDispInfoAtIndex(PRInt32 index, 
                                PRInt32 *outAbsoluteCertOffset)
 {
@@ -606,7 +606,6 @@ nsCertTree::GetCertsByTypeFromCertList(CERTCertList *aCertList,
         certdi->mTypeOfEntry = nsCertTreeDispInfo::direct_db;
         // not necessary: certdi->mHostWithPort.Clear();
         certdi->mOverrideBits = nsCertOverride::ob_None;
-        NS_IF_ADDREF(certdi);
         mDispInfo.InsertElementAt(InsertPosition, certdi);
         ++count;
         ++InsertPosition;
@@ -834,10 +833,7 @@ nsCertTree::DeleteEntryObject(PRUint32 index)
         }
       }
 
-      nsCertTreeDispInfo *certdi2 = mDispInfo.ElementAt(certIndex);
       mDispInfo.RemoveElementAt(certIndex);
-      NS_IF_RELEASE(certdi2);
-      certdi2 = 0;
 
       if (canRemoveEntry) {
         RemoveCacheEntry(cert);
@@ -868,7 +864,7 @@ NS_IMETHODIMP
 nsCertTree::GetCert(PRUint32 aIndex, nsIX509Cert **_cert)
 {
   NS_ENSURE_ARG(_cert);
-  *_cert = GetCertAtIndex(aIndex);
+  *_cert = GetCertAtIndex(aIndex).get();
   return NS_OK;
 }
 
@@ -878,7 +874,7 @@ nsCertTree::GetTreeItem(PRUint32 aIndex, nsICertTreeItem **_treeitem)
   NS_ENSURE_ARG(_treeitem);
 
   nsRefPtr<nsCertTreeDispInfo> certdi = 
-    getter_AddRefs(GetDispInfoAtIndex(aIndex));
+    GetDispInfoAtIndex(aIndex);
   if (!certdi)
     return NS_ERROR_FAILURE;
 
@@ -893,7 +889,7 @@ nsCertTree::IsHostPortOverride(PRUint32 aIndex, PRBool *_retval)
   NS_ENSURE_ARG(_retval);
 
   nsRefPtr<nsCertTreeDispInfo> certdi = 
-    getter_AddRefs(GetDispInfoAtIndex(aIndex));
+    GetDispInfoAtIndex(aIndex);
   if (!certdi)
     return NS_ERROR_FAILURE;
 
@@ -1114,7 +1110,7 @@ nsCertTree::GetCellText(PRInt32 row, nsITreeColumn* col,
 
   PRInt32 absoluteCertOffset;
   nsRefPtr<nsCertTreeDispInfo> certdi = 
-    getter_AddRefs(GetDispInfoAtIndex(row, &absoluteCertOffset));
+    GetDispInfoAtIndex(row, &absoluteCertOffset);
   if (!certdi)
     return NS_ERROR_FAILURE;
 
@@ -1396,7 +1392,7 @@ nsCertTree::dumpMap()
       nsAutoString td(el->orgName);
       PR_LOG(gPIPNSSLog, PR_LOG_DEBUG, ("thread desc[%d]: %s", i, NS_LossyConvertUTF16toASCII(td).get()));
     }
-    nsCOMPtr<nsIX509Cert> ct = getter_AddRefs(GetCertAtIndex(i));
+    nsCOMPtr<nsIX509Cert> ct = GetCertAtIndex(i);
     if (ct != nsnull) {
       PRUnichar *goo;
       ct->GetCommonName(&goo);

security/manager/ssl/src/nsCertTree.h

@@ -154,9 +154,10 @@ private:
   nsCOMPtr<nsICertOverrideService> mOverrideService;
 
   treeArrayEl *GetThreadDescAtIndex(PRInt32 _index);
-  nsIX509Cert *GetCertAtIndex(PRInt32 _index, PRInt32 *outAbsoluteCertOffset = nsnull);
-  nsCertTreeDispInfo *GetDispInfoAtIndex(PRInt32 index,  
-                                         PRInt32 *outAbsoluteCertOffset = nsnull);
+  already_AddRefed<nsIX509Cert> 
+    GetCertAtIndex(PRInt32 _index, PRInt32 *outAbsoluteCertOffset = nsnull);
+  already_AddRefed<nsCertTreeDispInfo> 
+    GetDispInfoAtIndex(PRInt32 index, PRInt32 *outAbsoluteCertOffset = nsnull);
   void FreeCertArray();
   nsresult UpdateUIContents();