Bug 119982 - Make Element::attach non-virtual
Summary: Make Element::attach non-virtual
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-18 09:06 PDT by Antti Koivisto
Modified: 2013-08-18 13:00 PDT (History)
1 user (show)

See Also:

Attachments
le patch (62.51 KB, patch)
2013-08-18 11:23 PDT, Antti Koivisto 		kling: review+
Details | Formatted Diff | Diff
fix lists (62.51 KB, patch)
2013-08-18 12:23 PDT, Antti Koivisto 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2013-08-18 09:06:59 PDT 
This will be helpful for future refactorings.
Comment 1 Antti Koivisto 2013-08-18 11:23:49 PDT 
Created attachment 209039 [details]
le patch
Comment 2 Andreas Kling 2013-08-18 11:35:53 PDT 
Comment on attachment 209039 [details]
le patch

r=me
Comment 3 Antti Koivisto 2013-08-18 12:23:20 PDT 
Created attachment 209040 [details]
fix lists
Comment 4 Andreas Kling 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.
Comment 5 Antti Koivisto 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.
Comment 6 Antti Koivisto 2013-08-18 12:37:04 PDT 
private in Element I mean.
Comment 7 Antti Koivisto 2013-08-18 13:00:48 PDT 
https://trac.webkit.org/r154254