WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(7.84 KB, patch)
2019-09-17 09:25 PDT
,
Keith Rollin
no flags
Details
Formatted Diff
Diff
Patch
(7.90 KB, patch)
2019-09-17 12:35 PDT
,
Keith Rollin
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2019-09-16 17:29:20 PDT
<
rdar://problem/55422044
>
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
Created
attachment 378919
[details]
Patch
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
Created
attachment 378970
[details]
Patch
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
Created
attachment 378979
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug