See the comment on https://bugs.webkit.org/show_bug.cgi?id=79197#c35. It might be better to rewrite Reified tree traversal APIs using cursor pattern instead of stateless static function pattern. Let me implement a cursor pattern version here and compare both versions.
Created attachment 133460 [details] WIP. concept of walker. Not tested yet.
Created attachment 134267 [details] let me knwo required symbols on windows
Comment on attachment 134267 [details] let me knwo required symbols on windows Attachment 134267 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12147000
Comment on attachment 134267 [details] let me knwo required symbols on windows Attachment 134267 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12146999
Comment on attachment 134267 [details] let me knwo required symbols on windows Attachment 134267 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/12155095
Comment on attachment 134267 [details] let me knwo required symbols on windows Attachment 134267 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12154103
Comment on attachment 134267 [details] let me knwo required symbols on windows Attachment 134267 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/12155096
Comment on attachment 134267 [details] let me knwo required symbols on windows Attachment 134267 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12153144
Comment on attachment 134267 [details] let me knwo required symbols on windows Attachment 134267 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12154104
Comment on attachment 134267 [details] let me knwo required symbols on windows While you're still working on this, I have another bikeshedding suggestion for the name. We need to mention "Shadow DOM" here somewhere. Soo... I thought about this and have this new name... Please don't shoot me! ... It's "ComposedShadowTreeWalker". What do you think? Please feel free to reject the suggestion.
lol. I don't have a strong opinion. Either is okay. ComposedShadowTreeWalker is long, but more intuitive. In general, I prefer a long intuitive name to a short non-intuitive name. So let's use ComposedShadowTreeWalker from now if no one objects to it. It's not too late. (In reply to comment #10) > (From update of attachment 134267 [details]) > While you're still working on this, I have another bikeshedding suggestion for the name. We need to mention "Shadow DOM" here somewhere. Soo... I thought about this and have this new name... Please don't shoot me! ... It's "ComposedShadowTreeWalker". What do you think? Please feel free to reject the suggestion.
Created attachment 134725 [details] give me necesarry symbols
Comment on attachment 134725 [details] give me necesarry symbols Attachment 134725 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12288056
Comment on attachment 134725 [details] give me necesarry symbols Attachment 134725 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12262081
Created attachment 134735 [details] add symbols
Created attachment 134739 [details] ready for review
Comment on attachment 134739 [details] ready for review View in context: https://bugs.webkit.org/attachment.cgi?id=134739&action=review This is sooo much more elegant. Thank you for doing this! A few tweaks before you land: > Source/WebCore/dom/ComposedShadowTreeWalker.cpp:45 > +#ifndef NDEBUG > +#define ASSERT_PRECONDITION() assertPrecondition() > +#else > +#define ASSERT_PRECONDITION() > +#endif > + > +#ifndef NDEBUG > +#define ASSERT_POSTCONDITION() assertPostcondition() > +#else > +#define ASSERT_POSTCONDITION() > +#endif The WebKit typical approach here is to declare these as empty inline-in-header functions for #ifdef NDEBUG. > Source/WebCore/dom/ComposedShadowTreeWalker.cpp:77 > +ComposedShadowTreeWalker ComposedShadowTreeWalker::startedWithFirstChild(const Node* node, Policy policy) I think just fromFirstChild is nicer. > Source/WebCore/dom/ComposedShadowTreeWalker.h:61 > + void nextNode(); > + void previousNode(); These can just be next and previous.
Thank you for the review. Let me land this patch after I address your comments. (In reply to comment #17) > (From update of attachment 134739 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=134739&action=review > > This is sooo much more elegant. Thank you for doing this! A few tweaks before you land: > > > Source/WebCore/dom/ComposedShadowTreeWalker.cpp:45 > > +#ifndef NDEBUG > > +#define ASSERT_PRECONDITION() assertPrecondition() > > +#else > > +#define ASSERT_PRECONDITION() > > +#endif > > + > > +#ifndef NDEBUG > > +#define ASSERT_POSTCONDITION() assertPostcondition() > > +#else > > +#define ASSERT_POSTCONDITION() > > +#endif > > The WebKit typical approach here is to declare these as empty inline-in-header functions for #ifdef NDEBUG. Okay. I'll move these to headers. > > > Source/WebCore/dom/ComposedShadowTreeWalker.cpp:77 > > +ComposedShadowTreeWalker ComposedShadowTreeWalker::startedWithFirstChild(const Node* node, Policy policy) > > I think just fromFirstChild is nicer. Thats' nice. > > > Source/WebCore/dom/ComposedShadowTreeWalker.h:61 > > + void nextNode(); > > + void previousNode(); > > These can just be next and previous. Okay. Let me do it. I'll rename parentNode() to parent() also.
Created attachment 135026 [details] try builds
Created attachment 135042 [details] Patch for landing
Comment on attachment 135042 [details] Patch for landing Clearing flags on attachment: 135042 Committed r112845: <http://trac.webkit.org/changeset/112845>
All reviewed patches have been landed. Closing bug.