<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[202262] trunk/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/202262">202262</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2016-06-20 20:01:02 -0700 (Mon, 20 Jun 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Simplify / Optimize DataDetector's searchForLinkRemovingExistingDDLinks()
https://bugs.webkit.org/show_bug.cgi?id=158968
Reviewed by Ryosuke Niwa.
Simplify / Optimize DataDetector's searchForLinkRemovingExistingDDLinks():
- Use modern ancestorsOfType<HTMLAnchorElement>() to traverse anchor ancestors
instead of traversing by hand.
- Use NodeTraversal::next() to traverse the tree until we find endNode and
use a for loop instead of a while loop. Previously, the logic the determine
the next node was at the end of the loop and was identical behavior-wise
to NodeTraversal::next(). However, the previous code for a lot less efficient
because it was calling Node::childNodes() to get a NodeList of the children,
then calling length() on it to check if we had children and finally use
the first item in the list as next node. This was very inefficient because
NodeList::length() would need to traverse all children to figure out the
length and would cache all the children in a Vector in CollectionIndexCache.
* dom/ElementAncestorIterator.h:
(WebCore::ancestorsOfType):
* dom/ElementIterator.h:
(WebCore::findElementAncestorOfType):
(WebCore::findElementAncestorOfType<Element>):
Update ancestorsOfType() to take a Node instead of an Element. There are no
performance benefits to taking an Element here and it is a valid use case to
want an Element ancestor of a non-Element node.
* editing/cocoa/DataDetection.mm:
(WebCore::searchForLinkRemovingExistingDDLinks):
(WebCore::dataDetectorTypeForCategory): Deleted.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoredomElementAncestorIteratorh">trunk/Source/WebCore/dom/ElementAncestorIterator.h</a></li>
<li><a href="#trunkSourceWebCoredomElementIteratorh">trunk/Source/WebCore/dom/ElementIterator.h</a></li>
<li><a href="#trunkSourceWebCoreeditingcocoaDataDetectionmm">trunk/Source/WebCore/editing/cocoa/DataDetection.mm</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (202261 => 202262)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-06-21 01:48:53 UTC (rev 202261)
+++ trunk/Source/WebCore/ChangeLog 2016-06-21 03:01:02 UTC (rev 202262)
</span><span class="lines">@@ -1,3 +1,36 @@
</span><ins>+2016-06-20 Chris Dumez <cdumez@apple.com>
+
+ Simplify / Optimize DataDetector's searchForLinkRemovingExistingDDLinks()
+ https://bugs.webkit.org/show_bug.cgi?id=158968
+
+ Reviewed by Ryosuke Niwa.
+
+ Simplify / Optimize DataDetector's searchForLinkRemovingExistingDDLinks():
+ - Use modern ancestorsOfType<HTMLAnchorElement>() to traverse anchor ancestors
+ instead of traversing by hand.
+ - Use NodeTraversal::next() to traverse the tree until we find endNode and
+ use a for loop instead of a while loop. Previously, the logic the determine
+ the next node was at the end of the loop and was identical behavior-wise
+ to NodeTraversal::next(). However, the previous code for a lot less efficient
+ because it was calling Node::childNodes() to get a NodeList of the children,
+ then calling length() on it to check if we had children and finally use
+ the first item in the list as next node. This was very inefficient because
+ NodeList::length() would need to traverse all children to figure out the
+ length and would cache all the children in a Vector in CollectionIndexCache.
+
+ * dom/ElementAncestorIterator.h:
+ (WebCore::ancestorsOfType):
+ * dom/ElementIterator.h:
+ (WebCore::findElementAncestorOfType):
+ (WebCore::findElementAncestorOfType<Element>):
+ Update ancestorsOfType() to take a Node instead of an Element. There are no
+ performance benefits to taking an Element here and it is a valid use case to
+ want an Element ancestor of a non-Element node.
+
+ * editing/cocoa/DataDetection.mm:
+ (WebCore::searchForLinkRemovingExistingDDLinks):
+ (WebCore::dataDetectorTypeForCategory): Deleted.
+
</ins><span class="cx"> 2016-06-20 Commit Queue <commit-queue@webkit.org>
</span><span class="cx">
</span><span class="cx"> Unreviewed, rolling out r202248.
</span></span></pre></div>
<a id="trunkSourceWebCoredomElementAncestorIteratorh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/ElementAncestorIterator.h (202261 => 202262)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/ElementAncestorIterator.h 2016-06-21 01:48:53 UTC (rev 202261)
+++ trunk/Source/WebCore/dom/ElementAncestorIterator.h 2016-06-21 03:01:02 UTC (rev 202262)
</span><span class="lines">@@ -76,8 +76,8 @@
</span><span class="cx"> ElementAncestorConstIteratorAdapter<Element> elementAncestors(const Element* descendant);
</span><span class="cx"> template <typename ElementType> ElementAncestorIteratorAdapter<ElementType> lineageOfType(Element& first);
</span><span class="cx"> template <typename ElementType> ElementAncestorConstIteratorAdapter<ElementType> lineageOfType(const Element& first);
</span><del>-template <typename ElementType> ElementAncestorIteratorAdapter<ElementType> ancestorsOfType(Element& descendant);
-template <typename ElementType> ElementAncestorConstIteratorAdapter<ElementType> ancestorsOfType(const Element& descendant);
</del><ins>+template <typename ElementType> ElementAncestorIteratorAdapter<ElementType> ancestorsOfType(Node& descendant);
+template <typename ElementType> ElementAncestorConstIteratorAdapter<ElementType> ancestorsOfType(const Node& descendant);
</ins><span class="cx">
</span><span class="cx"> // ElementAncestorIterator
</span><span class="cx">
</span><span class="lines">@@ -198,7 +198,7 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> template <typename ElementType>
</span><del>-inline ElementAncestorIteratorAdapter<ElementType> ancestorsOfType(Element& descendant)
</del><ins>+inline ElementAncestorIteratorAdapter<ElementType> ancestorsOfType(Node& descendant)
</ins><span class="cx"> {
</span><span class="cx"> ElementType* first = findElementAncestorOfType<ElementType>(descendant);
</span><span class="cx"> return ElementAncestorIteratorAdapter<ElementType>(first);
</span><span class="lines">@@ -205,7 +205,7 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> template <typename ElementType>
</span><del>-inline ElementAncestorConstIteratorAdapter<ElementType> ancestorsOfType(const Element& descendant)
</del><ins>+inline ElementAncestorConstIteratorAdapter<ElementType> ancestorsOfType(const Node& descendant)
</ins><span class="cx"> {
</span><span class="cx"> const ElementType* first = findElementAncestorOfType<const ElementType>(descendant);
</span><span class="cx"> return ElementAncestorConstIteratorAdapter<ElementType>(first);
</span></span></pre></div>
<a id="trunkSourceWebCoredomElementIteratorh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/ElementIterator.h (202261 => 202262)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/ElementIterator.h 2016-06-21 01:48:53 UTC (rev 202261)
+++ trunk/Source/WebCore/dom/ElementIterator.h 2016-06-21 03:01:02 UTC (rev 202262)
</span><span class="lines">@@ -192,7 +192,7 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> template <typename ElementType>
</span><del>-inline ElementType* findElementAncestorOfType(const Element& current)
</del><ins>+inline ElementType* findElementAncestorOfType(const Node& current)
</ins><span class="cx"> {
</span><span class="cx"> for (Element* ancestor = current.parentElement(); ancestor; ancestor = ancestor->parentElement()) {
</span><span class="cx"> if (is<ElementType>(*ancestor))
</span><span class="lines">@@ -202,7 +202,7 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> template <>
</span><del>-inline Element* findElementAncestorOfType<Element>(const Element& current)
</del><ins>+inline Element* findElementAncestorOfType<Element>(const Node& current)
</ins><span class="cx"> {
</span><span class="cx"> return current.parentElement();
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCoreeditingcocoaDataDetectionmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/editing/cocoa/DataDetection.mm (202261 => 202262)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/editing/cocoa/DataDetection.mm 2016-06-21 01:48:53 UTC (rev 202261)
+++ trunk/Source/WebCore/editing/cocoa/DataDetection.mm 2016-06-21 03:01:02 UTC (rev 202262)
</span><span class="lines">@@ -29,6 +29,7 @@
</span><span class="cx"> #import "Attr.h"
</span><span class="cx"> #import "CSSStyleDeclaration.h"
</span><span class="cx"> #import "DataDetectorsSPI.h"
</span><ins>+#import "ElementAncestorIterator.h"
</ins><span class="cx"> #import "ElementTraversal.h"
</span><span class="cx"> #import "FrameView.h"
</span><span class="cx"> #import "HTMLAnchorElement.h"
</span><span class="lines">@@ -276,8 +277,7 @@
</span><span class="cx"> static bool searchForLinkRemovingExistingDDLinks(Node& startNode, Node& endNode, bool& didModifyDOM)
</span><span class="cx"> {
</span><span class="cx"> didModifyDOM = false;
</span><del>- Node* node = &startNode;
- while (node) {
</del><ins>+ for (Node* node = &startNode; node; NodeTraversal::next(*node)) {
</ins><span class="cx"> if (is<HTMLAnchorElement>(*node)) {
</span><span class="cx"> auto& anchor = downcast<HTMLAnchorElement>(*node);
</span><span class="cx"> if (!equalIgnoringASCIICase(anchor.fastGetAttribute(x_apple_data_detectorsAttr), "true"))
</span><span class="lines">@@ -289,34 +289,14 @@
</span><span class="cx"> if (node == &endNode) {
</span><span class="cx"> // If we found the end node and no link, return false unless an ancestor node is a link.
</span><span class="cx"> // The only ancestors not tested at this point are in the direct line from self's parent to the top.
</span><del>- node = startNode.parentNode();
- while (node) {
- if (is<HTMLAnchorElement>(*node)) {
- auto& anchor = downcast<HTMLAnchorElement>(*node);
- if (!equalIgnoringASCIICase(anchor.fastGetAttribute(x_apple_data_detectorsAttr), "true"))
- return true;
- removeResultLinksFromAnchor(anchor);
- didModifyDOM = true;
- }
- node = node->parentNode();
</del><ins>+ for (auto& anchor : ancestorsOfType<HTMLAnchorElement>(startNode)) {
+ if (!equalIgnoringASCIICase(anchor.fastGetAttribute(x_apple_data_detectorsAttr), "true"))
+ return true;
+ removeResultLinksFromAnchor(anchor);
+ didModifyDOM = true;
</ins><span class="cx"> }
</span><span class="cx"> return false;
</span><span class="cx"> }
</span><del>-
- RefPtr<NodeList> childNodes = node->childNodes();
- if (childNodes->length())
- node = childNodes->item(0);
- else {
- Node* newNode = node->nextSibling();
- Node* parentNode = node;
- while (!newNode) {
- parentNode = parentNode->parentNode();
- if (!parentNode)
- return false;
- newNode = parentNode->nextSibling();
- }
- node = newNode;
- }
</del><span class="cx"> }
</span><span class="cx"> return false;
</span><span class="cx"> }
</span></span></pre>
</div>
</div>
</body>
</html>