1032800 - Remove public destructors of NS_*_INLINE_* refcounted classes, Remove NS_HIDDEN, Fix dangerous public destructors.

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Philip Chee]

Port relevant bits from:

Bug 1015664 - Remove NS_HIDDEN usage in Gecko
Bug 1027251 - Remove public destructors of NS_*_INLINE_* refcounted classes, outside of a finite explicit whitelist
Bug 1028588 - Fix dangerous public destructors in miscellaneous places
Bug 758992 - Make the classes which use the XPCOM nsISupports implementation macros final, to avoid the warning about deleting using a pointer to a base class with virtual functions and no virtual dtor (browser parts)

Comment 1

[Philip Chee]

Attachment: Patch v1.0 Proposed fix.

>  class nsWindowsShellService : public nsIWindowsShellService
>  {
>  public:
>    nsWindowsShellService() : mCheckedThisSessionClient(false) {};
> -  ~nsWindowsShellService() {};
> -  NS_HIDDEN_(nsresult) Init();
> +  virtual ~nsWindowsShellService() {};
Is virtual necessary? Should the destructor be moved to protected:

> +  nsresult Init();

Questions: Should the dtor be moved to protected: for the following?
1. nsNetscapeProfileMigratorBase.h
2. nsThunderbirdProfileMigrator.h

Comment 2

[[e-mail]]

Comment on attachment 8448716 [details] [diff] [review]
Patch v1.0 Proposed fix.

>+  ~nsFeedSniffer() {}
Missing MOZ_FINAL on the class.

>+    ~AppendingEnumerator() {}
Missing MOZ_FINAL on the class.

> class nsWindowsShellService : public nsIWindowsShellService
Missing MOZ_FINAL on the class.

> public:
>   nsWindowsShellService() : mCheckedThisSessionClient(false) {};
>+  virtual ~nsWindowsShellService() {};
Needs to be nonpublic.

Comment 3

[[e-mail]]

(In reply to Philip Chee from comment #1)
> > +  virtual ~nsWindowsShellService() {};
> Is virtual necessary? Should the destructor be moved to protected:
No and yes (and fix the mac shell service too please).

> Questions: Should the dtor be moved to protected: for the following?
> 1. nsNetscapeProfileMigratorBase.h
> 2. nsThunderbirdProfileMigrator.h
Yes.

On a side note, the migrators are abusing the macros.

Comment 4

[Philip Chee]

Attachment: Patch v1.1 fix review issues.

>>+    ~AppendingEnumerator() {}
> Missing MOZ_FINAL on the class.
Fixed.

>> class nsWindowsShellService : public nsIWindowsShellService
> Missing MOZ_FINAL on the class.
Fixed.

>> public:
>>   nsWindowsShellService() : mCheckedThisSessionClient(false) {};
>>+  virtual ~nsWindowsShellService() {};
> Needs to be nonpublic.
Fixed.

>>> +  virtual ~nsWindowsShellService() {};
>> Is virtual necessary? Should the destructor be moved to protected:
> No and yes
Fixed. Fixed.

> (and fix the mac shell service too please).
Fixed.

>> Questions: Should the dtor be moved to protected: for the following?
>> 1. nsNetscapeProfileMigratorBase.h
>> 2. nsThunderbirdProfileMigrator.h
> Yes.
Fixed.

> On a side note, the migrators are abusing the macros.

Comment 5

[[e-mail]]

Comment on attachment 8449629 [details] [diff] [review]
Patch v1.1 fix review issues.

>+  virtual ~nsMacShellService() {}
Nit: doesn't need to be virtual.

Comment 6

[Philip Chee]

Pushed comm-central changeset c4202cb87f5d
http://hg.mozilla.org/comm-central/rev/c4202cb87f5d