Summary: | [iOS] getsectdata() is deprecated and we use it in DRT | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||||
Component: | Tools / Tests | Assignee: | Myles C. Maxfield <mmaxfield> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ap, dbates, ddkilzer, dino, jonlee, simon.fraser, thorton | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Myles C. Maxfield
2014-06-20 14:11:42 PDT
Created attachment 233453 [details]
Patch
Comment on attachment 233453 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=233453&action=review > Tools/DumpRenderTree/mac/DumpRenderTree.mm:536 > - char* fontData = getsectdata("__DATA", fontSectionNames[i], &fontDataLength); > + const uint8_t* const fontData = getsectiondata(_NSGetMachExecuteHeader(), "__DATA", fontSectionNames[i], &fontDataLength); The recommended replacement for getsectdata is magic linker symbols, see <rdar://problem/14122320>. As previously mentioned in bug 127730, it's unfortunate that we use different mechanisms for built-in fonts on Mac and on iOS. Comment on attachment 233453 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=233453&action=review >> Tools/DumpRenderTree/mac/DumpRenderTree.mm:536 >> + const uint8_t* const fontData = getsectiondata(_NSGetMachExecuteHeader(), "__DATA", fontSectionNames[i], &fontDataLength); > > The recommended replacement for getsectdata is magic linker symbols, see <rdar://problem/14122320>. > > As previously mentioned in bug 127730, it's unfortunate that we use different mechanisms for built-in fonts on Mac and on iOS. It looks like the magic linker symbols rely on strings of the form "section$start$__DATA$__my" which means it will be tricky to iterate over an array of sections. I'll see what I can do. Comment on attachment 233453 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=233453&action=review >>> Tools/DumpRenderTree/mac/DumpRenderTree.mm:536 >>> + const uint8_t* const fontData = getsectiondata(_NSGetMachExecuteHeader(), "__DATA", fontSectionNames[i], &fontDataLength); >> >> The recommended replacement for getsectdata is magic linker symbols, see <rdar://problem/14122320>. >> >> As previously mentioned in bug 127730, it's unfortunate that we use different mechanisms for built-in fonts on Mac and on iOS. > > It looks like the magic linker symbols rely on strings of the form "section$start$__DATA$__my" which means it will be tricky to iterate over an array of sections. I'll see what I can do. Yeah, __asm() requires a string literal, which means I can't even do this iteration using template parameters. We may be stuck with getsectiondata :( Comment on attachment 233453 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=233453&action=review >>>> Tools/DumpRenderTree/mac/DumpRenderTree.mm:536 >>>> + const uint8_t* const fontData = getsectiondata(_NSGetMachExecuteHeader(), "__DATA", fontSectionNames[i], &fontDataLength); >>> >>> The recommended replacement for getsectdata is magic linker symbols, see <rdar://problem/14122320>. >>> >>> As previously mentioned in bug 127730, it's unfortunate that we use different mechanisms for built-in fonts on Mac and on iOS. >> >> It looks like the magic linker symbols rely on strings of the form "section$start$__DATA$__my" which means it will be tricky to iterate over an array of sections. I'll see what I can do. > > Yeah, __asm() requires a string literal, which means I can't even do this iteration using template parameters. We may be stuck with getsectiondata :( Actually, I could likely do it with a preprocessor macro I don't have a strong opinion, just pointing out what I was previously told. If using magic symbols is hard, then we shouldn't do that. Created attachment 233567 [details]
Patch
Comment on attachment 233567 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=233567&action=review > Tools/DumpRenderTree/mac/DumpRenderTree.mm:532 > + if (!GSFontAddCGFont(cgFont)) { We can probably use CTFontManagerEnableFontDescriptors() instead, relieving the dependency on GraphicsServices (In reply to comment #8) > (From update of attachment 233567 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=233567&action=review > > > Tools/DumpRenderTree/mac/DumpRenderTree.mm:532 > > + if (!GSFontAddCGFont(cgFont)) { > > We can probably use CTFontManagerEnableFontDescriptors() instead, relieving the dependency on GraphicsServices https://bugs.webkit.org/show_bug.cgi?id=134133 Comment on attachment 233567 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=233567&action=review Nice! > Tools/DumpRenderTree/mac/DumpRenderTree.mm:517 > +static void activateFontIOS(const uint8_t* ptr, unsigned long length, std::string sectionName) I suggest using a more descriptive name for "ptr" (maybe fontData?) > Tools/DumpRenderTree/mac/DumpRenderTree.mm:519 > + CGDataProviderRef data = CGDataProviderCreateWithData(NULL, ptr, length, NULL); s/NULL/nullptr/g > Tools/DumpRenderTree/mac/DumpRenderTree.mm:538 > +} > +static void activateFontsIOS() Missing empty line. |