There is a design issue of current ComposedShadowTreeWalker: - parentIncludingInsertionPointAndShadowRoot() does not use Policy. - Nothing prevents users from mixing parentIncludingInsertionPointAndShadowRoot() and other functions. It might be better that we have: - A separated Walker class which provides only parentIncludingInsertionPointAndShadowRoot() and get rid of the function from current Walker class. - or yet another traversing Policy used for parentIncludingInsertionPointAndShadowRoot(). - or making parentIncludingInsertionPointAndShadowRoot() a static function.
Created attachment 144305 [details] refactor a walker
Comment on attachment 144305 [details] refactor a walker View in context: https://bugs.webkit.org/attachment.cgi?id=144305&action=review > Source/WebCore/ChangeLog:3 > + Introcues ComposedShadowTreeParentWalker, extracted from ComposedShadowTreeWalker. Introcues -> Introduces > Source/WebCore/dom/ComposedShadowTreeWalker.h:124 > + void parentIncludingInsertionPointAndShadowRoot(); If this is a specialized walker, why not just call this "parent"?
Thank you for the review. (In reply to comment #2) > (From update of attachment 144305 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=144305&action=review > > > Source/WebCore/ChangeLog:3 > > + Introcues ComposedShadowTreeParentWalker, extracted from ComposedShadowTreeWalker. > > Introcues -> Introduces My bad. Let me fix. > > > Source/WebCore/dom/ComposedShadowTreeWalker.h:124 > > + void parentIncludingInsertionPointAndShadowRoot(); > > If this is a specialized walker, why not just call this "parent"? I agree that this is long and ugly function name, but I'd like to leave words of 'IncludingInsertionPointAndShadowRoot' somewhere to make its behavior clear and remind caller of the specialness of the function. Another idea is to change class name, say ComposedShadowTreeParentXXXXInserionPointAndShadowRootXXXWalker' (I cannot have a good name :( ) and call the function 'parent()' since the class has only one traversal member function. But I'd like to avoid such a long class name. So let me land this as is. I am okay to rename the function name if we have better one.
Created attachment 144684 [details] Patch for landing
Comment on attachment 144684 [details] Patch for landing Clearing flags on attachment: 144684 Committed r118890: <http://trac.webkit.org/changeset/118890>
All reviewed patches have been landed. Closing bug.