[webkit-dev] attached()

Maciej Stachowiak mjs at apple.com
Thu Sep 9 01:46:19 PDT 2010


On Sep 9, 2010, at 1:29 AM, Eric Seidel wrote:

> On Thu, Sep 9, 2010 at 12:43 AM, Maciej Stachowiak <mjs at apple.com> wrote:
>> 
>> On Sep 9, 2010, at 12:00 AM, Eric Seidel wrote:
>> 
>>> I'm about to remove the last non-lazy attaching element.
>>> https://bugs.webkit.org/show_bug.cgi?id=45365
>>> 
>>> 
>>> I think we can now kill the whole concept of being "attached".
>>> 
>>> attached() does not necessarily mean you have a renderer.  It just
>>> means that something called attach() or lazyAttach() on you.
>>> 
>>> 
>>> Instead, we could change all elements to mark themselves (and their
>>> parents) as needing style recalc during insertedIntoDocument(),
>>> effectively "attaching/lazyAttaching".
>> 
>> Presumably they should mark their parents as "child needs style recalc", not needing recalc themselves.
> 
> Turns out they also have to mark themselves as needing recalc.  I
> tried only marking the parents in this latest patch and failed. :)
> http://trac.webkit.org/browser/trunk/WebCore/dom/Node.cpp#L759

Then I somewhat suspect the patch may be buggy buggy and may cause performance regressions. In cases where you insert elements into an attached tree. It very much seems like this would cause unnecessary style recalcs.

> 
>>> 
>>> (We could also fix the current lazyAttach() code to check to see if a
>>> parent is already marked with a child needing style resolve, and stop
>>> walking to prevent N^2 behavior.)
>>> 
>>> I'm not sure if it would make sense to keep the "attached" flag around
>>> to make sure the attach logic is never run twice.
>> 
>> Do any attach() methods still do nontrivial work? Is it an error to call it twice, or not properly paired up with detach()?
> 
> I haven't done a full survey of attach() yet, but yes, many elements
> override attach.
> 
> It is an error to call attach() twice:
> http://trac.webkit.org/browser/trunk/WebCore/dom/Node.cpp#L1191

Then it seems like the flag is needed for now. Perhaps some of this work can move to insertedIntoDocument over time.

Regards,
Maciej



More information about the webkit-dev mailing list