Diff
Modified: trunk/Source/WTF/ChangeLog (247217 => 247218)
--- trunk/Source/WTF/ChangeLog 2019-07-08 18:19:13 UTC (rev 247217)
+++ trunk/Source/WTF/ChangeLog 2019-07-08 18:26:47 UTC (rev 247218)
@@ -1,3 +1,17 @@
+2019-07-08 Chris Dumez <[email protected]>
+
+ Add threading assertion to WTF::CompletionHandler
+ https://bugs.webkit.org/show_bug.cgi?id=199516
+
+ Reviewed by Alex Christensen.
+
+ Add threading assertion to WTF::CompletionHandler to try and catch common cases
+ of unsafe usage (completion handler constructed on one thread but called on
+ another).
+
+ * wtf/CompletionHandler.h:
+ (WTF::CompletionHandler<Out):
+
2019-07-08 Antoine Quint <[email protected]>
[Pointer Events] Enable only on the most recent version of the supported iOS family
Modified: trunk/Source/WTF/wtf/CompletionHandler.h (247217 => 247218)
--- trunk/Source/WTF/wtf/CompletionHandler.h 2019-07-08 18:19:13 UTC (rev 247217)
+++ trunk/Source/WTF/wtf/CompletionHandler.h 2019-07-08 18:26:47 UTC (rev 247218)
@@ -26,6 +26,7 @@
#pragma once
#include <wtf/Function.h>
+#include <wtf/MainThread.h>
namespace WTF {
@@ -40,6 +41,9 @@
template<typename CallableType, class = typename std::enable_if<std::is_rvalue_reference<CallableType&&>::value>::type>
CompletionHandler(CallableType&& callable)
: m_function(WTFMove(callable))
+#if !ASSERT_DISABLED
+ , m_wasConstructedOnMainThread(isMainThread())
+#endif
{
}
@@ -55,6 +59,7 @@
Out operator()(In... in)
{
+ ASSERT(m_wasConstructedOnMainThread == isMainThread());
ASSERT_WITH_MESSAGE(m_function, "Completion handler should not be called more than once");
return std::exchange(m_function, nullptr)(std::forward<In>(in)...);
}
@@ -61,6 +66,9 @@
private:
Function<Out(In...)> m_function;
+#if !ASSERT_DISABLED
+ bool m_wasConstructedOnMainThread;
+#endif
};
namespace Detail {
Modified: trunk/Source/WebCore/ChangeLog (247217 => 247218)
--- trunk/Source/WebCore/ChangeLog 2019-07-08 18:19:13 UTC (rev 247217)
+++ trunk/Source/WebCore/ChangeLog 2019-07-08 18:26:47 UTC (rev 247218)
@@ -1,3 +1,26 @@
+2019-07-08 Chris Dumez <[email protected]>
+
+ Add threading assertion to WTF::CompletionHandler
+ https://bugs.webkit.org/show_bug.cgi?id=199516
+
+ Reviewed by Alex Christensen.
+
+ Update some MessagePort-related code to use WTF::Function instead of WTF::CompletionHandler
+ since the callback is always called on the main thread, even when it was created on a
+ worker thread. Ideally, this code would be refactored so that the callback gets called on
+ the worker thread directly.
+
+ * dom/messageports/MessagePortChannel.cpp:
+ (WebCore::MessagePortChannel::checkRemotePortForActivity):
+ * dom/messageports/MessagePortChannel.h:
+ * dom/messageports/MessagePortChannelProvider.h:
+ * dom/messageports/MessagePortChannelProviderImpl.cpp:
+ (WebCore::MessagePortChannelProviderImpl::checkRemotePortForActivity):
+ * dom/messageports/MessagePortChannelProviderImpl.h:
+ * dom/messageports/MessagePortChannelRegistry.cpp:
+ (WebCore::MessagePortChannelRegistry::checkRemotePortForActivity):
+ * dom/messageports/MessagePortChannelRegistry.h:
+
2019-07-08 Charlie Turner <[email protected]>
REGRESSION(r243197): [GStreamer] Web process hangs when scrolling twitter timeline which contains HLS videos
Modified: trunk/Source/WebCore/dom/messageports/MessagePortChannel.cpp (247217 => 247218)
--- trunk/Source/WebCore/dom/messageports/MessagePortChannel.cpp 2019-07-08 18:19:13 UTC (rev 247217)
+++ trunk/Source/WebCore/dom/messageports/MessagePortChannel.cpp 2019-07-08 18:26:47 UTC (rev 247218)
@@ -182,7 +182,7 @@
});
}
-void MessagePortChannel::checkRemotePortForActivity(const MessagePortIdentifier& remotePort, CompletionHandler<void(MessagePortChannelProvider::HasActivity)>&& callback)
+void MessagePortChannel::checkRemotePortForActivity(const MessagePortIdentifier& remotePort, Function<void(MessagePortChannelProvider::HasActivity)>&& callback)
{
ASSERT(isMainThread());
ASSERT(remotePort == m_ports[0] || remotePort == m_ports[1]);
@@ -207,7 +207,7 @@
return;
}
- auto outerCallback = CompletionHandler<void(MessagePortChannelProvider::HasActivity)> { [this, protectedThis = makeRef(*this), callback = WTFMove(callback)] (MessagePortChannelProvider::HasActivity hasActivity) mutable {
+ auto outerCallback = Function<void(MessagePortChannelProvider::HasActivity)> { [this, protectedThis = makeRef(*this), callback = WTFMove(callback)] (MessagePortChannelProvider::HasActivity hasActivity) mutable {
if (hasActivity == MessagePortChannelProvider::HasActivity::Yes) {
callback(hasActivity);
return;
Modified: trunk/Source/WebCore/dom/messageports/MessagePortChannel.h (247217 => 247218)
--- trunk/Source/WebCore/dom/messageports/MessagePortChannel.h 2019-07-08 18:19:13 UTC (rev 247217)
+++ trunk/Source/WebCore/dom/messageports/MessagePortChannel.h 2019-07-08 18:26:47 UTC (rev 247218)
@@ -54,7 +54,7 @@
bool postMessageToRemote(MessageWithMessagePorts&&, const MessagePortIdentifier& remoteTarget);
void takeAllMessagesForPort(const MessagePortIdentifier&, Function<void(Vector<MessageWithMessagePorts>&&, Function<void()>&&)>&&);
- void checkRemotePortForActivity(const MessagePortIdentifier&, CompletionHandler<void(MessagePortChannelProvider::HasActivity)>&& callback);
+ void checkRemotePortForActivity(const MessagePortIdentifier&, Function<void(MessagePortChannelProvider::HasActivity)>&& callback);
WEBCORE_EXPORT bool hasAnyMessagesPendingOrInFlight() const;
Modified: trunk/Source/WebCore/dom/messageports/MessagePortChannelProvider.h (247217 => 247218)
--- trunk/Source/WebCore/dom/messageports/MessagePortChannelProvider.h 2019-07-08 18:19:13 UTC (rev 247217)
+++ trunk/Source/WebCore/dom/messageports/MessagePortChannelProvider.h 2019-07-08 18:26:47 UTC (rev 247218)
@@ -46,7 +46,11 @@
virtual void entangleLocalPortInThisProcessToRemote(const MessagePortIdentifier& local, const MessagePortIdentifier& remote) = 0;
virtual void messagePortDisentangled(const MessagePortIdentifier& local) = 0;
virtual void messagePortClosed(const MessagePortIdentifier& local) = 0;
+
+ // FIXME: Ideally the callback would be a CompletionHandler but it is always called on the caller's
+ // thread at the moment.
virtual void takeAllMessagesForPort(const MessagePortIdentifier&, Function<void(Vector<MessageWithMessagePorts>&&, Function<void()>&&)>&&) = 0;
+
virtual void postMessageToRemote(MessageWithMessagePorts&&, const MessagePortIdentifier& remoteTarget) = 0;
enum class HasActivity {
@@ -53,7 +57,9 @@
Yes,
No,
};
- virtual void checkRemotePortForActivity(const MessagePortIdentifier& remoteTarget, CompletionHandler<void(HasActivity)>&& callback) = 0;
+ // FIXME: Ideally the callback would be a CompletionHandler but it is always called on the caller's
+ // thread at the moment.
+ virtual void checkRemotePortForActivity(const MessagePortIdentifier& remoteTarget, Function<void(HasActivity)>&& callback) = 0;
// Operations that the coordinating process performs (e.g. the UIProcess)
virtual void checkProcessLocalPortForActivity(const MessagePortIdentifier&, ProcessIdentifier, CompletionHandler<void(HasActivity)>&&) = 0;
Modified: trunk/Source/WebCore/dom/messageports/MessagePortChannelProviderImpl.cpp (247217 => 247218)
--- trunk/Source/WebCore/dom/messageports/MessagePortChannelProviderImpl.cpp 2019-07-08 18:19:13 UTC (rev 247217)
+++ trunk/Source/WebCore/dom/messageports/MessagePortChannelProviderImpl.cpp 2019-07-08 18:26:47 UTC (rev 247218)
@@ -100,9 +100,9 @@
});
}
-void MessagePortChannelProviderImpl::checkRemotePortForActivity(const MessagePortIdentifier& remoteTarget, CompletionHandler<void(HasActivity)>&& outerCallback)
+void MessagePortChannelProviderImpl::checkRemotePortForActivity(const MessagePortIdentifier& remoteTarget, Function<void(HasActivity)>&& outerCallback)
{
- auto callback = CompletionHandler<void(HasActivity)> { [outerCallback = WTFMove(outerCallback)](HasActivity hasActivity) mutable {
+ auto callback = Function<void(HasActivity)> { [outerCallback = WTFMove(outerCallback)](HasActivity hasActivity) mutable {
ASSERT(isMainThread());
outerCallback(hasActivity);
} };
Modified: trunk/Source/WebCore/dom/messageports/MessagePortChannelProviderImpl.h (247217 => 247218)
--- trunk/Source/WebCore/dom/messageports/MessagePortChannelProviderImpl.h 2019-07-08 18:19:13 UTC (rev 247217)
+++ trunk/Source/WebCore/dom/messageports/MessagePortChannelProviderImpl.h 2019-07-08 18:26:47 UTC (rev 247218)
@@ -42,7 +42,7 @@
void messagePortClosed(const MessagePortIdentifier& local) final;
void postMessageToRemote(MessageWithMessagePorts&&, const MessagePortIdentifier& remoteTarget) final;
void takeAllMessagesForPort(const MessagePortIdentifier&, Function<void(Vector<MessageWithMessagePorts>&&, Function<void()>&&)>&&) final;
- void checkRemotePortForActivity(const MessagePortIdentifier& remoteTarget, CompletionHandler<void(HasActivity)>&& callback) final;
+ void checkRemotePortForActivity(const MessagePortIdentifier& remoteTarget, Function<void(HasActivity)>&& callback) final;
void checkProcessLocalPortForActivity(const MessagePortIdentifier&, ProcessIdentifier, CompletionHandler<void(HasActivity)>&&) final;
Modified: trunk/Source/WebCore/dom/messageports/MessagePortChannelRegistry.cpp (247217 => 247218)
--- trunk/Source/WebCore/dom/messageports/MessagePortChannelRegistry.cpp 2019-07-08 18:19:13 UTC (rev 247217)
+++ trunk/Source/WebCore/dom/messageports/MessagePortChannelRegistry.cpp 2019-07-08 18:26:47 UTC (rev 247218)
@@ -157,7 +157,7 @@
channel->takeAllMessagesForPort(port, WTFMove(callback));
}
-void MessagePortChannelRegistry::checkRemotePortForActivity(const MessagePortIdentifier& remoteTarget, CompletionHandler<void(MessagePortChannelProvider::HasActivity)>&& callback)
+void MessagePortChannelRegistry::checkRemotePortForActivity(const MessagePortIdentifier& remoteTarget, Function<void(MessagePortChannelProvider::HasActivity)>&& callback)
{
ASSERT(isMainThread());
Modified: trunk/Source/WebCore/dom/messageports/MessagePortChannelRegistry.h (247217 => 247218)
--- trunk/Source/WebCore/dom/messageports/MessagePortChannelRegistry.h 2019-07-08 18:19:13 UTC (rev 247217)
+++ trunk/Source/WebCore/dom/messageports/MessagePortChannelRegistry.h 2019-07-08 18:26:47 UTC (rev 247218)
@@ -44,7 +44,7 @@
WEBCORE_EXPORT void didCloseMessagePort(const MessagePortIdentifier& local);
WEBCORE_EXPORT bool didPostMessageToRemote(MessageWithMessagePorts&&, const MessagePortIdentifier& remoteTarget);
WEBCORE_EXPORT void takeAllMessagesForPort(const MessagePortIdentifier&, Function<void(Vector<MessageWithMessagePorts>&&, Function<void()>&&)>&&);
- WEBCORE_EXPORT void checkRemotePortForActivity(const MessagePortIdentifier& remoteTarget, CompletionHandler<void(MessagePortChannelProvider::HasActivity)>&& callback);
+ WEBCORE_EXPORT void checkRemotePortForActivity(const MessagePortIdentifier& remoteTarget, Function<void(MessagePortChannelProvider::HasActivity)>&& callback);
WEBCORE_EXPORT MessagePortChannel* existingChannelContainingPort(const MessagePortIdentifier&);
Modified: trunk/Source/WebKit/ChangeLog (247217 => 247218)
--- trunk/Source/WebKit/ChangeLog 2019-07-08 18:19:13 UTC (rev 247217)
+++ trunk/Source/WebKit/ChangeLog 2019-07-08 18:26:47 UTC (rev 247218)
@@ -1,3 +1,22 @@
+2019-07-08 Chris Dumez <[email protected]>
+
+ Add threading assertion to WTF::CompletionHandler
+ https://bugs.webkit.org/show_bug.cgi?id=199516
+
+ Reviewed by Alex Christensen.
+
+ Update some MessagePort-related code to use WTF::Function instead of WTF::CompletionHandler
+ since the callback is always called on the main thread, even when it was created on a
+ worker thread. Ideally, this code would be refactored so that the callback gets called on
+ the worker thread directly.
+
+ * UIProcess/UIMessagePortChannelProvider.cpp:
+ (WebKit::UIMessagePortChannelProvider::checkRemotePortForActivity):
+ * UIProcess/UIMessagePortChannelProvider.h:
+ * WebProcess/WebCoreSupport/WebMessagePortChannelProvider.cpp:
+ (WebKit::WebMessagePortChannelProvider::checkRemotePortForActivity):
+ * WebProcess/WebCoreSupport/WebMessagePortChannelProvider.h:
+
2019-07-08 Antoine Quint <[email protected]>
[Pointer Events] "touch-action: none" does not prevent double-tap-to-zoom
Modified: trunk/Source/WebKit/UIProcess/UIMessagePortChannelProvider.cpp (247217 => 247218)
--- trunk/Source/WebKit/UIProcess/UIMessagePortChannelProvider.cpp 2019-07-08 18:19:13 UTC (rev 247217)
+++ trunk/Source/WebKit/UIProcess/UIMessagePortChannelProvider.cpp 2019-07-08 18:26:47 UTC (rev 247218)
@@ -85,7 +85,7 @@
ASSERT_NOT_REACHED();
}
-void UIMessagePortChannelProvider::checkRemotePortForActivity(const MessagePortIdentifier&, CompletionHandler<void(HasActivity)>&&)
+void UIMessagePortChannelProvider::checkRemotePortForActivity(const MessagePortIdentifier&, Function<void(HasActivity)>&&)
{
// Should never be called in the UI process provider.
ASSERT_NOT_REACHED();
Modified: trunk/Source/WebKit/UIProcess/UIMessagePortChannelProvider.h (247217 => 247218)
--- trunk/Source/WebKit/UIProcess/UIMessagePortChannelProvider.h 2019-07-08 18:19:13 UTC (rev 247217)
+++ trunk/Source/WebKit/UIProcess/UIMessagePortChannelProvider.h 2019-07-08 18:26:47 UTC (rev 247218)
@@ -45,7 +45,7 @@
void messagePortClosed(const WebCore::MessagePortIdentifier& local) final;
void takeAllMessagesForPort(const WebCore::MessagePortIdentifier&, Function<void(Vector<WebCore::MessageWithMessagePorts>&&, Function<void()>&&)>&&) final;
void postMessageToRemote(WebCore::MessageWithMessagePorts&&, const WebCore::MessagePortIdentifier& remoteTarget) final;
- void checkRemotePortForActivity(const WebCore::MessagePortIdentifier& remoteTarget, CompletionHandler<void(HasActivity)>&& callback) final;
+ void checkRemotePortForActivity(const WebCore::MessagePortIdentifier& remoteTarget, Function<void(HasActivity)>&& callback) final;
void checkProcessLocalPortForActivity(const WebCore::MessagePortIdentifier&, WebCore::ProcessIdentifier, CompletionHandler<void(HasActivity)>&&) final;
WebCore::MessagePortChannelRegistry m_registry;
Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebMessagePortChannelProvider.cpp (247217 => 247218)
--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebMessagePortChannelProvider.cpp 2019-07-08 18:19:13 UTC (rev 247217)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebMessagePortChannelProvider.cpp 2019-07-08 18:26:47 UTC (rev 247218)
@@ -128,7 +128,7 @@
ASSERT_NOT_REACHED();
}
-void WebMessagePortChannelProvider::checkRemotePortForActivity(const MessagePortIdentifier& remoteTarget, CompletionHandler<void(HasActivity)>&& completionHandler)
+void WebMessagePortChannelProvider::checkRemotePortForActivity(const MessagePortIdentifier& remoteTarget, Function<void(HasActivity)>&& completionHandler)
{
static std::atomic<uint64_t> currentHandlerIdentifier;
uint64_t identifier = ++currentHandlerIdentifier;
@@ -135,10 +135,8 @@
{
Locker<Lock> locker(m_remoteActivityCallbackLock);
- auto result = m_remoteActivityCallbacks.ensure(identifier, [completionHandler = WTFMove(completionHandler)]() mutable {
- return WTFMove(completionHandler);
- });
- ASSERT_UNUSED(result, result.isNewEntry);
+ ASSERT(!m_remoteActivityCallbacks.contains(identifier));
+ m_remoteActivityCallbacks.set(identifier, WTFMove(completionHandler));
}
WebProcess::singleton().parentProcessConnection()->send(Messages::WebProcessProxy::CheckRemotePortForActivity(remoteTarget, identifier), 0);
Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebMessagePortChannelProvider.h (247217 => 247218)
--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebMessagePortChannelProvider.h 2019-07-08 18:19:13 UTC (rev 247217)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebMessagePortChannelProvider.h 2019-07-08 18:26:47 UTC (rev 247218)
@@ -52,12 +52,12 @@
void checkProcessLocalPortForActivity(const WebCore::MessagePortIdentifier&, WebCore::ProcessIdentifier, CompletionHandler<void(HasActivity)>&&) final;
// To be called only in the UI process
- void checkRemotePortForActivity(const WebCore::MessagePortIdentifier& remoteTarget, CompletionHandler<void(HasActivity)>&& callback) final;
+ void checkRemotePortForActivity(const WebCore::MessagePortIdentifier& remoteTarget, Function<void(HasActivity)>&& callback) final;
Lock m_takeAllMessagesCallbackLock;
- HashMap<uint64_t, CompletionHandler<void(Vector<WebCore::MessageWithMessagePorts>&&, Function<void()>&&)>> m_takeAllMessagesCallbacks;
+ HashMap<uint64_t, Function<void(Vector<WebCore::MessageWithMessagePorts>&&, Function<void()>&&)>> m_takeAllMessagesCallbacks;
Lock m_remoteActivityCallbackLock;
- HashMap<uint64_t, CompletionHandler<void(HasActivity)>> m_remoteActivityCallbacks;
+ HashMap<uint64_t, Function<void(HasActivity)>> m_remoteActivityCallbacks;
};
} // namespace WebKit
