[webkit-changes] [WebKit/WebKit] c288f1: REGRESSION(285181 at main): UIP crash from CheckedPtr...

[Abrar Rahman Protyasha]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: c288f170a2f5469bc9fed5ebe83dd406c9a29382
      https://github.com/WebKit/WebKit/commit/c288f170a2f5469bc9fed5ebe83dd406c9a29382
  Author: Abrar Rahman Protyasha <a_protyasha at apple.com>
  Date:   2025-02-04 (Tue, 04 Feb 2025)

  Changed paths:
    M Source/WebKit/Platform/cocoa/PaymentAuthorizationPresenter.h
    M Source/WebKit/Platform/cocoa/WKPaymentAuthorizationDelegate.mm
    M Source/WebKit/Platform/ios/PaymentAuthorizationController.mm
    M Source/WebKit/Shared/ApplePay/WebPaymentCoordinatorProxy.h

  Log Message:
  -----------
  REGRESSION(285181 at main): UIP crash from CheckedPtr assertion under com.apple.WebKit: -[WKPaymentAuthorizationDelegate(Protected) _didFinish] when WP dies
https://bugs.webkit.org/show_bug.cgi?id=287010
rdar://140611989

Reviewed by Wenson Hsieh.

In 285181 at main, PaymentAuthorizationPresenter::Client callers adopted
its CheckedPtr instances, notably in WKPaymentAuthorizationDelegate.
This seemed appropriate for the Client usage pattern prior to said
patch. However, upon further analysis of the ownership model, clearing
the RefPtr<PaymentAuthorizationPresenter::Client> held by WebPageProxy
whenever we are executing a Client method causes a CheckedPtr assertion
to fire, crashing the UI process. This exact flow occurs whenever the
web process crashes while a user is finalizing a payment in the Apple
Pay sheet, since WebPageProxy::resetState() clears the client pointer it
holds.

Based on the analysis above, it makes sense for us to start protecting
this Client object before calling into it. As such, we introduce a
RefPtr accessor (and delete the CheckedPtr variant). To facilitate an
accessor like this, we make PaymentAuthorizationPresenter::Client
inherit from AbstractRefCountedAndCanMakeWeakPtr. We still want to make
a WeakPtr from the Client since we don't change the ownership model
where PaymentAuthorizationPresenter holds onto a weak reference to the
Client.

Tested manually. Unfortunately we do not have test infrastructure in
place to exercise calling into PassKit flows.

* Source/WebKit/Platform/cocoa/PaymentAuthorizationPresenter.h:
(WebKit::PaymentAuthorizationPresenter::protectedClient):
(WebKit::PaymentAuthorizationPresenter::client): Deleted.
(WebKit::PaymentAuthorizationPresenter::checkedClient): Deleted.
* Source/WebKit/Platform/cocoa/WKPaymentAuthorizationDelegate.mm:
(-[WKPaymentAuthorizationDelegate _didAuthorizePayment:completion:]):
(-[WKPaymentAuthorizationDelegate _didFinish]):
(-[WKPaymentAuthorizationDelegate _didRequestMerchantSession:]):
(-[WKPaymentAuthorizationDelegate _didSelectPaymentMethod:completion:]):
(-[WKPaymentAuthorizationDelegate _didSelectShippingContact:completion:]):
(-[WKPaymentAuthorizationDelegate _didSelectShippingMethod:completion:]):
(-[WKPaymentAuthorizationDelegate _didChangeCouponCode:completion:]):
* Source/WebKit/Platform/ios/PaymentAuthorizationController.mm:
(-[WKPaymentAuthorizationControllerDelegate initWithRequest:presenter:]):

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



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications