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

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

Index: third_party/WebKit/Source/core/dom/NodeTraversal.h
diff --git a/third_party/WebKit/Source/core/dom/NodeTraversal.h b/third_party/WebKit/Source/core/dom/NodeTraversal.h
index 99bf817d9564a3b74b1ad6eb9e3039a38ae2f8b9..70c6c1d1dbbb533d7b72d9cda0bb540bedb88560 100644
--- a/third_party/WebKit/Source/core/dom/NodeTraversal.h
+++ b/third_party/WebKit/Source/core/dom/NodeTraversal.h
@@ -34,6 +34,8 @@
 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;
@@ -100,6 +102,9 @@ public:
     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&);
@@ -141,6 +146,32 @@ protected:
 };
 
 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:
@@ -192,6 +223,21 @@ 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);