1049075 - remove virtual from methods of nsGridCell

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Trevor Saunders (:tbsaunde)]

This class doesn't inherit from anything, so I don't see a good reason for it to have virtual methods.

Comment 1

[Daniel Holbert [:dholbert]]

(To be clear, it looks like this is XUL grid, not CSS grid -- http://mxr.mozilla.org/mozilla-central/source/layout/xul/grid/nsGridCell.h .)

Indeed, this class doesn't inherit, nor does anything inherit from it. So, no point in 'virtual'.

The only virtual method I see is the destructor.  We should also mark the class MOZ_FINAL, to verify that we're actually correct in thinking that nothing inherits from it. (and in the unlikely scenario that anyone adds a child class, they'll have to remove that annotation and hopefully will notice that they should re-virtualize the destructor.)

Comment 2

[Daniel Holbert [:dholbert]]

Trevor, were you intending to take this?  If so, excellent! If not, I may take it or mark it as a mentored bug.

Comment 3

[Trevor Saunders (:tbsaunde)]

(In reply to Daniel Holbert [:dholbert] from comment #2)
> Trevor, were you intending to take this?  If so, excellent! If not, I may
> take it or mark it as a mentored bug.

nah, was just filing follow ups for bug 1047696, feel free to find someone to fix it.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: fix: drop 'virtual', add MOZ_FINAL

(I ended up opting to write the patch myself; I tend to think this isn't substantial enough or exciting enough to make for a particularly good mentored bug.)

3 changes:
 - drop 'virtual' from destructor
 - Add MOZ_FINAL to make sure that dropping 'virtual' is safe
 - Add #include for Attributes.h, which provides MOZ_FINAL

Comment 5

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d8b1d2b3e762

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d8b1d2b3e762