RESOLVED FIXED 119982
Make Element::attach non-virtual 
https://bugs.webkit.org/show_bug.cgi?id=119982
Summary Make Element::attach non-virtual
Antti Koivisto
Reported 2013-08-18 09:06:59 PDT
This will be helpful for future refactorings.
Attachments
le patch (62.51 KB, patch)
2013-08-18 11:23 PDT, Antti Koivisto 		kling: review+
DetailsFormatted DiffDiff
fix lists (62.51 KB, patch)
2013-08-18 12:23 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Antti Koivisto
Comment 1 2013-08-18 11:23:49 PDT
Created attachment 209039 [details] le patch
Andreas Kling
Comment 2 2013-08-18 11:35:53 PDT
Comment on attachment 209039 [details] le patch r=me
Antti Koivisto
Comment 3 2013-08-18 12:23:20 PDT
Created attachment 209040 [details] fix lists
Andreas Kling
Comment 4 2013-08-18 12:28:02 PDT
Two thoughts about this that we already discussed on IRC, crystallizing here: 1. We should make the has-custom-style-resolve-callbacks bit part of the node creation flag-mask. 2. It might be good to always call the base class willAttachRenderers()/didAttachRenderers() for consistency. As long as "useless" base class implementations are inline, there will be zero overhead.
Antti Koivisto
Comment 5 2013-08-18 12:35:13 PDT
(In reply to comment #4) > Two thoughts about this that we already discussed on IRC, crystallizing here: > > 1. We should make the has-custom-style-resolve-callbacks bit part of the node creation flag-mask. This is a good idea. > 2. It might be good to always call the base class willAttachRenderers()/didAttachRenderers() for consistency. As long as "useless" base class implementations are inline, there will be zero overhead. I'm not so sure about this. The call is unnecessary in most cases and calling no-op functions for "consistency" feels strange. willAttachRenderers() and pals are currently private so calling them unnecessarily will fail. Easy to test.
Antti Koivisto
Comment 6 2013-08-18 12:37:04 PDT
private in Element I mean.
Antti Koivisto
Comment 7 2013-08-18 13:00:48 PDT
https://trac.webkit.org/r154254
Note You need to log in before you can comment on or make changes to this bug.