Closed Bug 1032800 Opened 10 years ago Closed 10 years ago

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

Categories

(SeaMonkey :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.30

People

(Reporter: philip.chee, Assigned: philip.chee)

References

Details

Attachments

(1 file, 1 obsolete file)

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)
Attached patch Patch v1.0 Proposed fix. (obsolete) — Splinter Review
>  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
Attachment #8448716 - Flags: review?(neil)
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.
(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.
>>+    ~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.
Attachment #8448716 - Attachment is obsolete: true
Attachment #8448716 - Flags: review?(neil)
Attachment #8449629 - Flags: review?(neil)
Comment on attachment 8449629 [details] [diff] [review]
Patch v1.1 fix review issues.

>+  virtual ~nsMacShellService() {}
Nit: doesn't need to be virtual.
Attachment #8449629 - Flags: review?(neil) → review+
Pushed comm-central changeset c4202cb87f5d
http://hg.mozilla.org/comm-central/rev/c4202cb87f5d
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.30
Blocks: 1035650
You need to log in before you can comment on or make changes to this bug.