The only remaining virtual method in CSSRule is the destructor. If we make it non-virtual, each instance will shrink by one pointer (the vptr.)
Created attachment 113347 [details] Possibly a patch
Comment on attachment 113347 [details] Possibly a patch View in context: https://bugs.webkit.org/attachment.cgi?id=113347&action=review r=me with a few comments. > Source/WebCore/css/CSSImportRule.h:60 > + class ImportedStyleSheetClient : public CachedStyleSheetClient { This could use a comment explaining why the class itself can't implement the interface. > Source/WebCore/css/CSSRule.cpp:76 > +void CSSRule::deref() > +{ > + if (!derefBase()) > + return; > + switch (type()) { > + case UNKNOWN_RULE: The deletion part should be factored into a function. deref() can be inlined. > Source/WebCore/css/CSSRule.h:40 > + // Override RefCounted's deref() to ensure operator delete is called on > + // the appropriate subclass type. > + void deref(); You should explain that this class is non-virtual for a reason.
Comment on attachment 113347 [details] Possibly a patch View in context: https://bugs.webkit.org/attachment.cgi?id=113347&action=review > Source/WebCore/css/CSSRule.h:36 > + ~CSSRule() { } This base class destructor should be protected.
Maybe it would make sense to have RefCounted do deleting by calling static_cast<T*>(this)->destruct(); instead of calling delete directly (with the default implementation just doing delete)? This way you would only need to override destruct() in cases like this. We may want to devirtualize some more in the future...
(In reply to comment #4) > Maybe it would make sense to have RefCounted do deleting by calling > > static_cast<T*>(this)->destruct(); > > instead of calling delete directly (with the default implementation just doing delete)? This way you would only need to override destruct() in cases like this. We may want to devirtualize some more in the future. Seems OK. I would call the function “destroy”, not “destruct”, though.
Committed r99171: <http://trac.webkit.org/changeset/99171>