[webkit-reviews] review denied: [Bug 206096] Remove old iOS version macros : [Attachment 387405] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Jonathan Bedard
<jbedard at apple.com>'s request for review:
Bug 206096: Remove old iOS version macros
https://bugs.webkit.org/show_bug.cgi?id=206096

Attachment 387405: Patch

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




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

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

> Source/WebCore/PAL/pal/spi/cf/CFNetworkSPI.h:216
> -#if PLATFORM(MAC) || (PLATFORM(IOS_FAMILY) &&
__IPHONE_OS_VERSION_MIN_REQUIRED >= 110000)
> +#if PLATFORM(MAC) || PLATFORM(IOS_FAMILY)

I don't think that this is a no-op. Isn't __IPHONE_OS_VERSION_MIN_REQUIRED
frozen at a lower number on watchOS and tvOS? I think that this is actually
"Mac or iOS or macCatalyst".

This change may or may not be desirable, but this patch shouldn't change
behavior of course.

Really, all these things should be in Platform.h, not inline. It seems OK to
leave those that will go away soon, but those that encode decisions like
excluding watchOS and tvOS should be centralized. It is a lot of work, so I
don't want to block this patch on it, but it would be a substantial
improvement.

> Source/WebCore/PAL/pal/spi/cg/CoreGraphicsSPI.h:285
> -#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400) ||
(PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 120000)
> +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400) ||
PLATFORM(IOS_FAMILY)

Ditto.

> Source/WebCore/PAL/pal/spi/ios/MediaPlayerSPI.h:-37
> -#if __IPHONE_OS_VERSION_MIN_REQUIRED >= 120000

Ditto. I stopped looking at this point, please check the rest of the patch.