[webkit-reviews] review denied: [Bug 130636] Remove *virtual* keyword in destructor of NavigatorContentUtils : [Attachment 227538] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Gyuyoung Kim
<gyuyoung.kim at samsung.com>'s request for review:
Bug 130636: Remove *virtual* keyword in destructor of NavigatorContentUtils
https://bugs.webkit.org/show_bug.cgi?id=130636

Attachment 227538: Patch
https://bugs.webkit.org/attachment.cgi?id=227538&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=227538&action=review


>>>> Source/WebCore/Modules/navigatorcontentutils/NavigatorContentUtils.h:46
>>>> +	  ~NavigatorContentUtils();
>>> 
>>> The destructor *is* virtual as RefCountedSupplement has a virtual
destructor. I don't think it is a good idea to remove virtual here as it makes
it less obvious the destructor really is virtual.
>> 
>> I see. Do you think *final* keyword is not good as well ? I think
NavigatorContentUtils won't become a parent class.
> 
> No, the final seems fine to me but I am not aware of the WebKit policy when
it comes to final keyword.

Yes, it’s fine to add final here and it’s wrong to remove virtual. In fact, I
would approve patches adding virtual to destructors in cases like this.

I don’t think there’s any benefit to adding final here, but it’s correct and
not harmful.