Changeset 241611 in webkit


Ignore:
Timestamp:
Feb 15, 2019 1:25:28 PM (5 years ago)
Author:
achristensen@apple.com
Message:

Make WebPaymentCoordinatorProxy more robust and modern
​https://bugs.webkit.org/show_bug.cgi?id=194678

Reviewed by Andy Estes.

Use WeakPtr instead of storing raw pointers in lambdas or the global activePaymentCoordinatorProxy to avoid UAF problems.
Call CompletionHandlers in all code paths to avoid hangs.
Use Delayed instead of LegacySync for synchronous messaging to progress towards removing LegacySync messages.

  • Scripts/webkit/messages.py:
  • UIProcess/ApplePay/WebPaymentCoordinatorProxy.cpp:

(WebKit::activePaymentCoordinatorProxy):
(WebKit::WebPaymentCoordinatorProxy::~WebPaymentCoordinatorProxy):
(WebKit::WebPaymentCoordinatorProxy::availablePaymentNetworks):
(WebKit::WebPaymentCoordinatorProxy::canMakePayments):
(WebKit::WebPaymentCoordinatorProxy::showPaymentUI):
(WebKit::WebPaymentCoordinatorProxy::didReachFinalState):

  • UIProcess/ApplePay/WebPaymentCoordinatorProxy.h:
  • UIProcess/ApplePay/WebPaymentCoordinatorProxy.messages.in:
  • UIProcess/ApplePay/ios/WebPaymentCoordinatorProxyIOS.mm:

(WebKit::WebPaymentCoordinatorProxy::platformShowPaymentUI):

  • UIProcess/ApplePay/mac/WebPaymentCoordinatorProxyMac.mm:

(WebKit::WebPaymentCoordinatorProxy::platformShowPaymentUI):

Location:
trunk/Source/WebKit
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r241609 r241611  
     12019-02-15  Alex Christensen  <achristensen@webkit.org>
     2
     3        Make WebPaymentCoordinatorProxy more robust and modern
     4        https://bugs.webkit.org/show_bug.cgi?id=194678
     5
     6        Reviewed by Andy Estes.
     7
     8        Use WeakPtr instead of storing raw pointers in lambdas or the global activePaymentCoordinatorProxy to avoid UAF problems.
     9        Call CompletionHandlers in all code paths to avoid hangs.
     10        Use Delayed instead of LegacySync for synchronous messaging to progress towards removing LegacySync messages.
     11
     12        * Scripts/webkit/messages.py:
     13        * UIProcess/ApplePay/WebPaymentCoordinatorProxy.cpp:
     14        (WebKit::activePaymentCoordinatorProxy):
     15        (WebKit::WebPaymentCoordinatorProxy::~WebPaymentCoordinatorProxy):
     16        (WebKit::WebPaymentCoordinatorProxy::availablePaymentNetworks):
     17        (WebKit::WebPaymentCoordinatorProxy::canMakePayments):
     18        (WebKit::WebPaymentCoordinatorProxy::showPaymentUI):
     19        (WebKit::WebPaymentCoordinatorProxy::didReachFinalState):
     20        * UIProcess/ApplePay/WebPaymentCoordinatorProxy.h:
     21        * UIProcess/ApplePay/WebPaymentCoordinatorProxy.messages.in:
     22        * UIProcess/ApplePay/ios/WebPaymentCoordinatorProxyIOS.mm:
     23        (WebKit::WebPaymentCoordinatorProxy::platformShowPaymentUI):
     24        * UIProcess/ApplePay/mac/WebPaymentCoordinatorProxyMac.mm:
     25        (WebKit::WebPaymentCoordinatorProxy::platformShowPaymentUI):
     26
    1272019-02-15  Youenn Fablet  <youenn@apple.com>
    228

  • trunk/Source/WebKit/Scripts/webkit/messages.py

    r241595 r241611  
    190190    ])
    191191
    192     for message in receiver.messages:
    193         if message.reply_parameters != None:
    194             headers.add('<wtf/ThreadSafeRefCounted.h>')
    195             types_by_namespace['IPC'].update([('class', 'Connection')])
     192    headers.add('"Connection.h"')
     193    headers.add('<wtf/ThreadSafeRefCounted.h>')
    196194
    197195    no_forward_declaration_types = frozenset([

  • trunk/Source/WebKit/UIProcess/ApplePay/WebPaymentCoordinatorProxy.cpp

    r239427 r241611  
    3737namespace WebKit {
    3838
    39 static WebPaymentCoordinatorProxy* activePaymentCoordinatorProxy;
     39static WeakPtr<WebPaymentCoordinatorProxy>& activePaymentCoordinatorProxy()
     40{
     41    static NeverDestroyed<WeakPtr<WebPaymentCoordinatorProxy>> activePaymentCoordinatorProxy;
     42    return activePaymentCoordinatorProxy.get();
     43}
    4044
    4145WebPaymentCoordinatorProxy::WebPaymentCoordinatorProxy(WebPageProxy& webPageProxy)
    … …  
    5054WebPaymentCoordinatorProxy::~WebPaymentCoordinatorProxy()
    5155{
    52     if (activePaymentCoordinatorProxy == this)
    53         activePaymentCoordinatorProxy = nullptr;
    54 
    5556    if (m_state != State::Idle)
    5657        hidePaymentUI();
    … …  
    5960}
    6061
    61 void WebPaymentCoordinatorProxy::availablePaymentNetworks(Vector<String>& networks)
    62 {
    63     networks = platformAvailablePaymentNetworks();
    64 }
    65 
    66 void WebPaymentCoordinatorProxy::canMakePayments(bool& reply)
    67 {
    68     reply = platformCanMakePayments();
     62void WebPaymentCoordinatorProxy::availablePaymentNetworks(CompletionHandler<void(Vector<String>&&)>&& completionHandler)
     63{
     64    completionHandler(platformAvailablePaymentNetworks());
     65}
     66
     67void WebPaymentCoordinatorProxy::canMakePayments(CompletionHandler<void(bool)>&& reply)
     68{
     69    reply(platformCanMakePayments());
    6970}
    7071
    … …  
    9394}
    9495
    95 void WebPaymentCoordinatorProxy::showPaymentUI(const String& originatingURLString, const Vector<String>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest& paymentRequest, bool& result)
     96void WebPaymentCoordinatorProxy::showPaymentUI(const String& originatingURLString, const Vector<String>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest& paymentRequest, CompletionHandler<void(bool)>&& completionHandler)
    9697{
    9798    // FIXME: Make this a message check.
    9899    ASSERT(canBegin());
    99100
    100     if (activePaymentCoordinatorProxy) {
    101         activePaymentCoordinatorProxy->hidePaymentUI();
    102         activePaymentCoordinatorProxy->didCancelPaymentSession();
    103     }
    104 
    105     activePaymentCoordinatorProxy = this;
     101    if (auto& coordinator = activePaymentCoordinatorProxy()) {
     102        coordinator->hidePaymentUI();
     103        coordinator->didCancelPaymentSession();
     104    }
     105
     106    activePaymentCoordinatorProxy() = makeWeakPtr(this);
    106107
    107108    m_state = State::Activating;
    … …  
    113114        linkIconURLs.append(URL(URL(), linkIconURLString));
    114115
    115     platformShowPaymentUI(originatingURL, linkIconURLs, paymentRequest, [this](bool result) {
    116         ASSERT(m_state == State::Activating);
     116    platformShowPaymentUI(originatingURL, linkIconURLs, paymentRequest, [weakThis = makeWeakPtr(*this)](bool result) {
     117        if (!weakThis)
     118            return;
     119
     120        ASSERT(weakThis->m_state == State::Activating);
    117121        if (!result) {
    118             didCancelPaymentSession();
     122            weakThis->didCancelPaymentSession();
    119123            return;
    120124        }
    121125
    122         m_state = State::Active;
     126        weakThis->m_state = State::Active;
    123127    });
    124128
    125     result = true;
     129    completionHandler(true);
    126130}
    127131
    … …  
    333337    m_merchantValidationState = MerchantValidationState::Idle;
    334338
    335     ASSERT(activePaymentCoordinatorProxy == this);
    336     activePaymentCoordinatorProxy = nullptr;
     339    ASSERT(activePaymentCoordinatorProxy() == this);
     340    activePaymentCoordinatorProxy() = nullptr;
    337341}
    338342

  • trunk/Source/WebKit/UIProcess/ApplePay/WebPaymentCoordinatorProxy.h

    r239709 r241611  
    7979
    8080    // Message handlers.
    81     void availablePaymentNetworks(Vector<String>&);
    82     void canMakePayments(bool& reply);
     81    void availablePaymentNetworks(CompletionHandler<void(Vector<String>&&)>&&);
     82    void canMakePayments(CompletionHandler<void(bool)>&&);
    8383    void canMakePaymentsWithActiveCard(const String& merchantIdentifier, const String& domainName, uint64_t requestID);
    8484    void openPaymentSetup(const String& merchantIdentifier, const String& domainName, uint64_t requestID);
    85     void showPaymentUI(const String& originatingURLString, const Vector<String>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest&, bool& result);
     85    void showPaymentUI(const String& originatingURLString, const Vector<String>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest&, CompletionHandler<void(bool)>&&);
    8686    void completeMerchantValidation(const WebCore::PaymentMerchantSession&);
    8787    void completeShippingMethodSelection(const Optional<WebCore::ShippingMethodUpdate>&);
    … …  
    103103    void platformCanMakePaymentsWithActiveCard(const String& merchantIdentifier, const String& domainName, WTF::Function<void (bool)>&& completionHandler);
    104104    void platformOpenPaymentSetup(const String& merchantIdentifier, const String& domainName, WTF::Function<void (bool)>&& completionHandler);
    105     void platformShowPaymentUI(const URL& originatingURL, const Vector<URL>& linkIconURLs, const WebCore::ApplePaySessionPaymentRequest&, WTF::Function<void(bool)>&& completionHandler);
     105    void platformShowPaymentUI(const URL& originatingURL, const Vector<URL>& linkIconURLs, const WebCore::ApplePaySessionPaymentRequest&, CompletionHandler<void(bool)>&&);
    106106    void platformCompleteMerchantValidation(const WebCore::PaymentMerchantSession&);
    107107    void platformCompleteShippingMethodSelection(const Optional<WebCore::ShippingMethodUpdate>&);

  • trunk/Source/WebKit/UIProcess/ApplePay/WebPaymentCoordinatorProxy.messages.in

    r239427 r241611  
    2727messages -> WebPaymentCoordinatorProxy {
    2828
    29     AvailablePaymentNetworks() -> (Vector<String> availablePaymentNetworks) LegacySync
    30     CanMakePayments() -> (bool result) LegacySync
     29    AvailablePaymentNetworks() -> (Vector<String> availablePaymentNetworks) Delayed
     30    CanMakePayments() -> (bool result) Delayed
    3131    CanMakePaymentsWithActiveCard(String merchantIdentifier, String domainName, uint64_t requestID)
    3232    OpenPaymentSetup(String merchantIdentifier, String domainName, uint64_t requestID)
    3333
    34     ShowPaymentUI(String originatingURLString, Vector<String> linkIconURLStrings, WebCore::ApplePaySessionPaymentRequest paymentRequest) -> (bool result) LegacySync
     34    ShowPaymentUI(String originatingURLString, Vector<String> linkIconURLStrings, WebCore::ApplePaySessionPaymentRequest paymentRequest) -> (bool result) Delayed
    3535    CompleteMerchantValidation(WebCore::PaymentMerchantSession paymentMerchantSession)
    3636    CompleteShippingMethodSelection(Optional<WebCore::ShippingMethodUpdate> update)

  • trunk/Source/WebKit/UIProcess/ApplePay/ios/WebPaymentCoordinatorProxyIOS.mm

    r238771 r241611  
    3939namespace WebKit {
    4040
    41 void WebPaymentCoordinatorProxy::platformShowPaymentUI(const URL& originatingURL, const Vector<URL>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest& request, WTF::Function<void(bool)>&& completionHandler)
     41void WebPaymentCoordinatorProxy::platformShowPaymentUI(const URL& originatingURL, const Vector<URL>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest& request, CompletionHandler<void(bool)>&& completionHandler)
    4242{
    4343    UIViewController *presentingViewController = m_webPageProxy.uiClient().presentingViewController();

  • trunk/Source/WebKit/UIProcess/ApplePay/mac/WebPaymentCoordinatorProxyMac.mm

    r239078 r241611  
    3636namespace WebKit {
    3737
    38 void WebPaymentCoordinatorProxy::platformShowPaymentUI(const URL& originatingURL, const Vector<URL>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest& request, WTF::Function<void(bool)>&& completionHandler)
     38void WebPaymentCoordinatorProxy::platformShowPaymentUI(const URL& originatingURL, const Vector<URL>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest& request, CompletionHandler<void(bool)>&& completionHandler)
    3939{
    4040    auto paymentRequest = toPKPaymentRequest(m_webPageProxy, originatingURL, linkIconURLStrings, request);
    … …  
    4242    auto showPaymentUIRequestSeed = m_showPaymentUIRequestSeed;
    4343    auto weakThis = makeWeakPtr(*this);
    44     [PAL::getPKPaymentAuthorizationViewControllerClass() requestViewControllerWithPaymentRequest:paymentRequest.get() completion:makeBlockPtr([paymentRequest, showPaymentUIRequestSeed, weakThis, completionHandler = WTFMove(completionHandler)](PKPaymentAuthorizationViewController *viewController, NSError *error) {
     44    [PAL::getPKPaymentAuthorizationViewControllerClass() requestViewControllerWithPaymentRequest:paymentRequest.get() completion:makeBlockPtr([paymentRequest, showPaymentUIRequestSeed, weakThis, completionHandler = WTFMove(completionHandler)](PKPaymentAuthorizationViewController *viewController, NSError *error) mutable {
    4545        auto paymentCoordinatorProxy = weakThis.get();
    4646        if (!paymentCoordinatorProxy)
    47             return;
     47            return completionHandler(false);
    4848
    4949        if (error) {
    … …  
    5656        if (showPaymentUIRequestSeed != paymentCoordinatorProxy->m_showPaymentUIRequestSeed) {
    5757            // We've already been asked to hide the payment UI. Don't attempt to show it.
    58             return;
     58            return completionHandler(false);
    5959        }
    6060
Note: See TracChangeset for help on using the changeset viewer.