Bug 123662 - Add a child renderer iterator.
Summary: Add a child renderer iterator.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-02 02:39 PDT by Andreas Kling
Modified: 2013-11-02 13:24 PDT (History)
6 users (show)

See Also:


Attachments
Possibly a patch (41.16 KB, patch)
2013-11-02 02:50 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff
More refined possible patch (35.72 KB, patch)
2013-11-02 11:55 PDT, Andreas Kling
koivisto: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2013-11-02 02:39:16 PDT
We should have an ElementIterator-style thing for walking a RenderElement's children.
Comment 1 Andreas Kling 2013-11-02 02:50:19 PDT
Created attachment 215804 [details]
Possibly a patch
Comment 2 WebKit Commit Bot 2013-11-02 02:51:30 PDT
Attachment 215804 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/rendering/RenderChildIterator.h', u'Source/WebCore/rendering/RenderElement.cpp', u'Source/WebCore/rendering/RenderElement.h', u'Source/WebCore/rendering/RenderIterator.h', u'Source/WebCore/rendering/RenderObject.h', u'Source/WebCore/rendering/RenderTable.cpp', u'Source/WebCore/rendering/RenderTableSection.h', u'Source/WebCore/rendering/RenderText.h', u'Source/WebCore/rendering/RenderTraversal.h']" exit_code: 1
Source/WebCore/rendering/RenderIterator.h:201:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Antti Koivisto 2013-11-02 03:10:10 PDT
Comment on attachment 215804 [details]
Possibly a patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215804&action=review

> Source/WebCore/rendering/RenderChildIterator.h:117
> +inline RenderChildConstIterator<T>& RenderChildConstIterator<T>::operator++()
> +{
> +    return static_cast<RenderChildConstIterator<T>&>(RenderConstIterator<T>::traverseNextSibling());

I think you just want to do return *this; and avoid cast.

> Source/WebCore/rendering/RenderTraversal.h:35
> +template <typename T>
> +class RenderTraversal {
> +public:

I think it would be better not to add this type. On the DOM side ElementTraversal was the older mechanism and the iterators are using it since it was easy to implement them that way. They should be moved away eventually. 

Instead we could just have templated helper functions in RenderIterator.h itself. Having less types and files reduces complexity.
Comment 4 Andreas Kling 2013-11-02 11:55:31 PDT
Created attachment 215825 [details]
More refined possible patch
Comment 5 WebKit Commit Bot 2013-11-02 11:56:21 PDT
Attachment 215825 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/rendering/RenderChildIterator.h', u'Source/WebCore/rendering/RenderElement.cpp', u'Source/WebCore/rendering/RenderElement.h', u'Source/WebCore/rendering/RenderIterator.h', u'Source/WebCore/rendering/RenderObject.h', u'Source/WebCore/rendering/RenderTable.cpp', u'Source/WebCore/rendering/RenderTableSection.h', u'Source/WebCore/rendering/RenderText.h']" exit_code: 1
Source/WebCore/rendering/RenderIterator.h:233:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Antti Koivisto 2013-11-02 12:18:38 PDT
Comment on attachment 215825 [details]
More refined possible patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215825&action=review

> Source/WebCore/rendering/RenderIterator.h:43
> +// Traversal helpers
> +
> +template <typename T, typename U>
> +inline T* firstChild(U& current)
> +{
> +    RenderObject* object = current.firstChild();
> +    while (object && !isRendererOfType<const T>(*object))
> +        object = object->nextSibling();
> +    return static_cast<T*>(object);
> +}

I would move these below the class definitions.
Comment 7 Andreas Kling 2013-11-02 12:50:10 PDT
Committed r158495: <http://trac.webkit.org/changeset/158495>
Comment 8 Antti Koivisto 2013-11-02 13:24:54 PDT
You should all assertions against tree mutations at some point.