RESOLVED FIXED Bug 201851
Remove some support for < iOS 12
https://bugs.webkit.org/show_bug.cgi?id=201851
Summary Remove some support for < iOS 12
Keith Rollin
Reported 2019-09-16 17:29:09 PDT
Remove some support for iOS versions less than 13.0. Update conditionals that reference __IPHONE_OS_VERSION_MIN_REQUIRED and __MAC_OS_X_VERSION_MAX_ALLOWED, assuming that they both have values >= 130000. This means that expressions like "__IPHONE_OS_VERSION_MIN_REQUIRED < 101300" are always False and "__IPHONE_OS_VERSION_MIN_REQUIRED >= 101300" are always True. This removal is part of a series of patches effecting such removal.
Attachments
Patch (7.84 KB, patch)
2019-09-16 17:34 PDT, Keith Rollin
no flags
Patch (7.84 KB, patch)
2019-09-17 09:25 PDT, Keith Rollin
no flags
Patch (7.90 KB, patch)
2019-09-17 12:35 PDT, Keith Rollin
no flags
Radar WebKit Bug Importer
Comment 1 2019-09-16 17:29:20 PDT
Keith Rollin
Comment 2 2019-09-16 17:33:12 PDT
s/__MAC_OS_X_VERSION_MAX_ALLOWED/__IPHONE_OS_VERSION_MAX_ALLOWED/
Keith Rollin
Comment 3 2019-09-16 17:34:27 PDT
Jiewen Tan
Comment 4 2019-09-16 17:45:50 PDT
Comment on attachment 378919 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=378919&action=review > Source/WebCore/PAL/pal/spi/cocoa/IOSurfaceSPI.h:-95 > -#if (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000) || PLATFORM(MAC) Any reason why PLATFORM(MAC) is removed as well? > Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mm:146 > +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400) || (PLATFORM(IOS) && !PLATFORM(IOS_FAMILY_SIMULATOR)) Any reason why PLATFORM(IOS_FAMILY) is not used anymore?
Keith Rollin
Comment 5 2019-09-16 17:52:03 PDT
(In reply to Jiewen Tan from comment #4) > > Source/WebCore/PAL/pal/spi/cocoa/IOSurfaceSPI.h:-95 > > -#if (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000) || PLATFORM(MAC) > > Any reason why PLATFORM(MAC) is removed as well? When removing the version test, the expression becomes #if PLATFORM(IOS_FAMILY) || PLATFORM(MAC) This in turn means #if PLATFORM(COCOA). And this is a Cocoa-only file, so we can already assume PLATFORM(COCOA) is true. > > Tools/WebKitTestRunner/InjectedBundle/cocoa/ActivateFontsCocoa.mm:146 > > +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400) || (PLATFORM(IOS) && !PLATFORM(IOS_FAMILY_SIMULATOR)) > > Any reason why PLATFORM(IOS_FAMILY) is not used anymore? I talked with Kilzer about this and that's what he recommended.
Keith Rollin
Comment 6 2019-09-16 18:11:51 PDT
(In reply to Keith Rollin from comment #5) > (In reply to Jiewen Tan from comment #4) > > Any reason why PLATFORM(IOS_FAMILY) is not used anymore? > > I talked with Kilzer about this and that's what he recommended. I should elaborate. First, Kilzer was the one who added the code, so that's what I talked with him. Second, the check originally was for IOS; it was changed to IOS_FAMILY in ap's Grand Change. Third, tvOS and watchOS were filtered out due to the iOS version check, so IOS_FAMILY was effectively IOS anyway.
Jiewen Tan
Comment 7 2019-09-16 18:18:34 PDT
(In reply to Keith Rollin from comment #6) > (In reply to Keith Rollin from comment #5) > > (In reply to Jiewen Tan from comment #4) > > > Any reason why PLATFORM(IOS_FAMILY) is not used anymore? > > > > I talked with Kilzer about this and that's what he recommended. > > I should elaborate. First, Kilzer was the one who added the code, so that's > what I talked with him. Second, the check originally was for IOS; it was > changed to IOS_FAMILY in ap's Grand Change. Third, tvOS and watchOS were > filtered out due to the iOS version check, so IOS_FAMILY was effectively IOS > anyway. Got you. LGTM. r=me.
Alexey Proskuryakov
Comment 8 2019-09-16 20:20:03 PDT
Please don’t land this yet, WebKit CI is still on iOS 12.
Keith Rollin
Comment 9 2019-09-16 20:22:13 PDT
This patch doesn't actually remove support for iOS 12. These particular changes were chosen at this time because they affect iOS 11 and older.
Alexey Proskuryakov
Comment 10 2019-09-16 20:22:40 PDT
Also, how is this building on EWS?!
Keith Rollin
Comment 11 2019-09-16 20:27:35 PDT
Why wouldn't it build on EWS?
Alexey Proskuryakov
Comment 12 2019-09-17 09:21:51 PDT
> This patch doesn't actually remove support for iOS 12. These particular changes were chosen at this time because they affect iOS 11 and older. Please re-title accordingly before landing. It's super misleading to land a patch that says "Remove some support for < iOS 13" when it doesn't. > Why wouldn't it build on EWS? EWS is on iOS 12, and you claimed that you removed support for it.
Keith Rollin
Comment 13 2019-09-17 09:25:50 PDT
WebKit Commit Bot
Comment 14 2019-09-17 12:29:40 PDT
Comment on attachment 378970 [details] Patch Rejecting attachment 378970 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 378970, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!. Full output: https://webkit-queues.webkit.org/results/13040584
Keith Rollin
Comment 15 2019-09-17 12:35:52 PDT
WebKit Commit Bot
Comment 16 2019-09-17 13:20:44 PDT
Comment on attachment 378979 [details] Patch Clearing flags on attachment: 378979 Committed r249978: <https://trac.webkit.org/changeset/249978>
WebKit Commit Bot
Comment 17 2019-09-17 13:20:46 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.