Skip to content

[SaferCpp] Adopt more smart pointers in UIProcess/API/C/ part 2 #44043

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 24, 2025
Merged

[SaferCpp] Adopt more smart pointers in UIProcess/API/C/ part 2 #44043

merged 1 commit into from
Jun 24, 2025
+24 −26

Conversation

zakariaridouh
Copy link
Contributor

@zakariaridouh zakariaridouh commented Apr 14, 2025 •
edited by webkit-early-warning-system
Loading

ba0fd77

[SaferCpp] Adopt more smart pointers in UIProcess/API/C/ part 2
https://bugs.webkit.org/show_bug.cgi?id=291511
rdar://149191086

Reviewed by Chris Dumez.

Adopt more smart pointers in:

-UIProcess/API/C/WKOpenPanelResultListener.cpp
-UIProcess/API/C/WKPageConfigurationRef.cpp
-UIProcess/API/C/WKProtectionSpace.cpp
-UIProcess/API/C/WKUserContentControllerRef.cpp

* Source/WebKit/SaferCPPExpectations/UncountedCallArgsCheckerExpectations:
* Source/WebKit/UIProcess/API/C/WKOpenPanelResultListener.cpp:
(WKOpenPanelResultListenerChooseMediaFiles):
(WKOpenPanelResultListenerChooseFiles):
(WKOpenPanelResultListenerCancel):
* Source/WebKit/UIProcess/API/C/WKPageConfigurationRef.cpp:
(WKPageConfigurationGetContext):
(WKPageConfigurationSetContext):
(WKPageConfigurationGetUserContentController):
(WKPageConfigurationSetUserContentController):
(WKPageConfigurationGetPreferences):
(WKPageConfigurationSetPreferences):
(WKPageConfigurationGetRelatedPage):
(WKPageConfigurationSetRelatedPage):
(WKPageConfigurationGetWebsiteDataStore):
(WKPageConfigurationSetWebsiteDataStore):
(WKPageConfigurationSetInitialCapitalizationEnabled):
(WKPageConfigurationSetBackgroundCPULimit):
(WKPageConfigurationSetAllowTestOnlyIPC):
(WKPageConfigurationSetShouldSendConsoleLogsToUIProcessForTesting):
(WKPageConfigurationSetPortsForUpgradingInsecureSchemeForTesting):
* Source/WebKit/UIProcess/API/C/WKUserContentControllerRef.cpp:
(WKUserContentControllerCopyUserScripts):
(WKUserContentControllerAddUserScript):
(WKUserContentControllerRemoveAllUserScripts):
(WKUserContentControllerAddUserContentFilter):
(WKUserContentControllerRemoveAllUserContentFilters):

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

f381a80

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

@zakariaridouh zakariaridouh requested a review from cdumez as a code owner April 14, 2025 06:03
@zakariaridouh zakariaridouh self-assigned this Apr 14, 2025
@zakariaridouh zakariaridouh added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Apr 14, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Apr 14, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 2f13c31)

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

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 14, 2025
@zakariaridouh zakariaridouh removed the merging-blocked Applied to prevent a change from being merged label Apr 14, 2025
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 14, 2025
@zakariaridouh zakariaridouh force-pushed the eng/SaferCpp-Adopt-more-smart-pointers-in-UIProcess-API-C-part-2 branch from 2f13c31 to 72e6ae8 Compare April 14, 2025 06:50
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Apr 14, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 72e6ae8)

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

@webkit-ews-buildbot
Copy link
Collaborator

Safer C++ Build #31798 (2f13c31)

❌ Found 2 failing files with 6 issues. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

@webkit-ews-buildbot
Copy link
Collaborator

Safer C++ Build #31806 (72e6ae8)

❌ Found 2 failing files with 6 issues. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

cdumez
cdumez reviewed Apr 14, 2025
View reviewed changes
cdumez
cdumez reviewed Apr 14, 2025
View reviewed changes
cdumez
cdumez reviewed Apr 14, 2025
View reviewed changes
cdumez
cdumez reviewed Apr 14, 2025
View reviewed changes
cdumez
cdumez reviewed Apr 14, 2025
View reviewed changes
cdumez
cdumez reviewed Apr 14, 2025
View reviewed changes
cdumez
cdumez requested changes Apr 14, 2025
View reviewed changes
Copy link
Contributor

@cdumez cdumez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't follow our usual patterns.

@zakariaridouh zakariaridouh marked this pull request as draft April 14, 2025 18:56
@zakariaridouh zakariaridouh removed the merging-blocked Applied to prevent a change from being merged label Jun 23, 2025
@zakariaridouh zakariaridouh force-pushed the eng/SaferCpp-Adopt-more-smart-pointers-in-UIProcess-API-C-part-2 branch from 72e6ae8 to 476d6d2 Compare June 23, 2025 21:28
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 23, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 476d6d2)

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

@zakariaridouh zakariaridouh marked this pull request as ready for review June 23, 2025 22:12
@zakariaridouh zakariaridouh requested a review from cdumez June 23, 2025 22:12
@webkit-ews-buildbot
Copy link
Collaborator

Safer C++ Build #41319 (476d6d2)

❌ Found 1 failing file with 5 issues. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

@zakariaridouh zakariaridouh force-pushed the eng/SaferCpp-Adopt-more-smart-pointers-in-UIProcess-API-C-part-2 branch from 476d6d2 to f381a80 Compare June 24, 2025 05:45
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 24, 2025 •
edited
Loading

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

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

cdumez
cdumez approved these changes Jun 24, 2025
View reviewed changes
Source/WebKit/UIProcess/API/C/WKPageConfigurationRef.cpp
}

WKPageRef WKPageConfigurationGetRelatedPage(WKPageConfigurationRef configuration)
{
return toAPI(toImpl(configuration)->relatedPage());
RefPtr relatedPage = toProtectedImpl(configuration)->relatedPage();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a protectedRelatedPage() getter instead of introducing an unnecessary local variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed this comment before sending to merge queue. Small followup fix here:

https://bugs.webkit.org/show_bug.cgi?id=294929
#47135

@zakariaridouh zakariaridouh added the merge-queue Applied to send a pull request to merge-queue label Jun 24, 2025
@zakariaridouh
[SaferCpp] Adopt more smart pointers in UIProcess/API/C/ part 2
ba0fd77 
https://bugs.webkit.org/show_bug.cgi?id=291511
rdar://149191086

Reviewed by Chris Dumez.

Adopt more smart pointers in:

-UIProcess/API/C/WKOpenPanelResultListener.cpp
-UIProcess/API/C/WKPageConfigurationRef.cpp
-UIProcess/API/C/WKProtectionSpace.cpp
-UIProcess/API/C/WKUserContentControllerRef.cpp

* Source/WebKit/SaferCPPExpectations/UncountedCallArgsCheckerExpectations:
* Source/WebKit/UIProcess/API/C/WKOpenPanelResultListener.cpp:
(WKOpenPanelResultListenerChooseMediaFiles):
(WKOpenPanelResultListenerChooseFiles):
(WKOpenPanelResultListenerCancel):
* Source/WebKit/UIProcess/API/C/WKPageConfigurationRef.cpp:
(WKPageConfigurationGetContext):
(WKPageConfigurationSetContext):
(WKPageConfigurationGetUserContentController):
(WKPageConfigurationSetUserContentController):
(WKPageConfigurationGetPreferences):
(WKPageConfigurationSetPreferences):
(WKPageConfigurationGetRelatedPage):
(WKPageConfigurationSetRelatedPage):
(WKPageConfigurationGetWebsiteDataStore):
(WKPageConfigurationSetWebsiteDataStore):
(WKPageConfigurationSetInitialCapitalizationEnabled):
(WKPageConfigurationSetBackgroundCPULimit):
(WKPageConfigurationSetAllowTestOnlyIPC):
(WKPageConfigurationSetShouldSendConsoleLogsToUIProcessForTesting):
(WKPageConfigurationSetPortsForUpgradingInsecureSchemeForTesting):
* Source/WebKit/UIProcess/API/C/WKUserContentControllerRef.cpp:
(WKUserContentControllerCopyUserScripts):
(WKUserContentControllerAddUserScript):
(WKUserContentControllerRemoveAllUserScripts):
(WKUserContentControllerAddUserContentFilter):
(WKUserContentControllerRemoveAllUserContentFilters):

Canonical link: https://commits.webkit.org/296568@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/SaferCpp-Adopt-more-smart-pointers-in-UIProcess-API-C-part-2 branch from f381a80 to ba0fd77 Compare June 24, 2025 16:10
@webkit-commit-queue
Copy link
Collaborator

Committed 296568@main (ba0fd77): https://commits.webkit.org/296568@main

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

@webkit-commit-queue webkit-commit-queue merged commit ba0fd77 into WebKit:main Jun 24, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 24, 2025
@zakariaridouh zakariaridouh deleted the eng/SaferCpp-Adopt-more-smart-pointers-in-UIProcess-API-C-part-2 branch June 24, 2025 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdumez cdumez cdumez approved these changes

Assignees

@zakariaridouh zakariaridouh

Labels
New Bugs Unclassified bugs are placed in this component until the correct component can be determined.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zakariaridouh @webkit-early-warning-system @webkit-ews-buildbot @webkit-commit-queue @cdumez