bug#16742 Fixing delete on nsISupports. Thanks for patch from <heikki@citec.fi> Plus using IMPL_ and DECL_ macros and removing operating on refcnt directly. r=dp

content/xul/content/src/nsRDFDOMNodeList.cpp

@@ -50,7 +50,7 @@ static NS_DEFINE_CID(kDOMScriptObjectFactoryCID,  NS_DOM_SCRIPT_OBJECT_FACTORY_C
 
 
 nsRDFDOMNodeList::nsRDFDOMNodeList(void)
-    : mInner(nsnull),
+    : //mInner(nsnull), Not being used?
       mElements(nsnull),
       mScriptObject(nsnull)
 {
@@ -65,7 +65,7 @@ nsRDFDOMNodeList::~nsRDFDOMNodeList(void)
 #endif
 
     NS_IF_RELEASE(mElements);
-    delete mInner;
+    //delete mInner; Not being used?
 }
 
 nsresult

content/xul/content/src/nsRDFDOMNodeList.h

@@ -31,7 +31,7 @@ class nsRDFDOMNodeList : public nsIDOMNodeList,
                          public nsIScriptObjectOwner
 {
 private:
-    nsISupports* mInner;
+    //nsISupports* mInner; Not being used?
     nsISupportsArray* mElements;
     void* mScriptObject;
 

modules/plugin/base/src/ns4xPlugin.cpp

@@ -366,21 +366,18 @@ nsresult ns4xPlugin :: CreateInstance(nsISupports *aOuter,
 
   *aResult = NULL;  
   
-  nsISupports *inst;
+  // XXX This is suspicuous!
-
+  ns4xPluginInstance *inst = new ns4xPluginInstance(&fCallbacks);
-  inst = nsnull;
-  inst = (nsISupports *)(nsIPluginInstance *)new ns4xPluginInstance(&fCallbacks);
 
   if (inst == NULL) {  
     return NS_ERROR_OUT_OF_MEMORY;  
   }  
 
+  NS_ADDREF(inst);  // Stabilize
+
   nsresult res = inst->QueryInterface(aIID, aResult);
 
-  if (res != NS_OK) {  
+  NS_RELEASE(inst); // Destabilize and avoid leaks. Avoid calling delete <interface pointer>    
-    // We didn't get the right interface, so clean up  
-    delete inst;  
-  }  
 
   return res;  
 }  

modules/plugin/base/src/ns4xPluginInstance.cpp

@@ -512,17 +512,16 @@ NS_IMETHODIMP ns4xPluginInstance::SetWindow(nsPluginWindow* window)
 
 NS_IMETHODIMP ns4xPluginInstance::NewStream(nsIPluginStreamListener** listener)
 {
-	nsISupports* stream = nsnull;
+	ns4xPluginStreamListener* stream = new ns4xPluginStreamListener(this, nsnull);
-	stream = (nsISupports *)(nsIPluginStreamListener *)new ns4xPluginStreamListener(this, nsnull);
 
 	if(stream == nsnull)
 		return NS_ERROR_OUT_OF_MEMORY;
 
+	NS_ADDREF(stream);  // Stabilize
+    
     nsresult res = stream->QueryInterface(kIPluginStreamListenerIID, (void**)listener);
 
-	// If we didn't get the right interface, clean up  
+	NS_RELEASE(stream); // Destabilize and avoid leaks. Avoid calling delete <interface pointer>  
-	if (res != NS_OK)
-		delete stream;  
 
 	return res;
 }

modules/plugin/base/src/nsPluginFactory.cpp

@@ -35,18 +35,9 @@ static NS_DEFINE_IID(kIServiceManagerIID, NS_ISERVICEMANAGER_IID);
 class nsPluginFactory : public nsIFactory
 {   
   public:   
-    // nsISupports methods   
+    NS_DECL_ISUPPORTS
-    NS_IMETHOD QueryInterface(const nsIID &aIID,    
-                                       void **aResult);   
-    NS_IMETHOD_(nsrefcnt) AddRef(void);   
-    NS_IMETHOD_(nsrefcnt) Release(void);   
 
-    // nsIFactory methods   
+    NS_DECL_NSIFACTORY
-    NS_IMETHOD CreateInstance(nsISupports *aOuter,   
-                              const nsIID &aIID,   
-                              void **aResult);   
-
-    NS_IMETHOD LockFactory(PRBool aLock);   
 
     nsPluginFactory(const nsCID &aClass, nsIServiceManager* serviceMgr);   
 
@@ -54,15 +45,15 @@ class nsPluginFactory : public nsIFactory
     virtual ~nsPluginFactory();   
 
   private:   
-    nsrefcnt  mRefCnt;   
     nsCID     mClassID;
     nsIServiceManager *mserviceMgr;
 };   
 
 nsPluginFactory :: nsPluginFactory(const nsCID &aClass, nsIServiceManager* serviceMgr)   
 {   
-  mRefCnt = 0;
+  NS_INIT_ISUPPORTS();
   mClassID = aClass;
+  // XXX Are we sure about this weak reference. -dp
   mserviceMgr = serviceMgr;
 }   
 
@@ -70,32 +61,7 @@ nsPluginFactory :: ~nsPluginFactory()
 {
 }   
 
-nsresult nsPluginFactory :: QueryInterface(const nsIID &aIID,   
+NS_IMPL_ISUPPORTS(nsPluginFactory, NS_GET_IID(nsIFactory))
-                                       void **aResult)   
-{   
-  if (aResult == NULL) {   
-    return NS_ERROR_NULL_POINTER;   
-  }   
-
-  // Always NULL result, in case of failure   
-  *aResult = NULL;   
-
-  if (aIID.Equals(kISupportsIID)) {   
-    *aResult = (void *)(nsISupports*)this;   
-  } else if (aIID.Equals(kIFactoryIID)) {   
-    *aResult = (void *)(nsIFactory*)this;   
-  }   
-
-  if (*aResult == NULL) {   
-    return NS_NOINTERFACE;   
-  }   
-
-  AddRef(); // Increase reference count for caller   
-  return NS_OK;   
-}   
-
-NS_IMPL_ADDREF(nsPluginFactory);
-NS_IMPL_RELEASE(nsPluginFactory);
 
 nsresult nsPluginFactory :: CreateInstance(nsISupports *aOuter,  
                                           const nsIID &aIID,  
@@ -105,25 +71,31 @@ nsresult nsPluginFactory :: CreateInstance(nsISupports *aOuter,
     return NS_ERROR_NULL_POINTER;  
   }  
 
+  if (aOuter) {
+    *aResult = nsnull;
+    return NS_ERROR_NO_AGGREGATION;
+  }
+
   *aResult = NULL;  
   
-  nsISupports *inst = nsnull;
+  nsPluginHostImpl *inst = nsnull;
 
   //if (mClassID.Equals(kCPluginHost) || mClassID.Equals(kCPluginManagerCID) ){
-  if (mClassID.Equals(kCPluginManagerCID) ){
+  if (mClassID.Equals(kCPluginManagerCID) ) {
-    inst = (nsISupports *)(nsIPluginManager *)new nsPluginHostImpl(mserviceMgr);
+    inst = new nsPluginHostImpl(mserviceMgr);
+  } else {
+    return NS_ERROR_NO_INTERFACE;
   }
 
   if (inst == NULL) {  
     return NS_ERROR_OUT_OF_MEMORY;  
   }  
 
+  NS_ADDREF(inst);  // Stabilize
+  
   nsresult res = inst->QueryInterface(aIID, aResult);
 
-  if (res != NS_OK) {  
+  NS_RELEASE(inst); // Destabilize and avoid leaks. Avoid calling delete <interface pointer>  
-    // We didn't get the right interface, so clean up  
-    delete inst;  
-  }  
 
   return res;  
 }  
@@ -158,3 +130,4 @@ NSGetFactory(nsISupports* serviceMgr,
 
   return (*aFactory)->QueryInterface(kIFactoryIID, (void**)aFactory);
 }
+

modules/plugin/base/src/nsPluginHostImpl.cpp

@@ -2526,12 +2526,13 @@ nsresult nsPluginHostImpl::CreateInstance(nsISupports *aOuter,
   if (inst == NULL)
     return NS_ERROR_OUT_OF_MEMORY;  
 
+  // XXX Doh, we never get here... what is going on???
+
+  NS_ADDREF(inst);  // Stabilize
+  
   nsresult res = inst->QueryInterface(aIID, aResult);
 
-  if (res != NS_OK) {  
+  NS_RELEASE(inst); // Destabilize and avoid leaks. Avoid calling delete <interface pointer>  
-    // We didn't get the right interface, so clean up  
-    delete inst;  
-  }  
 
   return res;  
 }  

modules/plugin/nglsrc/ns4xPlugin.cpp

@@ -366,21 +366,18 @@ nsresult ns4xPlugin :: CreateInstance(nsISupports *aOuter,
 
   *aResult = NULL;  
   
-  nsISupports *inst;
+  // XXX This is suspicuous!
-
+  ns4xPluginInstance *inst = new ns4xPluginInstance(&fCallbacks);
-  inst = nsnull;
-  inst = (nsISupports *)(nsIPluginInstance *)new ns4xPluginInstance(&fCallbacks);
 
   if (inst == NULL) {  
     return NS_ERROR_OUT_OF_MEMORY;  
   }  
 
+  NS_ADDREF(inst);  // Stabilize
+
   nsresult res = inst->QueryInterface(aIID, aResult);
 
-  if (res != NS_OK) {  
+  NS_RELEASE(inst); // Destabilize and avoid leaks. Avoid calling delete <interface pointer>    
-    // We didn't get the right interface, so clean up  
-    delete inst;  
-  }  
 
   return res;  
 }  

modules/plugin/nglsrc/ns4xPluginInstance.cpp

@@ -512,17 +512,16 @@ NS_IMETHODIMP ns4xPluginInstance::SetWindow(nsPluginWindow* window)
 
 NS_IMETHODIMP ns4xPluginInstance::NewStream(nsIPluginStreamListener** listener)
 {
-	nsISupports* stream = nsnull;
+	ns4xPluginStreamListener* stream = new ns4xPluginStreamListener(this, nsnull);
-	stream = (nsISupports *)(nsIPluginStreamListener *)new ns4xPluginStreamListener(this, nsnull);
 
 	if(stream == nsnull)
 		return NS_ERROR_OUT_OF_MEMORY;
 
+	NS_ADDREF(stream);  // Stabilize
+    
     nsresult res = stream->QueryInterface(kIPluginStreamListenerIID, (void**)listener);
 
-	// If we didn't get the right interface, clean up  
+	NS_RELEASE(stream); // Destabilize and avoid leaks. Avoid calling delete <interface pointer>  
-	if (res != NS_OK)
-		delete stream;  
 
 	return res;
 }

modules/plugin/nglsrc/nsPluginFactory.cpp

@@ -35,18 +35,9 @@ static NS_DEFINE_IID(kIServiceManagerIID, NS_ISERVICEMANAGER_IID);
 class nsPluginFactory : public nsIFactory
 {   
   public:   
-    // nsISupports methods   
+    NS_DECL_ISUPPORTS
-    NS_IMETHOD QueryInterface(const nsIID &aIID,    
-                                       void **aResult);   
-    NS_IMETHOD_(nsrefcnt) AddRef(void);   
-    NS_IMETHOD_(nsrefcnt) Release(void);   
 
-    // nsIFactory methods   
+    NS_DECL_NSIFACTORY
-    NS_IMETHOD CreateInstance(nsISupports *aOuter,   
-                              const nsIID &aIID,   
-                              void **aResult);   
-
-    NS_IMETHOD LockFactory(PRBool aLock);   
 
     nsPluginFactory(const nsCID &aClass, nsIServiceManager* serviceMgr);   
 
@@ -54,15 +45,15 @@ class nsPluginFactory : public nsIFactory
     virtual ~nsPluginFactory();   
 
   private:   
-    nsrefcnt  mRefCnt;   
     nsCID     mClassID;
     nsIServiceManager *mserviceMgr;
 };   
 
 nsPluginFactory :: nsPluginFactory(const nsCID &aClass, nsIServiceManager* serviceMgr)   
 {   
-  mRefCnt = 0;
+  NS_INIT_ISUPPORTS();
   mClassID = aClass;
+  // XXX Are we sure about this weak reference. -dp
   mserviceMgr = serviceMgr;
 }   
 
@@ -70,32 +61,7 @@ nsPluginFactory :: ~nsPluginFactory()
 {
 }   
 
-nsresult nsPluginFactory :: QueryInterface(const nsIID &aIID,   
+NS_IMPL_ISUPPORTS(nsPluginFactory, NS_GET_IID(nsIFactory))
-                                       void **aResult)   
-{   
-  if (aResult == NULL) {   
-    return NS_ERROR_NULL_POINTER;   
-  }   
-
-  // Always NULL result, in case of failure   
-  *aResult = NULL;   
-
-  if (aIID.Equals(kISupportsIID)) {   
-    *aResult = (void *)(nsISupports*)this;   
-  } else if (aIID.Equals(kIFactoryIID)) {   
-    *aResult = (void *)(nsIFactory*)this;   
-  }   
-
-  if (*aResult == NULL) {   
-    return NS_NOINTERFACE;   
-  }   
-
-  AddRef(); // Increase reference count for caller   
-  return NS_OK;   
-}   
-
-NS_IMPL_ADDREF(nsPluginFactory);
-NS_IMPL_RELEASE(nsPluginFactory);
 
 nsresult nsPluginFactory :: CreateInstance(nsISupports *aOuter,  
                                           const nsIID &aIID,  
@@ -105,25 +71,31 @@ nsresult nsPluginFactory :: CreateInstance(nsISupports *aOuter,
     return NS_ERROR_NULL_POINTER;  
   }  
 
+  if (aOuter) {
+    *aResult = nsnull;
+    return NS_ERROR_NO_AGGREGATION;
+  }
+
   *aResult = NULL;  
   
-  nsISupports *inst = nsnull;
+  nsPluginHostImpl *inst = nsnull;
 
   //if (mClassID.Equals(kCPluginHost) || mClassID.Equals(kCPluginManagerCID) ){
-  if (mClassID.Equals(kCPluginManagerCID) ){
+  if (mClassID.Equals(kCPluginManagerCID) ) {
-    inst = (nsISupports *)(nsIPluginManager *)new nsPluginHostImpl(mserviceMgr);
+    inst = new nsPluginHostImpl(mserviceMgr);
+  } else {
+    return NS_ERROR_NO_INTERFACE;
   }
 
   if (inst == NULL) {  
     return NS_ERROR_OUT_OF_MEMORY;  
   }  
 
+  NS_ADDREF(inst);  // Stabilize
+  
   nsresult res = inst->QueryInterface(aIID, aResult);
 
-  if (res != NS_OK) {  
+  NS_RELEASE(inst); // Destabilize and avoid leaks. Avoid calling delete <interface pointer>  
-    // We didn't get the right interface, so clean up  
-    delete inst;  
-  }  
 
   return res;  
 }  
@@ -158,3 +130,4 @@ NSGetFactory(nsISupports* serviceMgr,
 
   return (*aFactory)->QueryInterface(kIFactoryIID, (void**)aFactory);
 }
+

modules/plugin/nglsrc/nsPluginHostImpl.cpp

@@ -2526,12 +2526,13 @@ nsresult nsPluginHostImpl::CreateInstance(nsISupports *aOuter,
   if (inst == NULL)
     return NS_ERROR_OUT_OF_MEMORY;  
 
+  // XXX Doh, we never get here... what is going on???
+
+  NS_ADDREF(inst);  // Stabilize
+  
   nsresult res = inst->QueryInterface(aIID, aResult);
 
-  if (res != NS_OK) {  
+  NS_RELEASE(inst); // Destabilize and avoid leaks. Avoid calling delete <interface pointer>  
-    // We didn't get the right interface, so clean up  
-    delete inst;  
-  }  
 
   return res;  
 }  

rdf/content/src/nsRDFDOMNodeList.cpp

@@ -50,7 +50,7 @@ static NS_DEFINE_CID(kDOMScriptObjectFactoryCID,  NS_DOM_SCRIPT_OBJECT_FACTORY_C
 
 
 nsRDFDOMNodeList::nsRDFDOMNodeList(void)
-    : mInner(nsnull),
+    : //mInner(nsnull), Not being used?
       mElements(nsnull),
       mScriptObject(nsnull)
 {
@@ -65,7 +65,7 @@ nsRDFDOMNodeList::~nsRDFDOMNodeList(void)
 #endif
 
     NS_IF_RELEASE(mElements);
-    delete mInner;
+    //delete mInner; Not being used?
 }
 
 nsresult

rdf/content/src/nsRDFDOMNodeList.h

@@ -31,7 +31,7 @@ class nsRDFDOMNodeList : public nsIDOMNodeList,
                          public nsIScriptObjectOwner
 {
 private:
-    nsISupports* mInner;
+    //nsISupports* mInner; Not being used?
     nsISupportsArray* mElements;
     void* mScriptObject;
 

widget/src/build/nsWinWidgetFactory.cpp

@@ -506,17 +506,11 @@ nsresult nsWidgetFactory::CreateInstance( nsISupports* aOuter,
 
 
 
+    NS_ADDREF(inst);  // Stabilize
+    
     nsresult res = inst->QueryInterface(aIID, aResult);
 
-
+    NS_RELEASE(inst); // Destabilize and avoid leaks. Avoid calling delete <interface pointer>  
-
-    if (res != NS_OK) {  
-
-        // We didn't get the right interface, so clean up  
-
-        delete inst;  
-
-    }
 
 
 

xpcom/proxy/src/nsProxyObjectManager.cpp

@@ -414,8 +414,7 @@ NSGetFactory(nsISupports* aServMgr,
     }
 
     *aFactory = nsnull;
-    nsISupports *inst;
+    nsProxyEventFactory *inst = nsnull;
-
     
     if (aClass.Equals(kProxyObjectManagerCID) )
         inst = new nsProxyEventFactory();
@@ -425,12 +424,11 @@ NSGetFactory(nsISupports* aServMgr,
     if (inst == nsnull)
         return NS_ERROR_OUT_OF_MEMORY;
 
-    nsresult res = inst->QueryInterface(nsIFactory::GetIID(), (void**) aFactory);
+    NS_ADDREF(inst);    // Stabilize
 
-    if (NS_FAILED(res)) 
+    nsresult res = inst->QueryInterface(NS_GET_IID(nsIFactory), (void**) aFactory);
-    {   
+
-        delete inst;
+    NS_RELEASE(inst);   // Destabilize and avoid leaks. Note we also avoid delete <interface pointer>.
-    }
 
     return res;
 }

xpfe/AppCores/src/nsAppCores.cpp

@@ -104,12 +104,11 @@ NSGetFactory(nsISupports* serviceMgr,
     }
 
 
+    NS_ADDREF(inst);  // Stabilize
+    
     nsresult res = inst->QueryInterface(kIFactoryIID, (void**) aFactory);
 
-    if (res != NS_OK)
+    NS_RELEASE(inst); // Destabilize and avoid leaks. Avoid calling delete <interface pointer>    
-    {   
-        delete inst;
-    }
 
   return res;