See parent bug for the motivation behind this work: https://bugs.webkit.org/show_bug.cgi?id=223204
Created attachment 423226 [details] Patch
Created attachment 423260 [details] Patch
Comment on attachment 423260 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423260&action=review > Source/WebCore/css/CSSValuePair.cpp:37 > + return first + separatorCssText() + second; makeString() might be more efficient. > Source/WebCore/css/CSSValuePair.h:36 > + * Intended to replace Pair.h, which is only capable of containing CSSPrimitiveValues, and is not a sub-class of > + * CSSValue itself. See https://bugs.webkit.org/show_bug.cgi?id=223204. This comment won't be useful in the future when Pair.h is lost in the mists of time.
Created attachment 423358 [details] Patch
Thanks for the review, Simon. Marked this new patch with cq?
Committed r274546: <https://commits.webkit.org/r274546> All reviewed patches have been landed. Closing bug and clearing flags on attachment 423358 [details].
<rdar://problem/75506725>
Comment on attachment 423358 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423358&action=review > Source/WebCore/css/CSSValue.h:80 > + String separatorCssText() const; For efficiency, this should return ASCIILiteral instead of String. It never needs to return a value that’s not an ASCII literal, and this saves some memory allocation. The name should be separatorCSSText, rather than separatorCssText. > Source/WebCore/css/CSSValueList.cpp:89 > + String separator = separatorCssText(); If we change separatorCSSText to return an ASCIILiteral, that’s the type we’ll want for this local variable too. Could just use "auto".
(In reply to Darin Adler from comment #8) > Comment on attachment 423358 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=423358&action=review > > > Source/WebCore/css/CSSValue.h:80 > > + String separatorCssText() const; > > For efficiency, this should return ASCIILiteral instead of String. It never > needs to return a value that’s not an ASCII literal, and this saves some > memory allocation. > > The name should be separatorCSSText, rather than separatorCssText. > > > Source/WebCore/css/CSSValueList.cpp:89 > > + String separator = separatorCssText(); > > If we change separatorCSSText to return an ASCIILiteral, that’s the type > we’ll want for this local variable too. Could just use "auto". Good to know, thanks! I've made these changes here[1]. I'll mark this follow-up patch r? and cq? but feel free to wait to be sure EWS goes green first. [1]: https://bugs.webkit.org/show_bug.cgi?id=223300