Bug 134122 – [iOS] getsectdata() is deprecated and we use it in DRT

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Myles C. Maxfield]

[iOS] getsectdata() is deprecated and we use it in DRT

[Myles C. Maxfield]

Created attachment 233453 [details]
Patch

[Alexey Proskuryakov]

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.

[Myles C. Maxfield]

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.

[Myles C. Maxfield]

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 :(

[Myles C. Maxfield]

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

[Alexey Proskuryakov]

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.

[Myles C. Maxfield]

Created attachment 233567 [details]
Patch

[Myles C. Maxfield]

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

[Myles C. Maxfield]

(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

[Alexey Proskuryakov]

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.

[Myles C. Maxfield]

http://trac.webkit.org/changeset/170362