Bug 206841 – WebCore: Remove old iOS version macros

• Status: NEW
• Component: WebCore Misc.

[Jonathan Bedard]

We have a number of version detection macros which are really platform detection macros. These should be implemented in a way which makes their intention obvious.

[Jonathan Bedard]

Created attachment 388895 [details]
Patch

[Darin Adler]

Comment on attachment 388895 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=388895&action=review

All of this should go into PlatformHave.h I think

> Source/WebCore/page/SettingsDefaultValues.h:107
> +#if PLATFORM(COCOA) && !(PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101400) && !PLATFORM(APPLETV)

The condition is perfect here, but this seems to be HAVE(CG_CONTEXT_DRAW_CONIC_GRADIENT).

> Source/WebCore/platform/graphics/cg/GradientCG.cpp:185
> +#if PLATFORM(COCOA) && !(PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101400) && !PLATFORM(APPLETV)

Ditto.

> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:42
> +#define HAS_CORE_TEXT_WIDTH_ATTRIBUTE (PLATFORM(MAC) || PLATFORM(IOS) || PLATFORM(MACCATALYST))

Move to PlatformHave.h? And change the logic to:

    #if PLATFORM(COCOA) && !PLATFORM(WATCHOS) && !PLATFORM(APPLETV)

> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:1067
> +#if !PLATFORM(WATCHOS) && !PLATFORM(APPLETV)

I think this might be HAVE(CT_FONT_DESCRIPTOR_IS_SYSTEM_UI_FONT)

> Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp:1642
> +#if PLATFORM(IOS) || PLATFORM(MAC) || PLATFORM(MACCATALYST)

I think this might be HAVE(CT_FONT_DESCRIPTOR_CREATE_LAST_RESORT)

Also, it’s arbitrary to write this as the reverse of how we wrote the identical condition above.

> Source/WebCore/platform/graphics/cocoa/FontDescriptionCocoa.cpp:85
> +#if PLATFORM(IOS) || PLATFORM(MACCATALYST)

The other patch added HAVE(....STYLE_TITLE_0/4) so we should use that here.

> Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm:127
> +#define WORKAROUND_CORETEXT_VARIATIONS_WITH_FALLBACK_LIST_BUG (ENABLE(VARIATION_FONTS) && (PLATFORM(WATCHOS) || PLATFORM(APPLETV)))

I bet the real story is that we don’t need this workaround at all.

> Source/WebCore/platform/graphics/ios/FontCacheIOS.mm:159
> +#if !PLATFORM(WATCHOS) && !PLATFORM(APPLETV)

I think this might be HAVE(CT_FONT_DESCRIPTOR_CREATE_LAST_RESORT)

> Source/WebCore/platform/graphics/mac/FontCustomPlatformData.cpp:60
>      auto array = adoptCF(CTFontManagerCreateFontDescriptorsFromData(bufferData.get()));

I think this might be HAVE(CT_FONT_MANAGER_CREATE_FONT_DESCRIPTORS_FROM_DATA)

[Jonathan Bedard]

Comment on attachment 388895 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=388895&action=review

>> Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm:127
>> +#define WORKAROUND_CORETEXT_VARIATIONS_WITH_FALLBACK_LIST_BUG (ENABLE(VARIATION_FONTS) && (PLATFORM(WATCHOS) || PLATFORM(APPLETV)))
> 
> I bet the real story is that we don’t need this workaround at all.

Probably, I have a hit list of likely incorrect platform differences, and this is on it.

If I split this patch further, I wouldn't include this change, but the changes in WebCore were starting to get small, so I thought it would be better to bundle them together.

>> Source/WebCore/platform/graphics/mac/FontCustomPlatformData.cpp:60
>>      auto array = adoptCF(CTFontManagerCreateFontDescriptorsFromData(bufferData.get()));
> 
> I think this might be HAVE(CT_FONT_MANAGER_CREATE_FONT_DESCRIPTORS_FROM_DATA)

Differing bit here is actually the fact that watchOS and appleTV won't iterate through itemInCollection, I can't think of a HAVE(...) statement here that wouldn't make things more confusing.

[Jonathan Bedard]

Created attachment 389032 [details]
Patch

[Jonathan Bedard]

(In reply to Jonathan Bedard from comment #4)
> Created attachment 389032 [details]
> Patch

I'm not against splitting this patch up further, but the composite patches would start to become one-liners.

[Alexey Proskuryakov]

Style checker needs to be updated after Platform.h split :(