Changeset 202262 in webkit


Ignore:
Timestamp:
Jun 20, 2016 8:01:02 PM (8 years ago)
Author:
Chris Dumez
Message:

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.

Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r202255 r202262  
     12016-06-20  Chris Dumez  <cdumez@apple.com>
     2
     3        Simplify / Optimize DataDetector's searchForLinkRemovingExistingDDLinks()
     4        https://bugs.webkit.org/show_bug.cgi?id=158968
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Simplify / Optimize DataDetector's searchForLinkRemovingExistingDDLinks():
     9        - Use modern ancestorsOfType<HTMLAnchorElement>() to traverse anchor ancestors
     10          instead of traversing by hand.
     11        - Use NodeTraversal::next() to traverse the tree until we find endNode and
     12          use a for loop instead of a while loop. Previously, the logic the determine
     13          the next node was at the end of the loop and was identical behavior-wise
     14          to NodeTraversal::next(). However, the previous code for a lot less efficient
     15          because it was calling Node::childNodes() to get a NodeList of the children,
     16          then calling length() on it to check if we had children and finally use
     17          the first item in the list as next node. This was very inefficient because
     18          NodeList::length() would need to traverse all children to figure out the
     19          length and would cache all the children in a Vector in CollectionIndexCache.
     20
     21        * dom/ElementAncestorIterator.h:
     22        (WebCore::ancestorsOfType):
     23        * dom/ElementIterator.h:
     24        (WebCore::findElementAncestorOfType):
     25        (WebCore::findElementAncestorOfType<Element>):
     26        Update ancestorsOfType() to take a Node instead of an Element. There are no
     27        performance benefits to taking an Element here and it is a valid use case to
     28        want an Element ancestor of a non-Element node.
     29
     30        * editing/cocoa/DataDetection.mm:
     31        (WebCore::searchForLinkRemovingExistingDDLinks):
     32        (WebCore::dataDetectorTypeForCategory): Deleted.
     33
    1342016-06-20  Commit Queue  <commit-queue@webkit.org>
    235

  • trunk/Source/WebCore/dom/ElementAncestorIterator.h

    r173907 r202262  
    7777template <typename ElementType> ElementAncestorIteratorAdapter<ElementType> lineageOfType(Element& first);
    7878template <typename ElementType> ElementAncestorConstIteratorAdapter<ElementType> lineageOfType(const Element& first);
    79 template <typename ElementType> ElementAncestorIteratorAdapter<ElementType> ancestorsOfType(Element& descendant);
    80 template <typename ElementType> ElementAncestorConstIteratorAdapter<ElementType> ancestorsOfType(const Element& descendant);
     79template <typename ElementType> ElementAncestorIteratorAdapter<ElementType> ancestorsOfType(Node& descendant);
     80template <typename ElementType> ElementAncestorConstIteratorAdapter<ElementType> ancestorsOfType(const Node& descendant);
    8181
    8282// ElementAncestorIterator
    … …  
    199199
    200200template <typename ElementType>
    201 inline ElementAncestorIteratorAdapter<ElementType> ancestorsOfType(Element& descendant)
     201inline ElementAncestorIteratorAdapter<ElementType> ancestorsOfType(Node& descendant)
    202202{
    203203    ElementType* first = findElementAncestorOfType<ElementType>(descendant);
    … …  
    206206
    207207template <typename ElementType>
    208 inline ElementAncestorConstIteratorAdapter<ElementType> ancestorsOfType(const Element& descendant)
     208inline ElementAncestorConstIteratorAdapter<ElementType> ancestorsOfType(const Node& descendant)
    209209{
    210210    const ElementType* first = findElementAncestorOfType<const ElementType>(descendant);

  • trunk/Source/WebCore/dom/ElementIterator.h

    r188520 r202262  
    193193
    194194template <typename ElementType>
    195 inline ElementType* findElementAncestorOfType(const Element& current)
     195inline ElementType* findElementAncestorOfType(const Node& current)
    196196{
    197197    for (Element* ancestor = current.parentElement(); ancestor; ancestor = ancestor->parentElement()) {
    … …  
    203203
    204204template <>
    205 inline Element* findElementAncestorOfType<Element>(const Element& current)
     205inline Element* findElementAncestorOfType<Element>(const Node& current)
    206206{
    207207    return current.parentElement();

  • trunk/Source/WebCore/editing/cocoa/DataDetection.mm

    r200874 r202262  
    3030#import "CSSStyleDeclaration.h"
    3131#import "DataDetectorsSPI.h"
     32#import "ElementAncestorIterator.h"
    3233#import "ElementTraversal.h"
    3334#import "FrameView.h"
    … …  
    277278{
    278279    didModifyDOM = false;
    279     Node* node = &startNode;
    280     while (node) {
     280    for (Node* node = &startNode; node; NodeTraversal::next(*node)) {
    281281        if (is<HTMLAnchorElement>(*node)) {
    282282            auto& anchor = downcast<HTMLAnchorElement>(*node);
    … …  
    290290            // If we found the end node and no link, return false unless an ancestor node is a link.
    291291            // The only ancestors not tested at this point are in the direct line from self's parent to the top.
    292             node = startNode.parentNode();
    293             while (node) {
    294                 if (is<HTMLAnchorElement>(*node)) {
    295                     auto& anchor = downcast<HTMLAnchorElement>(*node);
    296                     if (!equalIgnoringASCIICase(anchor.fastGetAttribute(x_apple_data_detectorsAttr), "true"))
    297                         return true;
    298                     removeResultLinksFromAnchor(anchor);
    299                     didModifyDOM = true;
    300                 }
    301                 node = node->parentNode();
     292            for (auto& anchor : ancestorsOfType<HTMLAnchorElement>(startNode)) {
     293                if (!equalIgnoringASCIICase(anchor.fastGetAttribute(x_apple_data_detectorsAttr), "true"))
     294                    return true;
     295                removeResultLinksFromAnchor(anchor);
     296                didModifyDOM = true;
    302297            }
    303298            return false;
    304         }
    305        
    306         RefPtr<NodeList> childNodes = node->childNodes();
    307         if (childNodes->length())
    308             node = childNodes->item(0);
    309         else {
    310             Node* newNode = node->nextSibling();
    311             Node* parentNode = node;
    312             while (!newNode) {
    313                 parentNode = parentNode->parentNode();
    314                 if (!parentNode)
    315                     return false;
    316                 newNode = parentNode->nextSibling();
    317             }
    318             node = newNode;
    319299        }
    320300    }
Note: See TracChangeset for help on using the changeset viewer.