Change a few more destructors in XPCOM to be non-virtual, and comment destructors that need to be virtual. Bug 229875, r=dougt, sr=dbaron.

git-svn-id: svn://10.0.0.236/trunk@152655 18797224-902f-48f8-a5cc-f745e15eee43

mozilla/xpcom/io/nsBinaryStream.h

@@ -45,6 +45,7 @@ class nsBinaryOutputStream : public nsIObjectOutputStream
 {
 public:
     nsBinaryOutputStream() {};
+    // virtual dtor since subclasses call our Release()
     virtual ~nsBinaryOutputStream() {};
 
 protected:
@@ -84,6 +85,7 @@ class nsBinaryInputStream : public nsIObjectInputStream
 {
 public:
     nsBinaryInputStream() {};
+    // virtual dtor since subclasses call our Release()
     virtual ~nsBinaryInputStream() {};
 
 protected:

mozilla/xpcom/io/nsFastLoadService.h

@@ -50,8 +50,10 @@ class nsFastLoadService : public nsIFastLoadService
 {
   public:
     nsFastLoadService();
-    virtual ~nsFastLoadService();
+  private:
+    ~nsFastLoadService();
 
+  public:
     NS_DECL_ISUPPORTS
     NS_DECL_NSIFASTLOADSERVICE
 

mozilla/xpcom/io/nsLocalFileWin.cpp

@@ -70,7 +70,8 @@ class ShortcutResolver
 {
 public:
     ShortcutResolver();
-    virtual ~ShortcutResolver();
+    // nonvirtual since we're not subclassed
+    ~ShortcutResolver();
 
     nsresult Init();
     nsresult Resolve(const WCHAR* in, char* out);
@@ -377,7 +378,9 @@ class nsDirEnumerator : public nsISimpleEnumerator
             return NS_OK;
         }
 
-        virtual ~nsDirEnumerator()
+        // dtor can be non-virtual since there are no subclasses, but must be
+        // public to use the class on the stack.
+        ~nsDirEnumerator()
         {
             if (mDir)
             {

mozilla/xpcom/io/nsPipe3.cpp

@@ -126,7 +126,6 @@ public:
         , mAvailable(0)
         , mCallbackFlags(0)
         { }
-    virtual ~nsPipeInputStream() { }
 
     nsresult Fill();
     void SetNonBlocking(PRBool aNonBlocking) { mBlocking = !aNonBlocking; }
@@ -184,7 +183,6 @@ public:
         , mWritable(PR_TRUE)
         , mCallbackFlags(0)
         { }
-    virtual ~nsPipeOutputStream() { }
 
     void SetNonBlocking(PRBool aNonBlocking) { mBlocking = !aNonBlocking; }
     void SetWritable(PRBool writable) { mWritable = writable; }
@@ -225,8 +223,11 @@ public:
 
     // nsPipe methods:
     nsPipe();
-    virtual ~nsPipe();
 
+private:
+    ~nsPipe();
+
+public:
     //
     // methods below may only be called while inside the pipe's monitor
     //
@@ -1261,7 +1262,8 @@ NS_NewPipe2(nsIAsyncInputStream **pipeIn,
                     segmentCount,
                     segmentAlloc);
     if (NS_FAILED(rv)) {
-        delete pipe;
+        NS_ADDREF(pipe);
+        NS_RELEASE(pipe);
         return rv;
     }
 

mozilla/xpcom/io/nsStreamUtils.cpp

@@ -272,6 +272,7 @@ public:
     {
     }
 
+    // virtual since subclasses call superclass Release()
     virtual ~nsAStreamCopier()
     {
         if (mLock)

mozilla/xpcom/io/nsStringStream.cpp

@@ -87,12 +87,14 @@ public:
         , mLength(0)
     {}
     
-    virtual ~nsStringInputStream()
+private:
+    ~nsStringInputStream()
     {
         if (mOwned)
             nsMemory::Free((char*)mConstString);
     }
 
+public:
     NS_DECL_ISUPPORTS
 
     NS_DECL_NSISTRINGINPUTSTREAM
@@ -331,14 +333,16 @@ NS_NewStringInputStream(nsIInputStream** aStreamResult,
         return NS_ERROR_OUT_OF_MEMORY;
     }
 
+    NS_ADDREF(stream);
+
     nsresult rv = stream->AdoptData(data, aStringToRead.Length());
     if (NS_FAILED(rv)) {
         nsMemory::Free(data);
-        delete stream;
+        NS_RELEASE(stream);
         return rv;
     }
     
-    NS_ADDREF(*aStreamResult = stream);
+    *aStreamResult = stream;
     return NS_OK;
 }
 
@@ -360,14 +364,16 @@ NS_NewCStringInputStream(nsIInputStream** aStreamResult,
         return NS_ERROR_OUT_OF_MEMORY;
     }
 
+    NS_ADDREF(stream);
+
     nsresult rv = stream->AdoptData(data, aStringToRead.Length());
     if (NS_FAILED(rv)) {
         nsMemory::Free(data);
-        delete stream;
+        NS_RELEASE(stream);
         return rv;
     }
     
-    NS_ADDREF(*aStreamResult = stream);
+    *aStreamResult = stream;
     return NS_OK;
 }
 
@@ -383,14 +389,16 @@ NS_NewCharInputStream(nsIInputStream** aStreamResult,
     if (! stream)
         return NS_ERROR_OUT_OF_MEMORY;
 
+    NS_ADDREF(stream);
+
     nsresult rv = stream->ShareData(aStringToRead, -1);
     
     if (NS_FAILED(rv)) {
-        delete stream;
+        NS_RELEASE(stream);
         return rv;
     }
     
-    NS_ADDREF(*aStreamResult = stream);
+    *aStreamResult = stream;
     return NS_OK;
 }
 
@@ -407,14 +415,16 @@ NS_NewByteInputStream(nsIInputStream** aStreamResult,
     if (! stream)
         return NS_ERROR_OUT_OF_MEMORY;
 
+    NS_ADDREF(stream);
+
     nsresult rv = stream->ShareData(aStringToRead, aLength);
     
     if (NS_FAILED(rv)) {
-        delete stream;
+        NS_RELEASE(stream);
         return rv;
     }
     
-    NS_ADDREF(*aStreamResult = stream);
+    *aStreamResult = stream;
     return NS_OK;
 }
 

mozilla/xpcom/obsolete/component/nsRegistry.cpp

@@ -150,6 +150,7 @@ struct nsRegSubtreeEnumerator : public nsIRegistryEnumerator {
 
     // ctor/dtor
     nsRegSubtreeEnumerator( HREG hReg, RKEY rKey, PRBool all );
+    // virtual dtor since subclasses call our Release()
     virtual ~nsRegSubtreeEnumerator();
 
 protected:

mozilla/xpcom/proxy/tests/proxytests.cpp

@@ -66,8 +66,6 @@ class nsTestXPCFoo : public nsITestProxy
     NS_IMETHOD Test3(nsISupports *p1, nsISupports **p2);
 
     nsTestXPCFoo();
-    virtual ~nsTestXPCFoo();
-
 };
 
 nsTestXPCFoo::nsTestXPCFoo()
@@ -75,10 +73,6 @@ nsTestXPCFoo::nsTestXPCFoo()
     NS_ADDREF_THIS();
 }
 
-nsTestXPCFoo::~nsTestXPCFoo()
-{
-}
-
 NS_IMPL_ISUPPORTS1(nsTestXPCFoo, nsITestProxy)
 
 NS_IMETHODIMP nsTestXPCFoo::Test(PRInt32 p1, PRInt32 p2, PRInt32* retval)
@@ -126,8 +120,6 @@ class nsTestXPCFoo2 : public nsITestProxy
     NS_IMETHOD Test3(nsISupports *p1, nsISupports **p2);
 
     nsTestXPCFoo2();
-    virtual ~nsTestXPCFoo2();
-
 };
 
 nsTestXPCFoo2::nsTestXPCFoo2()
@@ -135,10 +127,6 @@ nsTestXPCFoo2::nsTestXPCFoo2()
     NS_ADDREF_THIS();
 }
 
-nsTestXPCFoo2::~nsTestXPCFoo2()
-{
-}
-
 NS_IMPL_THREADSAFE_ISUPPORTS1(nsTestXPCFoo2, nsITestProxy)
 
 NS_IMETHODIMP nsTestXPCFoo2::Test(PRInt32 p1, PRInt32 p2, PRInt32* retval)

mozilla/xpcom/reflect/xptcall/tests/TestXPTCInvoke.cpp

@@ -717,8 +717,11 @@ public:
     NS_IMETHOD FooMethod2(PRInt32 i);
 
     FooImpl();
-    virtual ~FooImpl();
 
+protected:
+    ~FooImpl() {}
+
+public:
     virtual const char* ImplName() = 0;
 
     int SomeData1;
@@ -733,8 +736,11 @@ public:
     NS_IMETHOD BarMethod2(PRInt32 i);
 
     BarImpl();
-    virtual ~BarImpl();
 
+protected:
+    ~BarImpl() {}
+
+public:
     virtual const char * ImplName() = 0;
 
     int SomeData1;
@@ -747,9 +753,6 @@ public:
 FooImpl::FooImpl() : Name("FooImpl")
 {
 }
-FooImpl::~FooImpl()
-{
-}
 
 NS_IMETHODIMP FooImpl::FooMethod1(PRInt32 i)
 {
@@ -770,9 +773,6 @@ NS_IMETHODIMP FooImpl::FooMethod2(PRInt32 i)
 BarImpl::BarImpl() : Name("BarImpl")
 {
 }
-BarImpl::~BarImpl()
-{
-}
 
 NS_IMETHODIMP BarImpl::BarMethod1(PRInt32 i)
 {
@@ -798,7 +798,11 @@ public:
     const char* ImplName();
 
     FooBarImpl();
-    virtual ~FooBarImpl();
+
+private:
+    ~FooBarImpl() {}
+
+public:
     const char* MyName;
 };
 
@@ -807,10 +811,6 @@ FooBarImpl::FooBarImpl() : MyName("FooBarImpl")
     NS_ADDREF_THIS();
 }
 
-FooBarImpl::~FooBarImpl()
-{
-}
-
 const char* FooBarImpl::ImplName()
 {
     return MyName;
@@ -946,8 +946,12 @@ public:
     NS_DECL_ISUPPORTS
 
     FooBarImpl2();
-    virtual ~FooBarImpl2();
-        PRInt32 value;
+
+private:
+    ~FooBarImpl2() {}
+
+public:
+    PRInt32 value;
 };
 
 FooBarImpl2::FooBarImpl2() : value(0x12345678)
@@ -955,10 +959,6 @@ FooBarImpl2::FooBarImpl2() : value(0x12345678)
     NS_ADDREF_THIS();
 }
 
-FooBarImpl2::~FooBarImpl2()
-{
-}
-
 NS_IMETHODIMP FooBarImpl2::FooMethod1(PRInt32 i)
 {
     printf("\tFooBarImpl2::FooMethod1 called with i == %d, local value = %x\n", 

mozilla/xpcom/string/public/nsBufferHandle.h

@@ -348,10 +348,7 @@ class nsSharedBufferHandleWithDestroy
 
       virtual void Destroy() = 0;
 
-      // This doesn't really need to be |virtual|, but it saves us from
-      // having to turn off gcc warnings that might be useful to
-      // someone.
-      virtual ~nsSharedBufferHandleWithDestroy() { }
+      ~nsSharedBufferHandleWithDestroy() { }
 
 
   };

mozilla/xpcom/string/public/nsSharedBufferList.h

@@ -162,7 +162,11 @@ class NS_COM nsSharedBufferList
             }
         }
 
-      virtual ~nsSharedBufferList();
+      // No virtual destructor is required here.  |nsSharedBufferList| is
+      // used only as a member variable, and |nsSlidingSharedBufferList|s are
+      // never deleted via a base class pointer.
+
+      ~nsSharedBufferList();
 
     private:
         // pass-by-value is explicitly denied

mozilla/xpcom/tests/TestAutoPtr.cpp

@@ -27,12 +27,14 @@
 
 class TestObjectBaseA {
     public:
+        // Virtual dtor for deleting through base class pointer
         virtual ~TestObjectBaseA() { };
         int fooA;
 };
 
 class TestObjectBaseB {
     public:
+        // Virtual dtor for deleting through base class pointer
         virtual ~TestObjectBaseB() { };
         int fooB;
 };
@@ -45,6 +47,7 @@ class TestObject : public TestObjectBaseA, public TestObjectBaseB {
                    NS_STATIC_CAST(void*, this));
         }
 
+        // Virtual dtor for deleting through base class pointer
         virtual ~TestObject()
         {
             printf("  Destroying TestObject %p.\n",

mozilla/xpcom/tests/TestCOMPtr.cpp

@@ -61,6 +61,7 @@ class IFoo : public nsISupports
 
 		public:
       IFoo();
+      // virtual dtor because IBar uses our Release()
       virtual ~IFoo();
 
       NS_IMETHOD_(nsrefcnt) AddRef();