Introduce NodeTraversal::ancestorsOf() and inclusiveAncestors() for range-based for loop

third_party/WebKit/Source/core/dom/NodeTraversal.h

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2014 Samsung Electronics. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 16 matching lines @@
 #define NodeTraversal_h
 
 #include "core/CoreExport.h"
 #include "core/dom/ContainerNode.h"
 #include "core/dom/Node.h"
 #include "wtf/Allocator.h"
 
 namespace blink {
 
 template <class Iterator> class TraversalRange;
+template <class TraversalNext> class TraversalAncestorsIterator;
+template <class TraversalNext> class TraversalAncestorsOrSelfIterator;
 template <class TraversalNext> class TraversalChildrenIterator;
 template <class TraversalNext> class TraversalDescendantIterator;
 template <class TraversalNext> class TraversalInclusiveDescendantIterator;
 template <class TraversalNext> class TraversalNextIterator;
 
 class NodeTraversal {
     STATIC_ONLY(NodeTraversal);
 public:
     using TraversalNodeType = Node;
 
@@ skipping 46 matching lines @@
     static Node* firstChild(const Node& parent) { return parent.firstChild(); }
     static Node* lastChild(const Node& parent) { return parent.lastChild(); }
     static Node* nextSibling(const Node& node) { return node.nextSibling(); }
     static Node* previousSibling(const Node& node) { return node.previousSibling(); }
     static ContainerNode* parent(const Node& node) { return node.parentNode(); }
     static Node* commonAncestor(const Node& nodeA, const Node& nodeB);
     static unsigned index(const Node& node) { return node.nodeIndex(); }
     static unsigned countChildren(const Node& parent) { return parent.countChildren(); }
     static ContainerNode* parentOrShadowHostNode(const Node& node) { return node.parentOrShadowHostNode(); }
 
+    static TraversalRange<TraversalAncestorsIterator<NodeTraversal>> ancestorsOf(const Node&);
+    static TraversalRange<TraversalAncestorsOrSelfIterator<NodeTraversal>> ancestorsOrSelfOf(const Node*);

[fs, 2016/04/28 08:18:52]

Drive-by: ancestorsOrSelfOf is kind of a weird name for something that does not
return either 'self' or ancestors (but rather both).

Maybe inclusiveAncestorsOf? (Similar to inclusiveDescendantsOf/descendantsOf)

[yosin_UTC9, 2016/04/28 08:59:05]

Done.

+    static TraversalRange<TraversalAncestorsOrSelfIterator<NodeTraversal>> ancestorsOrSelfOf(const Node&);
     static TraversalRange<TraversalChildrenIterator<NodeTraversal>> childrenOf(const Node&);
     static TraversalRange<TraversalDescendantIterator<NodeTraversal>> descendantsOf(const Node&);
     static TraversalRange<TraversalInclusiveDescendantIterator<NodeTraversal>> inclusiveDescendantsOf(const Node&);
     static TraversalRange<TraversalNextIterator<NodeTraversal>> startsAt(const Node*);
     static TraversalRange<TraversalNextIterator<NodeTraversal>> startsAfter(const Node&);
 
 private:
     template <class NodeType>
     static Node* traverseNextTemplate(NodeType&);
     template <class NodeType>
@@ skipping 21 matching lines @@
     using NodeType = typename TraversalNext::TraversalNodeType;
     NodeType& operator*() { return *m_current; }
     bool operator!=(const TraversalIteratorBase& rval) const { return m_current != rval.m_current ; }
 protected:
     explicit TraversalIteratorBase(NodeType* current) : m_current(current) { }
 
     Member<NodeType> m_current;
 };
 
 template <class TraversalNext>
+class TraversalAncestorsIterator : public TraversalIteratorBase<TraversalNext> {
+    STACK_ALLOCATED();
+public:
+    using StartNodeType = Node;
+    using TraversalIteratorBase<TraversalNext>::m_current;
+    explicit TraversalAncestorsIterator(const StartNodeType* start) : TraversalIteratorBase<TraversalNext>(TraversalNext::parent(*start)) { }
+    void operator++() { m_current = TraversalNext::parent(*m_current); }
+    static TraversalAncestorsIterator end() { return TraversalAncestorsIterator(); }
+private:
+    TraversalAncestorsIterator() : TraversalIteratorBase<TraversalNext>(nullptr) { }
+};
+
+template <class TraversalNext>
+class TraversalAncestorsOrSelfIterator : public TraversalIteratorBase<TraversalNext> {
+    STACK_ALLOCATED();
+public:
+    using StartNodeType = Node;
+    using TraversalIteratorBase<TraversalNext>::m_current;
+    explicit TraversalAncestorsOrSelfIterator(const StartNodeType* start) : TraversalIteratorBase<TraversalNext>(const_cast<StartNodeType*>(start)) { }
+    void operator++() { m_current = TraversalNext::parent(*m_current); }
+    static TraversalAncestorsOrSelfIterator end() { return TraversalAncestorsOrSelfIterator(); }
+private:
+    TraversalAncestorsOrSelfIterator() : TraversalIteratorBase<TraversalNext>(nullptr) { }
+};
+
+template <class TraversalNext>
 class TraversalChildrenIterator : public TraversalIteratorBase<TraversalNext> {
     STACK_ALLOCATED();
 public:
     using StartNodeType = Node;
     using TraversalIteratorBase<TraversalNext>::m_current;
     explicit TraversalChildrenIterator(const StartNodeType* start) : TraversalIteratorBase<TraversalNext>(TraversalNext::firstWithin(*start)) { }
     void operator++() { m_current = TraversalNext::nextSibling(*m_current); }
     static TraversalChildrenIterator end() { return TraversalChildrenIterator(); }
 private:
     TraversalChildrenIterator() : TraversalIteratorBase<TraversalNext>(nullptr) { }
@@ skipping 31 matching lines @@
     using StartNodeType = typename TraversalNext::TraversalNodeType;
     using NodeType = typename TraversalNext::TraversalNodeType;
     using TraversalIteratorBase<TraversalNext>::m_current;
     explicit TraversalInclusiveDescendantIterator(const StartNodeType* start) : TraversalIteratorBase<TraversalNext>(const_cast<NodeType*>(start)), m_root(start) { }
     void operator++() { m_current = TraversalNext::next(*m_current, m_root); }
     static TraversalInclusiveDescendantIterator end() { return TraversalInclusiveDescendantIterator(nullptr); }
 private:
     Member<const StartNodeType> m_root;
 };
 
+inline TraversalRange<TraversalAncestorsIterator<NodeTraversal>> NodeTraversal::ancestorsOf(const Node& node)
+{
+    return TraversalRange<TraversalAncestorsIterator<NodeTraversal>>(&node);
+}
+
+inline TraversalRange<TraversalAncestorsOrSelfIterator<NodeTraversal>> NodeTraversal::ancestorsOrSelfOf(const Node& node)
+{
+    return TraversalRange<TraversalAncestorsOrSelfIterator<NodeTraversal>>(&node);
+}
+
+inline TraversalRange<TraversalAncestorsOrSelfIterator<NodeTraversal>> NodeTraversal::ancestorsOrSelfOf(const Node* node)

[hayato, 2016/04/28 08:38:20]

It might be better that we do not provide a pointer version. In my experience,
that is likely hide a potential wrong usage in the caller side.
In most cases, it would be better to check null-ness explicitly in the caller.


I know that we already have Node::startsAtstartAt(const Node* start), but we
should prevent spreading the wrong APIs.

[yosin_UTC9, 2016/04/28 08:59:04]

I don't agree, pointer version is useful and safe for nature of
inclusiveAncestorsOf(), renamed from ancestorsOrSelf().

In editing and event handling, we sometimes deal with null node situation,
explicit null checking with inclusiveAncestorsOf() increases nesting and need
more attention. I'm afraid that people simply add |*| to convert pointer to
reference.

Example:

OLD:
for (; node; node = node->parentNode()) {
            if (!node->isElementNode())

inclusiveAncestorsOf():
if (node) {
  for (Node& runner : NodeTraversal::inclusiveAncestorsOf(*node)) {
     if (!node->isElementNode())
}

[yosin_UTC9, 2016/04/28 09:01:29]

Actually, I introduced reference version only then started rewriting. I found
some of them requires null check, then I introduced pointer version for ease of
writing code.

+{
+    return TraversalRange<TraversalAncestorsOrSelfIterator<NodeTraversal>>(node);
+}
+
 inline TraversalRange<TraversalChildrenIterator<NodeTraversal>> NodeTraversal::childrenOf(const Node& parent)
 {
     return TraversalRange<TraversalChildrenIterator<NodeTraversal>>(&parent);
 }
 
 inline TraversalRange<TraversalDescendantIterator<NodeTraversal>> NodeTraversal::descendantsOf(const Node& root)
 {
     return TraversalRange<TraversalDescendantIterator<NodeTraversal>>(&root);
 }
 
@@ skipping 63 matching lines @@
 {
     Node* child = parent.firstChild();
     while (child && index--)
         child = child->nextSibling();
     return child;
 }
 
 } // namespace blink
 
 #endif