More Cocoa HAVE directives cleanup #47367

annevk · 2025-06-30T07:38:38Z

`0bdb0ca`

More Cocoa HAVE directives cleanup
https://bugs.webkit.org/show_bug.cgi?id=295192

Reviewed by Per Arne Vollan.

These all appear to be leftovers from a time when they were targeting
specific OS versions.

* Source/WebCore/page/PageGroup.cpp:

PLATFORM(MAC) is redundant with HAVE(MEDIA_ACCESSIBILITY_FRAMEWORK).

* Source/WebCore/platform/cocoa/CoreLocationGeolocationProvider.mm:

defined(CL_HAS_RADAR_88834301) has been around for long enough now that
we don't have to mention it explicitly anymore.

Canonical link: https://commits.webkit.org/296906@main

f7ed280

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	⏳ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge	✅ 🛠 tv	✅ 🛠 mac-safer-cpp	✅ 🛠 jsc-armv7
	✅ 🛠 tv-sim		❌ 🧪 jsc-armv7-tests
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2025-06-30T07:38:46Z

EWS run on previous version of this PR (hash 0f5fe81)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	❌ 🛠 ios	❌ 🛠 mac	✅ 🛠 wpe	~~🛠 win~~
✅ 🧪 bindings	❌ 🛠 ios-sim	❌ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	~~🧪 win-tests~~
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	❌ 🧪 api-mac	✅ 🧪 api-wpe
	❌ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	❌ 🧪 api-ios	❌ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	❌ 🛠 vision	❌ 🧪 mac-AS-debug-wk2	~~🧪 gtk-wk2~~
	❌ 🛠 vision-sim	❌ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	⏳ 🧪 vision-wk2	❌ 🧪 mac-intel-wk2	✅ 🛠 playstation
	❌ 🛠 tv	❌ 🛠 mac-safer-cpp	✅ 🛠 jsc-armv7
	❌ 🛠 tv-sim		❌ 🧪 jsc-armv7-tests
	❌ 🛠 watch
	❌ 🛠 watch-sim

webkit-early-warning-system · 2025-06-30T08:26:47Z

EWS run on previous version of this PR (hash 9bb5443)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	⏳ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp	✅ 🛠 jsc-armv7
	✅ 🛠 tv-sim		❌ 🧪 jsc-armv7-tests
	✅ 🛠 watch
	✅ 🛠 watch-sim

pvollan · 2025-06-30T21:17:09Z

Source/WebCore/html/MediaElementSession.cpp

@@ -330,10 +330,6 @@ void MediaElementSession::isVisibleInViewportChanged()
    RefPtr element = m_element.get();
    if (!element || element->isFullscreen() || element->isVisibleInViewport())
        m_elementIsHiddenUntilVisibleInViewport = false;
-
-#if PLATFORM(COCOA) && !HAVE(CGS_FIX_FOR_RADAR_97530095)


Should this be removed when HAVE(CGS_FIX_FOR_RADAR_97530095) is 0 on Cocoa platforms?

Oh oops, thank you! I just went by the radar being long since fixed, but I did not mean to introduce a functional change. Let me take that out and perhaps follow-up on it separately at some point.

webkit-early-warning-system · 2025-07-01T07:31:10Z

EWS run on current version of this PR (hash f7ed280)

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	⏳ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2	✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge	✅ 🛠 tv	✅ 🛠 mac-safer-cpp	✅ 🛠 jsc-armv7
	✅ 🛠 tv-sim		❌ 🧪 jsc-armv7-tests
	✅ 🛠 watch
	✅ 🛠 watch-sim

pvollan · 2025-07-01T14:48:21Z

Source/WebCore/platform/cocoa/CoreLocationGeolocationProvider.mm

@@ -70,7 +70,7 @@ - (instancetype)initWithWebsiteIdentifier:(const String&)websiteIdentifier clien
    _isWaitingForAuthorization = YES;
    _mode = mode;

-#if USE(APPLE_INTERNAL_SDK) && HAVE(CORE_LOCATION_WEBSITE_IDENTIFIERS) && defined(CL_HAS_RADAR_88834301)


It looks like CL_HAS_RADAR_88834301 is not defined. Can the code inside #If be removed, then?

That's what I thought initially, but It's defined in the Cocoa framework we use that introduces this API. Maybe @cdumez should double check this though since he introduced this code.

Ah, good point! I did not realize it was defined there.

@pvollan Chris confirmed out-of-band that this is okay so I think this PR is good for another round.

pvollan

R=me.

https://bugs.webkit.org/show_bug.cgi?id=295192 Reviewed by Per Arne Vollan. These all appear to be leftovers from a time when they were targeting specific OS versions. * Source/WebCore/page/PageGroup.cpp: PLATFORM(MAC) is redundant with HAVE(MEDIA_ACCESSIBILITY_FRAMEWORK). * Source/WebCore/platform/cocoa/CoreLocationGeolocationProvider.mm: defined(CL_HAS_RADAR_88834301) has been around for long enough now that we don't have to mention it explicitly anymore. Canonical link: https://commits.webkit.org/296906@main

webkit-commit-queue · 2025-07-02T12:58:21Z

Committed 296906@main (0bdb0ca): https://commits.webkit.org/296906@main

Reviewed commits have been landed. Closing PR #47367 and removing active labels.

annevk self-assigned this Jun 30, 2025

annevk added the WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore). label Jun 30, 2025

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 30, 2025

annevk removed the merging-blocked Applied to prevent a change from being merged label Jun 30, 2025

annevk force-pushed the eng/More-Cocoa-HAVE-directives-cleanup branch from 0f5fe81 to 9bb5443 Compare June 30, 2025 08:26

annevk marked this pull request as ready for review June 30, 2025 09:06

annevk requested review from cdumez and rniwa as code owners June 30, 2025 09:06

pvollan reviewed Jun 30, 2025

View reviewed changes

annevk force-pushed the eng/More-Cocoa-HAVE-directives-cleanup branch from 9bb5443 to f7ed280 Compare July 1, 2025 07:31

annevk requested a review from pvollan July 1, 2025 07:31

pvollan reviewed Jul 1, 2025

View reviewed changes

pvollan approved these changes Jul 2, 2025

View reviewed changes

annevk added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jul 2, 2025

webkit-commit-queue force-pushed the eng/More-Cocoa-HAVE-directives-cleanup branch from f7ed280 to 0bdb0ca Compare July 2, 2025 12:58

webkit-commit-queue merged commit 0bdb0ca into WebKit:main Jul 2, 2025

webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jul 2, 2025

annevk deleted the eng/More-Cocoa-HAVE-directives-cleanup branch July 2, 2025 13:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

More Cocoa HAVE directives cleanup #47367

More Cocoa HAVE directives cleanup #47367

annevk commented Jun 30, 2025 •

edited by webkit-early-warning-system

Loading

Uh oh!

webkit-early-warning-system commented Jun 30, 2025 •

edited

Loading

Uh oh!

webkit-early-warning-system commented Jun 30, 2025 •

edited

Loading

Uh oh!

pvollan Jun 30, 2025

Uh oh!

annevk Jul 1, 2025

Uh oh!

webkit-early-warning-system commented Jul 1, 2025 •

edited

Loading

Uh oh!

pvollan Jul 1, 2025 •

edited

Loading

Uh oh!

annevk Jul 1, 2025

Uh oh!

pvollan Jul 1, 2025

Uh oh!

annevk Jul 2, 2025

Uh oh!

pvollan left a comment

Uh oh!

webkit-commit-queue commented Jul 2, 2025

Uh oh!

Uh oh!

More Cocoa HAVE directives cleanup #47367

More Cocoa HAVE directives cleanup #47367

Conversation

annevk commented Jun 30, 2025 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

0bdb0ca

Uh oh!

webkit-early-warning-system commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pvollan Jun 30, 2025

Choose a reason for hiding this comment

Uh oh!

annevk Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

webkit-early-warning-system commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pvollan Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

annevk Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

pvollan Jul 1, 2025

Choose a reason for hiding this comment

Uh oh!

annevk Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

pvollan left a comment

Choose a reason for hiding this comment

Uh oh!

webkit-commit-queue commented Jul 2, 2025

Uh oh!

Uh oh!

annevk commented Jun 30, 2025 •

edited by webkit-early-warning-system

Loading

`0bdb0ca`

webkit-early-warning-system commented Jun 30, 2025 •

edited

Loading

webkit-early-warning-system commented Jun 30, 2025 •

edited

Loading

webkit-early-warning-system commented Jul 1, 2025 •

edited

Loading

pvollan Jul 1, 2025 •

edited

Loading