[webkit-reviews] review granted: [Bug 130384] Micro-optimize element descendant iterator. : [Attachment 227028] Even better patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Mar 18 10:56:52 PDT 2014
Antti Koivisto <koivisto at iki.fi> has granted Andreas Kling <akling at apple.com>'s
request for review:
Bug 130384: Micro-optimize element descendant iterator.
https://bugs.webkit.org/show_bug.cgi?id=130384
Attachment 227028: Even better patch
https://bugs.webkit.org/attachment.cgi?id=227028&action=review
------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=227028&action=review
> Source/WebCore/dom/ElementDescendantIterator.h:51
> + Vector<Element*, 16, UnsafeVectorOverflow> m_stack;
Please call these something more informative like m_ancestorSiblingStack
> Source/WebCore/dom/ElementDescendantIterator.h:192
> + Element* firstChild = Traversal<Element>::firstChild(m_current);
> + Element* nextSibling = Traversal<Element>::nextSibling(m_current);
Please just use ElementTraversal directly.
> Source/WebCore/dom/ElementDescendantIterator.h:196
> + if (firstChild) {
> + if (nextSibling)
> + m_stack.append(nextSibling);
Clever!
> Source/WebCore/dom/SelectorQuery.cpp:267
> - for (auto& element :
descendantsOfType<Element>(const_cast<ContainerNode&>(rootNode))) {
> + for (auto& element :
elementDescendants(const_cast<ContainerNode&>(rootNode))) {
This is bit confusing. It is ok as intermediate step but we should really add
all the missing functionality here and start returning
ElementDescendantIterator from descendantsOfType<Element>. The typed iterator
can then be implemented in terms of ElementDescendantIterator.
More information about the webkit-reviews
mailing list