[Cocoa] User-installed fonts don't work in the GPU Process (in WKWebView)
Created attachment 449673 [details] WIP
<rdar://problem/84958961>
Comment on attachment 449673 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=449673&action=review > Source/WebCore/PAL/pal/spi/cf/CoreTextSPI.h:99 > +typedef CF_OPTIONS(uint32_t, CTFontDescriptorOptions) { > +}; Apparently there has to be something inside the {}s :(
Created attachment 449687 [details] Patch
Created attachment 449689 [details] Patch
Comment on attachment 449689 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449689&action=review > Source/WebCore/PAL/pal/spi/cf/CoreTextSPI.h:100 > +typedef CF_OPTIONS(uint32_t, CTFontDescriptorOptions) { > + kCTFontDescriptorOptionThisIsNotARealOption = 0xFFFFFFFF > +}; Or we could just typedef it to uint32_t. > Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:468 > encoder << String(string); Seems a shame we have to convert CFString to String just to encode it on the wire. A repeated pattern.
Comment on attachment 449689 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449689&action=review >> Source/WebCore/PAL/pal/spi/cf/CoreTextSPI.h:100 >> +}; > > Or we could just typedef it to uint32_t. Do you think the typedef is preferable? I think I actually prefer the CF_OPTIONS thing, just because it more closely matches the original source. I don't want to delay this bug fix any longer, so I'll land this as is and if you prefer the typedef I'll make a follow-up patch. >> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:468 >> encoder << String(string); > > Seems a shame we have to convert CFString to String just to encode it on the wire. A repeated pattern. I believe we send CFStrings in other places ... let me see if I can fix this...
Comment on attachment 449689 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449689&action=review >>> Source/WebCore/PAL/pal/spi/cf/CoreTextSPI.h:100 >>> +}; >> >> Or we could just typedef it to uint32_t. > > Do you think the typedef is preferable? I think I actually prefer the CF_OPTIONS thing, just because it more closely matches the original source. > > I don't want to delay this bug fix any longer, so I'll land this as is and if you prefer the typedef I'll make a follow-up patch. This is fine. I didn’t mean to cause delay!
Comment on attachment 449689 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449689&action=review >>> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:468 >>> encoder << String(string); >> >> Seems a shame we have to convert CFString to String just to encode it on the wire. A repeated pattern. > > I believe we send CFStrings in other places ... let me see if I can fix this... Oh, yeah, of course - ArgumentCodersCF.h!
Comment on attachment 449689 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449689&action=review >>>> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:468 >>>> encoder << String(string); >>> >>> Seems a shame we have to convert CFString to String just to encode it on the wire. A repeated pattern. >> >> I believe we send CFStrings in other places ... let me see if I can fix this... > > Oh, yeah, of course - ArgumentCodersCF.h! Should find the others who are calling String() just to encode a CFStringRef. I bet this is not the only place.
Created attachment 449891 [details] Patch for committing
Created attachment 449892 [details] Patch for committing
Created attachment 449893 [details] Patch for committing
Comment on attachment 449893 [details] Patch for committing View in context: https://bugs.webkit.org/attachment.cgi?id=449893&action=review Fine to land as-is, but I see one further refinement. > Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:486 > auto currentPostScriptName = adoptCF(static_cast<CFStringRef>(CTFontDescriptorCopyAttribute(fontDescriptor, kCTFontNameAttribute))); Given we only call CFEqual on this, we don’t need the static_cast to CFStringRef. I suggest just taking it out.
Comment on attachment 449893 [details] Patch for committing View in context: https://bugs.webkit.org/attachment.cgi?id=449893&action=review >> Source/WebKit/Shared/Cocoa/WebCoreArgumentCodersCocoa.mm:486 >> auto currentPostScriptName = adoptCF(static_cast<CFStringRef>(CTFontDescriptorCopyAttribute(fontDescriptor, kCTFontNameAttribute))); > > Given we only call CFEqual on this, we don’t need the static_cast to CFStringRef. I suggest just taking it out. 🆒 I think I agree with you. This is more readable now.
Committed r288543 (246375@trunk): <https://commits.webkit.org/246375@trunk>