Simplify the design and make it easier to extend.
Created attachment 384536 [details] patch
Created attachment 384537 [details] patch
Created attachment 384538 [details] patch
Created attachment 384541 [details] patch
Comment on attachment 384541 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=384541&action=review > Source/WebCore/rendering/line/LineLayoutTraversalComplexPath.h:39 > + , m_sortedInlineTextBoxes(WTFMove(sortedInlineTextBoxes)) We do this all over, so maybe there is a good reason for it, but I don't think this WTFMove() is necessary. sortedInlineTextBoxes is already an rvalue, so the move constructor of m_sortedInlineTextBoxes should be used regardless. > Source/WebCore/rendering/line/LineLayoutTraversalSimplePath.h:69 > + // FIXME: SLL can contains <br> runs and this should skip over them. I would spell out SimpleLineLayout here instead of using SSL, took me a minute to figure out what it meant.
> > Source/WebCore/rendering/line/LineLayoutTraversalComplexPath.h:39 > > + , m_sortedInlineTextBoxes(WTFMove(sortedInlineTextBoxes)) > > We do this all over, so maybe there is a good reason for it, but I don't > think this WTFMove() is necessary. sortedInlineTextBoxes is already an > rvalue, so the move constructor of m_sortedInlineTextBoxes should be used > regardless. This compiles: struct Bar { Bar(Bar&&) = default; Bar(const Bar&) = delete; }; struct Foo { Foo(Bar&& bar) : bar(WTFMove(bar)) { } Bar bar; }; This doesn't (Call to deleted constructor of 'Bar'): struct Foo { Foo(Bar&& bar) : bar(bar) { } Bar bar; }; If you leave WTFMove out you'll accidentally invoke copy constructor, if any. https://en.cppreference.com/w/cpp/utility/move says: "Names of rvalue reference variables are lvalues and have to be converted to xvalues to be bound to the function overloads that accept rvalue reference parameters"
Created attachment 384543 [details] patch
🤘
Comment on attachment 384543 [details] patch Clearing flags on attachment: 384543 Committed r252959: <https://trac.webkit.org/changeset/252959>
All reviewed patches have been landed. Closing bug.
<rdar://problem/57540053>