[webkit-reviews] review granted: [Bug 104332] NodeRenderingContext is slow due to ComposedShadowTreeWalker : [Attachment 179214] Patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has granted Hajime Morrita
<morrita at google.com>'s request for review:
Bug 104332: NodeRenderingContext is slow due to ComposedShadowTreeWalker
https://bugs.webkit.org/show_bug.cgi?id=104332

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

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=179214&action=review


> Source/WebCore/ChangeLog:17
> +	   chagne, NRC directly used CSTW. This NRT module hides and narrows

chagne -> change

> Source/WebCore/ChangeLog:21
> +	     doesn't need to use CSTW and just goes neighboring nodes in a
plain DOM way.

doesn't -> don't, "just goes _to_ neighboring nodes"

> Source/WebCore/ChangeLog:25
> +	   - CSTW::ParentTraversalDetails is moved and renamed to

This could've been a nice separate patch :-\

> Source/WebCore/ChangeLog:37
> +	   node flag. Each DOM node is now markd as NeedsShadowTreeWalker if
> +	   it requires non-trivial traversal in NRT which uses CSTW. This

This is great.

> Source/WebCore/ChangeLog:44
> +	   - The node is a pseudo element or

or...? :)

> Source/WebCore/ChangeLog:46
> +	   This criteria criteria is defined in
Node::needsShadowTreeWalkerSlow(). The node actually needs

zap one "criteria" from the sentence. You have two.

> Source/WebCore/dom/ContainerNode.h:268
> +inline bool Node::needsShadowTreeWalker() const

Looks like this is the hottest function in the whole patch.

> Source/WebCore/dom/NodeRenderingTraversal.h:36
> +namespace NodeRenderingTraversal {

The name is a touch obtuse, but I don't have better ideas.

> Source/WebCore/html/HTMLOptGroupElement.cpp:29
> +#include "ElementShadow.h"

Why do we need this?

> Source/WebCore/html/HTMLOptionElement.cpp:32
> +#include "ElementShadow.h"

And this?