WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
201777
iOS 13: Some SPI targets 13.1
https://bugs.webkit.org/show_bug.cgi?id=201777
Summary
iOS 13: Some SPI targets 13.1
Jonathan Bedard
Reported
Friday, September 13, 2019 11:18:31 PM UTC
There are a number of SPIs which aren't available in the 13.0 SDK, and are instead for 13.1. Our macros should reflect this.
Attachments
Patch
(1.46 KB, patch)
2019-09-13 15:31 PDT
,
Jonathan Bedard
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Jonathan Bedard
Comment 1
Friday, September 13, 2019 11:31:56 PM UTC
Created
attachment 378755
[details]
Patch
Alexey Proskuryakov
Comment 2
Saturday, September 14, 2019 12:04:00 AM UTC
Comment on
attachment 378755
[details]
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.
Jonathan Bedard
Comment 3
Saturday, September 14, 2019 12:17:25 AM UTC
(In reply to Alexey Proskuryakov from
comment #2
)
> Comment on
attachment 378755
[details]
> 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.
I file <
https://bugs.webkit.org/show_bug.cgi?id=201781
>...I'd like to get those changes right, seems like the addition of the IOS_FAMILY macro has made some things more confusing.
Alexey Proskuryakov
Comment 4
Saturday, September 14, 2019 12:21:37 AM UTC
> seems like the addition of the IOS_FAMILY macro has made some things more confusing.
PLATFORM(IOS_FAMILY) is a new name for PLATFORM(IOS), which was a misnomer. It's less confusing, but it uncovered a lot of incorrect assumptions that people made thinking that PLATFORM(IOS) is only true on iOS. Keith Rollin recently fixed many of those.
WebKit Commit Bot
Comment 5
Saturday, September 14, 2019 1:00:51 AM UTC
Comment on
attachment 378755
[details]
Patch Clearing flags on attachment: 378755 Committed
r249859
: <
https://trac.webkit.org/changeset/249859
>
WebKit Commit Bot
Comment 6
Saturday, September 14, 2019 1:00:53 AM UTC
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 7
Saturday, September 14, 2019 1:04:29 AM UTC
<
rdar://problem/55358068
>
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