Small cleanup in RenderTheme
Created attachment 408287 [details] Patch
Comment on attachment 408287 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408287&action=review > Source/WebCore/rendering/RenderTheme.cpp:1286 > +#define NumSystemFonts 10 This can be constexpr; does not need to be "define": constexpr auto numSystemFonts = 10; > Source/WebCore/rendering/RenderTheme.cpp:1288 > + FontCascadeDescription& caption() { return (*this)[0]; } Seems to me you could just use an array here and use indices in the code below and don’t need the named getters. > Source/WebCore/rendering/RenderTheme.cpp:1298 > + static_assert(NumSystemFonts == 10); This seems pointless given the number is defined just above. > Source/WebCore/rendering/RenderThemeCocoa.mm:209 > + for (auto& description : (*this)) { > + if (description.isAbsoluteSize()) > + return false; > + } > + return true; Could write this with std::all_of or none_of: return std:: all_of(std::begin(descriptions), std::end(descriptions), [] (auto& description) ( return !description.isAbsoluteSize(); });
Created attachment 408304 [details] Patch
Comment on attachment 408304 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408304&action=review > Source/WebCore/platform/graphics/cocoa/FontDescriptionCocoa.cpp:82 > + auto atomStringComparison = [](const AtomString& lhs, const AtomString& rhs) { I think this name doesn’t say the thing it should, which is that this is the most-efficient comparison, comparison by pointer, with the drawback that it sorts things in pseudo-random, non-useful order. I think I would call this compareAsPointer; no need to state the types of the arguments. But maybe you have a different idea about the name. The name atomStringComparison would be a fine name for comparing the string by code point, or code unit, or case folded ASCII, or whatever, and none of those are what want to use here. > Source/WebCore/platform/graphics/cocoa/FontDescriptionCocoa.cpp:85 > + static auto strings { makeNeverDestroyed([&atomStringComparison] { Not 100% sure about the { } vs. = for the initialization, what is inside the lambda and what is outside, and other such things. I would have been tempted to write "const", but I suppose that’s not needed. > Source/WebCore/platform/graphics/cocoa/FontDescriptionCocoa.cpp:91 > + auto result = std::binary_search(strings.get().begin(), strings.get().end(), string, atomStringComparison); > + if (result) No need for a local variable "result" here.
Committed r266904: <https://trac.webkit.org/changeset/266904>
<rdar://problem/68679053>