Skip to content

Reduce unsafeness of pay-related code #47047

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

Reduce unsafeness of pay-related code #47047

merged 1 commit into from
Jun 24, 2025
+186 −146

Conversation

annevk
Copy link
Contributor

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

02a8fbd

Reduce unsafeness of pay-related code
https://bugs.webkit.org/show_bug.cgi?id=294827

Reviewed by Chris Dumez.

Apply https://github.com/WebKit/WebKit/wiki/Safer-CPP-Guidelines

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

0fe8acd

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
  🧪 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
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv   🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@annevk annevk self-assigned this Jun 23, 2025
@annevk annevk added the WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). label Jun 23, 2025
@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 91a7ef7)

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
✅ 🧪 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

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 23, 2025
@webkit-ews-buildbot
Copy link
Collaborator

Safer C++ Build #41229 (91a7ef7)

⚠️ Found 11 fixed files! Please update expectations in Source/[Project]/SaferCPPExpectations by running the following command and update your pull request:

  • Tools/Scripts/update-safer-cpp-expectations -p WebCore --UncheckedCallArgsChecker Modules/applepay-ams-ui/ApplePayAMSUIPaymentHandler.cpp --UncheckedLocalVarsChecker Modules/applepay/ApplePaySession.cpp --UncountedCallArgsChecker Modules/paymentrequest/MerchantValidationEvent.cpp Modules/paymentrequest/PaymentRequest.cpp Modules/paymentrequest/PaymentResponse.cpp Modules/applepay/PaymentCoordinator.cpp Modules/paymentrequest/PaymentRequestUpdateEvent.cpp Modules/applepay/ApplePaySetup.cpp testing/MockPaymentCoordinator.cpp Modules/applepay-ams-ui/ApplePayAMSUIPaymentHandler.cpp --UncountedLocalVarsChecker Modules/paymentrequest/PaymentRequest.cpp Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp Modules/applepay/ApplePaySession.cpp Modules/applepay/ApplePaySetup.cpp platform/graphics/cocoa/controls/ApplePayButtonCocoa.mm

@annevk annevk removed the merging-blocked Applied to prevent a change from being merged label Jun 23, 2025
@annevk annevk force-pushed the eng/Reduce-unsafeness-of-pay-related-code branch from 91a7ef7 to 51c6686 Compare June 23, 2025 13:15
@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 51c6686)

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
✅ 🧪 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

@annevk annevk marked this pull request as ready for review June 23, 2025 16:56
cdumez
cdumez approved these changes Jun 24, 2025
View reviewed changes
Source/WebCore/platform/graphics/cocoa/controls/ApplePayButtonCocoa.mm Outdated
@@ -108,16 +108,16 @@ static PKPaymentButtonStyle toPKPaymentButtonStyle(ApplePayButtonStyle style)
context.scale(FloatSize(1, -1));

auto logicalRect = borderRect.rect();
const auto& applePayButtonPart = owningApplePayButtonPart();
const Ref applePayButtonPart = owningApplePayButtonPart();
Copy link
Contributor

Choose a reason for hiding this comment

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

We should drop the const

@annevk annevk force-pushed the eng/Reduce-unsafeness-of-pay-related-code branch from 51c6686 to 0fe8acd Compare June 24, 2025 05:26
@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 0fe8acd)

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
  🧪 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
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv   🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@annevk annevk added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 24, 2025
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Reduce-unsafeness-of-pay-related-code branch from 0fe8acd to 02a8fbd Compare June 24, 2025 06:07
@webkit-commit-queue
Copy link
Collaborator

Committed 296553@main (02a8fbd): https://commits.webkit.org/296553@main

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

@webkit-commit-queue webkit-commit-queue merged commit 02a8fbd into WebKit:main Jun 24, 2025
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 24, 2025
@annevk annevk deleted the eng/Reduce-unsafeness-of-pay-related-code branch June 24, 2025 06:08
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

@aprotyas aprotyas Awaiting requested review from aprotyas aprotyas is a code owner

@marcoscaceres marcoscaceres Awaiting requested review from marcoscaceres marcoscaceres is a code owner

Assignees

@annevk annevk

Labels
WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit).
Projects
None yet
Milestone
No milestone
Development

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