[webkit-reviews] review granted: [Bug 201777] iOS 13: Some SPI targets 13.1 : [Attachment 378755] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Jonathan Bedard
<jbedard at apple.com>'s request for review:
Bug 201777: iOS 13: Some SPI targets 13.1
https://bugs.webkit.org/show_bug.cgi?id=201777

Attachment 378755: Patch

https://bugs.webkit.org/attachment.cgi?id=378755&action=review




--- Comment #2 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 378755
  --> https://bugs.webkit.org/attachment.cgi?id=378755
Patch

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

r=me, because the issues are pre-existing.

> Source/WTF/wtf/Platform.h:1490
> +#if PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000 &&
__IPHONE_OS_VERSION_MAX_ALLOWED >= 130100

PLATFORM(IOS_FAMILY) here is weird. The check for
__IPHONE_OS_VERSION_MIN_REQUIRED makes it so that the code is only enabled on
iOS, so we should change it to PLATFORM(IOS).

But maybe this is not intentional? May be worth checking with a domain expert.

> Source/WTF/wtf/Platform.h:1544
> +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500) ||
(PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000 &&
__IPHONE_OS_VERSION_MAX_ALLOWED >= 130100)

Same here, PLATFORM(IOS_FAMILY) is either misleading or incorrect.

To bee 100% precise, we would need a HAVE macro that would be accurate on all
platforms that have the function, and a USE macro that tells use where we
choose to use it.