This is in preparation to support multiple render elements in simple line layout.
Created attachment 240904 [details] Patch
I am not too happy about the RenderFlowContentIterator name. The class does a lot more than just iterating. Alternatively we could move some of the (non-iterating)class functions back to static inline and pass the RenderFlowContentIterator object around.
Comment on attachment 240904 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240904&action=review Looks like right direction. > Source/WebCore/rendering/SimpleLineLayout.cpp:216 > +template <typename CharacterType> class RenderFlowContentIterator { I think we usually put template arguments to a line of their own for classes. ContentIterator? FlowIterator? Actually this class doesn't really feel like an iterator at all. If I understand correctly it doesn't hold the current position and doesn't change state during traversal. Maybe it should? Or maybe it should just be named differently (FlowContents or similar perhaps)? > Source/WebCore/rendering/SimpleLineLayout.cpp:225 > + unsigned findNextBreakablePosition(unsigned position) I think this is logically const too though LazyLineBreakIterator interface currently requires non-const call.
LazyLineBreakIterator itself is poorly named. It looks more like a sort of iterator cache.
Created attachment 240960 [details] Patch
Comment on attachment 240960 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240960&action=review > Source/WebCore/rendering/SimpleLineLayout.cpp:444 > + const Style& style = contentIterator.style(); Could use auto& > Source/WebCore/rendering/SimpleLineLayout.cpp:551 > + const Style style = contentIterator.style(); This can be reference: auto& > Source/WebCore/rendering/SimpleLineLayout.cpp:678 > + FlowContentIterator<CharacterType> contentIterator = FlowContentIterator<CharacterType>(flow); This could use auto
Comment on attachment 240960 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=240960&action=review >> Source/WebCore/rendering/SimpleLineLayout.cpp:678 >> + FlowContentIterator<CharacterType> contentIterator = FlowContentIterator<CharacterType>(flow); > > This could use auto Actually this could just be FlowContentIterator<CharacterType> contentIterator(flow)
Created attachment 240974 [details] Patch
Comment on attachment 240974 [details] Patch Clearing flags on attachment: 240974 Committed r175601: <http://trac.webkit.org/changeset/175601>
All reviewed patches have been landed. Closing bug.