Diff
Modified: trunk/Source/WebKit/ChangeLog (241610 => 241611)
--- trunk/Source/WebKit/ChangeLog 2019-02-15 21:21:44 UTC (rev 241610)
+++ trunk/Source/WebKit/ChangeLog 2019-02-15 21:25:28 UTC (rev 241611)
@@ -1,3 +1,29 @@
+2019-02-15 Alex Christensen <[email protected]>
+
+ 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):
+
2019-02-15 Youenn Fablet <[email protected]>
Make ServiceWorkerClientFetch closer to WebResourceLoader
Modified: trunk/Source/WebKit/Scripts/webkit/messages.py (241610 => 241611)
--- trunk/Source/WebKit/Scripts/webkit/messages.py 2019-02-15 21:21:44 UTC (rev 241610)
+++ trunk/Source/WebKit/Scripts/webkit/messages.py 2019-02-15 21:25:28 UTC (rev 241611)
@@ -189,10 +189,8 @@
'String',
])
- for message in receiver.messages:
- if message.reply_parameters != None:
- headers.add('<wtf/ThreadSafeRefCounted.h>')
- types_by_namespace['IPC'].update([('class', 'Connection')])
+ headers.add('"Connection.h"')
+ headers.add('<wtf/ThreadSafeRefCounted.h>')
no_forward_declaration_types = frozenset([
'MachSendRight',
Modified: trunk/Source/WebKit/UIProcess/ApplePay/WebPaymentCoordinatorProxy.cpp (241610 => 241611)
--- trunk/Source/WebKit/UIProcess/ApplePay/WebPaymentCoordinatorProxy.cpp 2019-02-15 21:21:44 UTC (rev 241610)
+++ trunk/Source/WebKit/UIProcess/ApplePay/WebPaymentCoordinatorProxy.cpp 2019-02-15 21:25:28 UTC (rev 241611)
@@ -36,7 +36,11 @@
namespace WebKit {
-static WebPaymentCoordinatorProxy* activePaymentCoordinatorProxy;
+static WeakPtr<WebPaymentCoordinatorProxy>& activePaymentCoordinatorProxy()
+{
+ static NeverDestroyed<WeakPtr<WebPaymentCoordinatorProxy>> activePaymentCoordinatorProxy;
+ return activePaymentCoordinatorProxy.get();
+}
WebPaymentCoordinatorProxy::WebPaymentCoordinatorProxy(WebPageProxy& webPageProxy)
: m_webPageProxy(webPageProxy)
@@ -49,9 +53,6 @@
WebPaymentCoordinatorProxy::~WebPaymentCoordinatorProxy()
{
- if (activePaymentCoordinatorProxy == this)
- activePaymentCoordinatorProxy = nullptr;
-
if (m_state != State::Idle)
hidePaymentUI();
@@ -58,14 +59,14 @@
m_webPageProxy.process().removeMessageReceiver(Messages::WebPaymentCoordinatorProxy::messageReceiverName(), m_webPageProxy.pageID());
}
-void WebPaymentCoordinatorProxy::availablePaymentNetworks(Vector<String>& networks)
+void WebPaymentCoordinatorProxy::availablePaymentNetworks(CompletionHandler<void(Vector<String>&&)>&& completionHandler)
{
- networks = platformAvailablePaymentNetworks();
+ completionHandler(platformAvailablePaymentNetworks());
}
-void WebPaymentCoordinatorProxy::canMakePayments(bool& reply)
+void WebPaymentCoordinatorProxy::canMakePayments(CompletionHandler<void(bool)>&& reply)
{
- reply = platformCanMakePayments();
+ reply(platformCanMakePayments());
}
void WebPaymentCoordinatorProxy::canMakePaymentsWithActiveCard(const String& merchantIdentifier, const String& domainName, uint64_t requestID)
@@ -92,17 +93,17 @@
});
}
-void WebPaymentCoordinatorProxy::showPaymentUI(const String& originatingURLString, const Vector<String>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest& paymentRequest, bool& result)
+void WebPaymentCoordinatorProxy::showPaymentUI(const String& originatingURLString, const Vector<String>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest& paymentRequest, CompletionHandler<void(bool)>&& completionHandler)
{
// FIXME: Make this a message check.
ASSERT(canBegin());
- if (activePaymentCoordinatorProxy) {
- activePaymentCoordinatorProxy->hidePaymentUI();
- activePaymentCoordinatorProxy->didCancelPaymentSession();
+ if (auto& coordinator = activePaymentCoordinatorProxy()) {
+ coordinator->hidePaymentUI();
+ coordinator->didCancelPaymentSession();
}
- activePaymentCoordinatorProxy = this;
+ activePaymentCoordinatorProxy() = makeWeakPtr(this);
m_state = State::Activating;
@@ -112,17 +113,20 @@
for (const auto& linkIconURLString : linkIconURLStrings)
linkIconURLs.append(URL(URL(), linkIconURLString));
- platformShowPaymentUI(originatingURL, linkIconURLs, paymentRequest, [this](bool result) {
- ASSERT(m_state == State::Activating);
+ platformShowPaymentUI(originatingURL, linkIconURLs, paymentRequest, [weakThis = makeWeakPtr(*this)](bool result) {
+ if (!weakThis)
+ return;
+
+ ASSERT(weakThis->m_state == State::Activating);
if (!result) {
- didCancelPaymentSession();
+ weakThis->didCancelPaymentSession();
return;
}
- m_state = State::Active;
+ weakThis->m_state = State::Active;
});
- result = true;
+ completionHandler(true);
}
@@ -332,8 +336,8 @@
m_state = State::Idle;
m_merchantValidationState = MerchantValidationState::Idle;
- ASSERT(activePaymentCoordinatorProxy == this);
- activePaymentCoordinatorProxy = nullptr;
+ ASSERT(activePaymentCoordinatorProxy() == this);
+ activePaymentCoordinatorProxy() = nullptr;
}
}
Modified: trunk/Source/WebKit/UIProcess/ApplePay/WebPaymentCoordinatorProxy.h (241610 => 241611)
--- trunk/Source/WebKit/UIProcess/ApplePay/WebPaymentCoordinatorProxy.h 2019-02-15 21:21:44 UTC (rev 241610)
+++ trunk/Source/WebKit/UIProcess/ApplePay/WebPaymentCoordinatorProxy.h 2019-02-15 21:25:28 UTC (rev 241611)
@@ -78,11 +78,11 @@
void didReceiveSyncMessage(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder>&) override;
// Message handlers.
- void availablePaymentNetworks(Vector<String>&);
- void canMakePayments(bool& reply);
+ void availablePaymentNetworks(CompletionHandler<void(Vector<String>&&)>&&);
+ void canMakePayments(CompletionHandler<void(bool)>&&);
void canMakePaymentsWithActiveCard(const String& merchantIdentifier, const String& domainName, uint64_t requestID);
void openPaymentSetup(const String& merchantIdentifier, const String& domainName, uint64_t requestID);
- void showPaymentUI(const String& originatingURLString, const Vector<String>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest&, bool& result);
+ void showPaymentUI(const String& originatingURLString, const Vector<String>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest&, CompletionHandler<void(bool)>&&);
void completeMerchantValidation(const WebCore::PaymentMerchantSession&);
void completeShippingMethodSelection(const Optional<WebCore::ShippingMethodUpdate>&);
void completeShippingContactSelection(const Optional<WebCore::ShippingContactUpdate>&);
@@ -102,7 +102,7 @@
bool platformCanMakePayments();
void platformCanMakePaymentsWithActiveCard(const String& merchantIdentifier, const String& domainName, WTF::Function<void (bool)>&& completionHandler);
void platformOpenPaymentSetup(const String& merchantIdentifier, const String& domainName, WTF::Function<void (bool)>&& completionHandler);
- void platformShowPaymentUI(const URL& originatingURL, const Vector<URL>& linkIconURLs, const WebCore::ApplePaySessionPaymentRequest&, WTF::Function<void(bool)>&& completionHandler);
+ void platformShowPaymentUI(const URL& originatingURL, const Vector<URL>& linkIconURLs, const WebCore::ApplePaySessionPaymentRequest&, CompletionHandler<void(bool)>&&);
void platformCompleteMerchantValidation(const WebCore::PaymentMerchantSession&);
void platformCompleteShippingMethodSelection(const Optional<WebCore::ShippingMethodUpdate>&);
void platformCompleteShippingContactSelection(const Optional<WebCore::ShippingContactUpdate>&);
Modified: trunk/Source/WebKit/UIProcess/ApplePay/WebPaymentCoordinatorProxy.messages.in (241610 => 241611)
--- trunk/Source/WebKit/UIProcess/ApplePay/WebPaymentCoordinatorProxy.messages.in 2019-02-15 21:21:44 UTC (rev 241610)
+++ trunk/Source/WebKit/UIProcess/ApplePay/WebPaymentCoordinatorProxy.messages.in 2019-02-15 21:25:28 UTC (rev 241611)
@@ -26,12 +26,12 @@
messages -> WebPaymentCoordinatorProxy {
- AvailablePaymentNetworks() -> (Vector<String> availablePaymentNetworks) LegacySync
- CanMakePayments() -> (bool result) LegacySync
+ AvailablePaymentNetworks() -> (Vector<String> availablePaymentNetworks) Delayed
+ CanMakePayments() -> (bool result) Delayed
CanMakePaymentsWithActiveCard(String merchantIdentifier, String domainName, uint64_t requestID)
OpenPaymentSetup(String merchantIdentifier, String domainName, uint64_t requestID)
- ShowPaymentUI(String originatingURLString, Vector<String> linkIconURLStrings, WebCore::ApplePaySessionPaymentRequest paymentRequest) -> (bool result) LegacySync
+ ShowPaymentUI(String originatingURLString, Vector<String> linkIconURLStrings, WebCore::ApplePaySessionPaymentRequest paymentRequest) -> (bool result) Delayed
CompleteMerchantValidation(WebCore::PaymentMerchantSession paymentMerchantSession)
CompleteShippingMethodSelection(Optional<WebCore::ShippingMethodUpdate> update)
CompleteShippingContactSelection(Optional<WebCore::ShippingContactUpdate> update)
Modified: trunk/Source/WebKit/UIProcess/ApplePay/ios/WebPaymentCoordinatorProxyIOS.mm (241610 => 241611)
--- trunk/Source/WebKit/UIProcess/ApplePay/ios/WebPaymentCoordinatorProxyIOS.mm 2019-02-15 21:21:44 UTC (rev 241610)
+++ trunk/Source/WebKit/UIProcess/ApplePay/ios/WebPaymentCoordinatorProxyIOS.mm 2019-02-15 21:25:28 UTC (rev 241611)
@@ -38,7 +38,7 @@
namespace WebKit {
-void WebPaymentCoordinatorProxy::platformShowPaymentUI(const URL& originatingURL, const Vector<URL>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest& request, WTF::Function<void(bool)>&& completionHandler)
+void WebPaymentCoordinatorProxy::platformShowPaymentUI(const URL& originatingURL, const Vector<URL>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest& request, CompletionHandler<void(bool)>&& completionHandler)
{
UIViewController *presentingViewController = m_webPageProxy.uiClient().presentingViewController();
Modified: trunk/Source/WebKit/UIProcess/ApplePay/mac/WebPaymentCoordinatorProxyMac.mm (241610 => 241611)
--- trunk/Source/WebKit/UIProcess/ApplePay/mac/WebPaymentCoordinatorProxyMac.mm 2019-02-15 21:21:44 UTC (rev 241610)
+++ trunk/Source/WebKit/UIProcess/ApplePay/mac/WebPaymentCoordinatorProxyMac.mm 2019-02-15 21:25:28 UTC (rev 241611)
@@ -35,16 +35,16 @@
namespace WebKit {
-void WebPaymentCoordinatorProxy::platformShowPaymentUI(const URL& originatingURL, const Vector<URL>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest& request, WTF::Function<void(bool)>&& completionHandler)
+void WebPaymentCoordinatorProxy::platformShowPaymentUI(const URL& originatingURL, const Vector<URL>& linkIconURLStrings, const WebCore::ApplePaySessionPaymentRequest& request, CompletionHandler<void(bool)>&& completionHandler)
{
auto paymentRequest = toPKPaymentRequest(m_webPageProxy, originatingURL, linkIconURLStrings, request);
auto showPaymentUIRequestSeed = m_showPaymentUIRequestSeed;
auto weakThis = makeWeakPtr(*this);
- [PAL::getPKPaymentAuthorizationViewControllerClass() requestViewControllerWithPaymentRequest:paymentRequest.get() completion:makeBlockPtr([paymentRequest, showPaymentUIRequestSeed, weakThis, completionHandler = WTFMove(completionHandler)](PKPaymentAuthorizationViewController *viewController, NSError *error) {
+ [PAL::getPKPaymentAuthorizationViewControllerClass() requestViewControllerWithPaymentRequest:paymentRequest.get() completion:makeBlockPtr([paymentRequest, showPaymentUIRequestSeed, weakThis, completionHandler = WTFMove(completionHandler)](PKPaymentAuthorizationViewController *viewController, NSError *error) mutable {
auto paymentCoordinatorProxy = weakThis.get();
if (!paymentCoordinatorProxy)
- return;
+ return completionHandler(false);
if (error) {
LOG_ERROR("+[PKPaymentAuthorizationViewController requestViewControllerWithPaymentRequest:completion:] error %@", error);
@@ -55,7 +55,7 @@
if (showPaymentUIRequestSeed != paymentCoordinatorProxy->m_showPaymentUIRequestSeed) {
// We've already been asked to hide the payment UI. Don't attempt to show it.
- return;
+ return completionHandler(false);
}
ASSERT(viewController);
