Bug 190887 - Clean up some obsolete macOS version guards
Summary: Clean up some obsolete macOS version guards
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-10-24 14:22 PDT by Alexey Proskuryakov
Modified: 2018-10-24 17:20 PDT (History)
4 users (show)

See Also:


Attachments
proposed patch (22.32 KB, patch)
2018-10-24 14:36 PDT, Alexey Proskuryakov
ap: commit-queue-
Details | Formatted Diff | Diff
proposed patch (19.79 KB, patch)
2018-10-24 15:53 PDT, Alexey Proskuryakov
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2018-10-24 14:22:17 PDT
Not building for 10.11 any more.

Can't do iOS yet, because most of the obsolete guards actually act as checks for iOS proper (as opposed to watchOS et al.)
Comment 1 Alexey Proskuryakov 2018-10-24 14:36:04 PDT
Created attachment 353055 [details]
proposed patch

This should be no-op, except for the change in Source/WebKit/PluginProcess/mac/com.apple.WebKit.plugin-common.sb.in.

In that file, we have some rules guarded with "__MAC_OS_X_VERSION_MIN_REQUIRED < 101240". MIN_REQUIRED doesn't change with SDK, so the code was actually still built on macOS Sierra even after 10.12.4 update. But looks like the intention was that it wouldn't be included, so I removed it.
Comment 2 mitz 2018-10-24 14:49:46 PDT
WebKitAvailability.h is a public header. Are these changes source-compatible with projects targeting 10.10 and earlier?
Comment 3 Alexey Proskuryakov 2018-10-24 15:15:51 PDT
Comment on attachment 353055 [details]
proposed patch

Good catch. I'll undo this part if I can get r+ on the rest.
Comment 4 Alexey Proskuryakov 2018-10-24 15:53:37 PDT
Created attachment 353061 [details]
proposed patch

Had to fix Windows build, so also addressed Dan's comment.
Comment 5 mitz 2018-10-24 16:40:27 PDT
Comment on attachment 353061 [details]
proposed patch

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

r=me

> Source/WebKit/PluginProcess/mac/com.apple.WebKit.plugin-common.sb.in:-333
> -#if __MAC_OS_X_VERSION_MIN_REQUIRED < 101240

It’s not clear if we’re gaining anything at this point from changing the code from “as shipped and tested” to “as intended”. For clarity and safety, we could just change this to < 101300.
Comment 6 Alexey Proskuryakov 2018-10-24 16:50:15 PDT
Looking at history of related issues, I agree.
Comment 7 Alexey Proskuryakov 2018-10-24 17:19:35 PDT
Committed http://trac.webkit.org/r237405
Comment 8 Radar WebKit Bug Importer 2018-10-24 17:20:27 PDT
<rdar://problem/45539674>