Unify the Mac and iOS implementations of FontPlatformData a bit
Created attachment 243425 [details] Patch
Comment on attachment 243425 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=243425&action=review > Source/WebCore/platform/graphics/FontPlatformData.cpp:50 > + , m_ctFont(nullptr) Smart pointer, so doesn’t need explicit initialization. > Source/WebCore/platform/graphics/FontPlatformData.cpp:53 > + , m_cgFont(nullptr) Smart pointer, so doesn’t need explicit initialization. > Source/WebCore/platform/graphics/FontPlatformData.cpp:78 > + , m_font(nullptr) I suggest we use the newfangled syntax in the header: CTFontRef m_font { nullptr }; Then avoid lines of code like the one above. > Source/WebCore/platform/graphics/FontPlatformData.cpp:79 > + , m_ctFont(nullptr) Smart pointer, so doesn’t need explicit initialization. > Source/WebCore/platform/graphics/FontPlatformData.cpp:82 > + , m_cgFont(nullptr) Smart pointer, so doesn’t need explicit initialization. > Source/WebCore/platform/graphics/FontPlatformData.cpp:107 > + , m_font(nullptr) I suggest we use the newfangled syntax in the header: CTFontRef m_font { nullptr }; Then avoid lines of code like the one above. > Source/WebCore/platform/graphics/FontPlatformData.cpp:108 > + , m_ctFont(nullptr) Smart pointer, so doesn’t need explicit initialization. > Source/WebCore/platform/graphics/FontPlatformData.cpp:111 > + , m_cgFont(nullptr) Smart pointer, so doesn’t need explicit initialization. > Source/WebCore/platform/graphics/FontPlatformData.cpp:113 > + , m_scaledFont(nullptr) I suggest we use the newfangled syntax in the header: cairo_scaled_font_t* m_scaledFont { nullptr }; Then avoid lines of code like the one above. > Source/WebCore/platform/graphics/FontPlatformData.h:46 > #if PLATFORM(IOS) I like it better if we don’t nest #if like this. > Source/WebCore/platform/graphics/FontPlatformData.h:49 > +#if PLATFORM(MAC) I like it better if we don’t nest #if like this. Can we use USE(APPKIT) for this? > Source/WebCore/platform/graphics/FontPlatformData.h:75 > Extra blank line here, I think. > Source/WebCore/platform/graphics/FontPlatformData.h:92 > +#if PLATFORM(MAC) Why did you switch from USE(APPKIT) to PLATFORM(MAC)? > Source/WebCore/platform/graphics/FontPlatformData.h:120 > +#if PLATFORM(MAC) USE(APPKIT)? > Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm:49 > +#if PLATFORM(IOS) > , m_isEmoji(false) > +#endif Could use in-class initialization for this: bool m_isEmoji { false }; Unless there is a good reason not to.
Created attachment 243458 [details] Patch
Comment on attachment 243458 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=243458&action=review > Source/WebCore/platform/graphics/FontPlatformData.h:118 > + // Oops, probably want to go and remove this or add the comment that was intended to be here. > Source/WebCore/platform/graphics/FontPlatformData.h:139 > +#if PLATFORM(COCOA) > #endif I suggest removing this.
Landed in r177527.