Bug 200937 – Remove support for watchOS < 6.0

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Keith Rollin]

Update conditionals that reference __WATCH_OS_X_VERSION_MIN_REQUIRED and __WATCH_OS_X_VERSION_MAX_ALLOWED, assuming that they both have values >= 50000. This means that expressions like "__WATCH_OS_X_VERSION_MIN_REQUIRED < 50000" are always False and "__WATCH_OS_X_VERSION_MIN_REQUIRED >= 50000" are always True.

[Radar WebKit Bug Importer]

<rdar://problem/54524009>

[Keith Rollin]

s/OS_X/OS/g

[Keith Rollin]

Created attachment 376793 [details]
Patch

[EWS Watchlist]

Attachment 376793 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:141:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:146:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Source/WebCore/PAL/pal/spi/cocoa/NSKeyedArchiverSPI.h:32:  Incorrect OS version check. VERSION_MIN_REQUIRED values never include a minor version. You may be looking for a combination of VERSION_MIN_REQUIRED for target OS version check and VERSION_MAX_ALLOWED for SDK check.  [build/version_check] [5]
ERROR: Source/WebCore/PAL/pal/spi/cocoa/NSKeyedArchiverSPI.h:32:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
Total errors found: 4 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keith Rollin]

All of those style errors are pre-existing. Some will be cleaned up in subsequent patches that remove support for older OSes on other platforms.

[Keith Rollin]

Comment on attachment 376793 [details]
Patch

I need to check for 6.0, not 5.0.

[Keith Rollin]

Created attachment 376816 [details]
Patch

[EWS Watchlist]

Attachment 376816 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:141:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:146:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebCore/cocoa/AVFoundationSoftLinkTest.mm:161:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Source/WebCore/PAL/pal/spi/cocoa/NSKeyedArchiverSPI.h:32:  Incorrect OS version check. VERSION_MIN_REQUIRED values never include a minor version. You may be looking for a combination of VERSION_MIN_REQUIRED for target OS version check and VERSION_MAX_ALLOWED for SDK check.  [build/version_check] [5]
ERROR: Source/WebCore/PAL/pal/spi/cocoa/NSKeyedArchiverSPI.h:32:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Source/WebCore/PAL/pal/spi/cocoa/NSKeyedArchiverSPI.h:34:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
ERROR: Source/WebCore/PAL/pal/spi/cocoa/NSProgressSPI.h:30:  Misplaced OS version check. Please use a named macro in wtf/Platform.h, wtf/FeatureDefines.h, or an appropriate internal file.  [build/version_check] [5]
Total errors found: 7 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keith Rollin]

All of those style errors are pre-existing. Some will be cleaned up in subsequent patches that remove support for older OSes on other platforms.

[Darin Adler]

Comment on attachment 376816 [details]
Patch

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

> Source/WTF/wtf/FeatureDefines.h:195
> +#if (__IPHONE_OS_VERSION_MIN_REQUIRED >= 130000) || (PLATFORM(APPLETV) && __TV_OS_VERSION_MIN_REQUIRED >= 130000) || PLATFORM(WATCHOS)

Should check what __IPHONE_OS_VERSION_MIN_REQUIRED is set to on watchOS 6.0. If it's >= 130000, the PLATFORM(WATCHOS) clause is not needed.

> Source/WTF/wtf/Platform.h:1620
> +#if PLATFORM(MAC) || (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000) || PLATFORM(WATCHOS) || (PLATFORM(APPLETV) && __TV_OS_VERSION_MIN_REQUIRED >= 110000)

If IOS_FAMILY includes watchOS, should check what __IPHONE_OS_VERSION_MIN_REQUIRED is set to on watchOS 6.0. If it's >= 110000, the PLATFORM(WATCHOS) clause is not needed.

> Source/WTF/wtf/Platform.h:1628
> +#if (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000) || PLATFORM(WATCHOS) || (PLATFORM(APPLETV) && __TV_OS_VERSION_MIN_REQUIRED >= 130000)

If IOS_FAMILY includes watchOS, should check what __IPHONE_OS_VERSION_MIN_REQUIRED is set to on watchOS 6.0. If it's >= 130000, the PLATFORM(WATCHOS) clause is not needed.

> Source/WebCore/PAL/pal/spi/cocoa/NSKeyedArchiverSPI.h:32
> +#define USE_SECURE_ARCHIVER_API (PLATFORM(MAC) || (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 110200) || PLATFORM(WATCHOS) || (PLATFORM(APPLETV) && __TV_OS_VERSION_MIN_REQUIRED >= 110200))

If IOS_FAMILY includes watchOS, should check what __IPHONE_OS_VERSION_MIN_REQUIRED is set to on watchOS 6.0. If it's >= 110200, the PLATFORM(WATCHOS) clause is not needed.

> Source/WebCore/PAL/pal/spi/cocoa/NSKeyedArchiverSPI.h:34
> +#define USE_SECURE_ARCHIVER_FOR_ATTRIBUTED_STRING (PLATFORM(MAC) || (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000) || PLATFORM(WATCHOS) || (PLATFORM(APPLETV) && __TV_OS_VERSION_MIN_REQUIRED >= 130000))

If IOS_FAMILY includes watchOS, should check what __IPHONE_OS_VERSION_MIN_REQUIRED is set to on watchOS 6.0. If it's >= 130000, the PLATFORM(WATCHOS) clause is not needed.

[Tim Horton]

(In reply to Darin Adler from comment #10)
> Comment on attachment 376816 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=376816&action=review
> 
> > Source/WTF/wtf/FeatureDefines.h:195
> > +#if (__IPHONE_OS_VERSION_MIN_REQUIRED >= 130000) || (PLATFORM(APPLETV) && __TV_OS_VERSION_MIN_REQUIRED >= 130000) || PLATFORM(WATCHOS)
> 
> Should check what __IPHONE_OS_VERSION_MIN_REQUIRED is set to on watchOS 6.0.
> If it's >= 130000, the PLATFORM(WATCHOS) clause is not needed.

My understanding is that the iPhone version is stuck where it branched off (9?) forever.

[Keith Rollin]

(In reply to Tim Horton from comment #11)
> My understanding is that the iPhone version is stuck where it branched off
> (9?) forever.

My understanding matches Tim's. I went through and printed out all the values of the various TARGET_, WTF_OS_, WTF_PLATFORM_, etc. variables, and noticed that the IOS versions are defined to 90000 for watch and tv.

[WebKit Commit Bot]

Comment on attachment 376816 [details]
Patch

Clearing flags on attachment: 376816

Committed r248950: <https://trac.webkit.org/changeset/248950>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

(In reply to Keith Rollin from comment #12)
> (In reply to Tim Horton from comment #11)
> > My understanding is that the iPhone version is stuck where it branched off
> > (9?) forever.
> 
> My understanding matches Tim's. I went through and printed out all the
> values of the various TARGET_, WTF_OS_, WTF_PLATFORM_, etc. variables, and
> noticed that the IOS versions are defined to 90000 for watch and tv.

OK, got it.

Given that, it seems we should almost never check PLATFORM(IOS_FAMILY) and then __IPHONE_OS_VERSION_MIN_REQUIRED. Seems like most of those are mistakes; if there are version checks then they ought to instead be only PLATFORM(IOS).

[Alexey Proskuryakov]

Correct, these are mistakes made when PLATFORM(IOS) was true for all iOS family targets. A renaming that was performed last fall made these more obvious. 

Most of the time we check with >=, so the mistakes are benign, and the behavior is validated by QA and user experience anyway, but I expect that we probably still have some interesting edge cases.

[Keith Rollin]

(In reply to Darin Adler from comment #15)
> (In reply to Keith Rollin from comment #12)
> > (In reply to Tim Horton from comment #11)
> > > My understanding is that the iPhone version is stuck where it branched off
> > > (9?) forever.
> > 
> > My understanding matches Tim's. I went through and printed out all the
> > values of the various TARGET_, WTF_OS_, WTF_PLATFORM_, etc. variables, and
> > noticed that the IOS versions are defined to 90000 for watch and tv.
> 
> OK, got it.
> 
> Given that, it seems we should almost never check PLATFORM(IOS_FAMILY) and
> then __IPHONE_OS_VERSION_MIN_REQUIRED. Seems like most of those are
> mistakes; if there are version checks then they ought to instead be only
> PLATFORM(IOS).

Agreed. That's my intent when I get to cleaning up the iOS bits. I've been saving it for the end after I've addressed the other platforms in order to simplify things first. Then I'll need to address cases where we check for iOS and version, iOS family and version where we really mean just iOS, where we check for iOS family and version where we really do mean iOS family, and places where we check just the version without checking the platform. This part of the project will really need to look into the intent of what we want, and can't just be a mechanical process of removing checks as was the case with the other platforms.