Bug 104685

Summary: Add Element-specific traversal functions
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: allan.jensen, apinheiro, cfleizach, cmarcelo, darin, dglazkov, dmazzoni, d-r, eric, fmalita, jdiggs, kling, macpherson, menard, mifenton, ojan.autocc, pdr, rniwa, schenney, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
rniwa: review+, webkit.review.bot: commit-queue-
patch2 none

Antti Koivisto
Reported 2012-12-11 10:41:27 PST
In many cases where we use NodeTraversal we really only want Elements. By having Element specific iteration functions we can tighten the code and make it faster too.
Attachments
patch (55.06 KB, patch)
2012-12-11 14:44 PST, Antti Koivisto 		rniwa: review+
webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
patch2 (55.81 KB, patch)
2012-12-11 15:23 PST, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Antti Koivisto
Comment 1 2012-12-11 14:44:07 PST
Created attachment 178882 [details] patch
Ryosuke Niwa
Comment 2 2012-12-11 15:02:32 PST
Comment on attachment 178882 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=178882&action=review > Source/WebCore/ChangeLog:1529 > + Another build fix. > + > + * html/HTMLPropertiesCollection.cpp: > + (WebCore::nextNodeWithProperty): Please revert these changes. > Source/WebCore/dom/ContainerNodeAlgorithms.h:319 > + Element* element = (shouldIncludeRoot == IncludeRoot && root->isElementNode()) ? static_cast<Element*>(root) : ElementTraversal::firstWithin(root); I don't have toElement? > Source/WebCore/dom/NodeTraversal.h:102 > +template <class NodeType> > +inline Element* traverseNextElementTemplate(NodeType* current, const Node* stayWithin) > +{ > + Node* node = NodeTraversal::next(current, stayWithin); > + while (node && !node->isElementNode()) > + node = NodeTraversal::nextSkippingChildren(node, stayWithin); > + return static_cast<Element*>(node); > +} I'm sad that we have to duplicate the template like this. Can we had a boolean template argument and share the code between two? > Source/WebCore/dom/NodeTraversal.h:126 > +template <class NodeType> > +inline Element* traverseNextElementSkippingChildrenTemplate(NodeType* current, const Node* stayWithin) > +{ > + Node* node = NodeTraversal::nextSkippingChildren(current, stayWithin); > + while (node && !node->isElementNode()) > + node = NodeTraversal::nextSkippingChildren(node, stayWithin); > + return static_cast<Element*>(node); > +} Ditto.
WebKit Review Bot
Comment 3 2012-12-11 15:10:51 PST
Comment on attachment 178882 [details] patch Attachment 178882 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15277319
Antti Koivisto
Comment 4 2012-12-11 15:23:27 PST
Created attachment 178892 [details] patch2
Antti Koivisto
Comment 5 2012-12-11 15:24:56 PST
(In reply to comment #2) > I'm sad that we have to duplicate the template like this. Can we had a boolean template argument and share the code between two? I didn't do this as the templates get pretty messy and hard to understand if we also want optimal code.
Antti Koivisto
Comment 6 2012-12-11 19:15:06 PST
http://trac.webkit.org/changeset/137406
Note You need to log in before you can comment on or make changes to this bug.