AncestorChainWalker walker(node); walker.parent(); return walker.get(); It's not obvious that the above code should behave differently from: return node ? node->parentNode() : 0; The class name should communicate the semantic difference.
AncestorChainAcrossInsertionPointWalker?
Also, m_isCrossingInsertionPoint should be named m_hasCrossedInsertionPoint and parent() should be named moveToParent().
+Hayato-san, who will likely have opinions.
EventPathWalker should be a good name.
(In reply to comment #4) > EventPathWalker should be a good name. I like that!
(In reply to comment #4) > EventPathWalker should be a good name. +1. I've recently introduced 'EventPath' into EventContext in http://trac.webkit.org/changeset/142575. We should spread this term, which is used in DOM Core spec, to Walker class too. Hopefully, we can avoid a kind of confusion. This kind of confusion is apparently bad for the project. AncestorChainWalker was formally called ComposedShadowTreeParentWalker, which was separated from ComposedShadowTreeWalker. I think there are some historical debts. We should clean up now.
I think that for simple cases we need a function rather than a class so we don’t have use three lines for the idiom Ryosuke quoted.
Let me add one more history. Before ComposedShadowTreeWalker was introduced, there was a 'Reified DOM Tree traversal'. http://trac.webkit.org/changeset/112055 This is function style APIs. All public APIs are just static functions. ComposedShadowTreeWalker was introduced, deprecating ReifiedTreeTraversal. The reasons: - We need to parameterize Walker class. It might be good to have a parameter in its constructor. - I'd like to optimize the performance of walker without affecting the usage in caller side. I think NodeTraversal, which was introduced recently, might be wrapper APIs for common use cases for walker. If we can apply NodeTraversal, we should use it rather than using Walker directly.
(In reply to comment #8) > I think NodeTraversal, which was introduced recently, might be wrapper APIs for common use cases for walker. > If we can apply NodeTraversal, we should use it rather than using Walker directly. NodeTraversal isn't a class though...
I have to use yet another unfamiliar term, *reprojection*, to explain that. :) Node can be distributed to more than one insertion point during reprojection. https://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/shadow/index.html#reprojection Therefore, the walker has to visit each insertion point consecutively since an event should be fired on these insertion points. I am feeling that isCrossingInsertinPoint() is not good name, isVisitingInsertionPointInReprojection() might be a better name. (In reply to comment #2) > Also, m_isCrossingInsertionPoint should be named m_hasCrossedInsertionPoint and parent() should be named moveToParent().
Created attachment 189026 [details] Renamed
Though it seems mac bot and win bot stopped working, let me set cq+ and see what will happen.
Comment on attachment 189026 [details] Renamed Clearing flags on attachment: 189026 Committed r143422: <http://trac.webkit.org/changeset/143422>
All reviewed patches have been landed. Closing bug.