Better approach to iterating.
Created attachment 209551 [details] maybe something along these lines
Comment on attachment 209551 [details] maybe something along these lines I think this would be absolutely magical with C++11 range-based for loops. I also think get() should return an Element& and we can have something like getPtr() for Element*. (This to encourage ourselves to use references.)
Will look nice in C++11 for loops. When we use element pointers, it’s well defined what happens if the document changes while we are iterating. For iterators, we’d want to define what happens in that case. What our hash tables do with iterators is that in debug versions they invalidate iterators when changes are made to the underlying data structure. Maybe we could do some variation on that.
(In reply to comment #2) > I also think get() should return an Element& and we can have something like getPtr() for Element*. (This to encourage ourselves to use references.) C++ iterators normally use *iterator, not iterator.get().
Comment on attachment 209551 [details] maybe something along these lines View in context: https://bugs.webkit.org/attachment.cgi?id=209551&action=review > Source/WebCore/dom/ElementTraversal.h:107 > + bool operator!=(const ElementDescendantIterator& other) const { return m_current != other.m_current; } This method could assert that both iterators have the same root.
(In reply to comment #3) > Will look nice in C++11 for loops. > > When we use element pointers, it’s well defined what happens if the document changes while we are iterating. For iterators, we’d want to define what happens in that case. What our hash tables do with iterators is that in debug versions they invalidate iterators when changes are made to the underlying data structure. Maybe we could do some variation on that. Yeah. I think we should assert that there are no tree mutations during iteration. Not just in the current subtree, in the entire tree.
(In reply to comment #6) > Yeah. I think we should assert that there are no tree mutations during iteration. Not just in the current subtree, in the entire tree. Should probably go further and also assert that no DOM events were dispatched. An event handler could have mutated the tree; just because it didn’t actually happen does not mean the code is OK.
Created attachment 209595 [details] patch
Created attachment 209596 [details] another
Comment on attachment 209596 [details] another Attachment 209596 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1564277
Comment on attachment 209596 [details] another Attachment 209596 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/1561584
Comment on attachment 209596 [details] another Attachment 209596 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/1544952
Comment on attachment 209596 [details] another Attachment 209596 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/1562424
Comment on attachment 209596 [details] another Attachment 209596 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/1564290
Comment on attachment 209596 [details] another Attachment 209596 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/1559658
Created attachment 209603 [details] yet another
Comment on attachment 209603 [details] yet another View in context: https://bugs.webkit.org/attachment.cgi?id=209603&action=review > Source/WebCore/WebCore.vcxproj/WebCore.vcxproj:-1 > -<?xml version="1.0" encoding="utf-8"?> Removing this was probably a mistake.
https://trac.webkit.org/r154581