Diff
Modified: trunk/LayoutTests/ChangeLog (227215 => 227216)
--- trunk/LayoutTests/ChangeLog 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/LayoutTests/ChangeLog 2018-01-19 18:38:51 UTC (rev 227216)
@@ -1,3 +1,15 @@
+2018-01-19 Chris Dumez <[email protected]>
+
+ The WebContent process should not process incoming IPC while waiting for a sync IPC reply
+ https://bugs.webkit.org/show_bug.cgi?id=181560
+
+ Reviewed by Ryosuke Niwa.
+
+ Add layout test coverage.
+
+ * fast/misc/testIncomingSyncIPCMessageWhileWaitingForSyncReply-expected.txt: Added.
+ * fast/misc/testIncomingSyncIPCMessageWhileWaitingForSyncReply.html: Added.
+
2018-01-19 Antoine Quint <[email protected]>
[Web Animations] Remove http/wpt/wk-web-animations tests
Added: trunk/LayoutTests/fast/misc/testIncomingSyncIPCMessageWhileWaitingForSyncReply-expected.txt (0 => 227216)
--- trunk/LayoutTests/fast/misc/testIncomingSyncIPCMessageWhileWaitingForSyncReply-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/misc/testIncomingSyncIPCMessageWhileWaitingForSyncReply-expected.txt 2018-01-19 18:38:51 UTC (rev 227216)
@@ -0,0 +1,10 @@
+Tests that the WebProcess does not process incoming sync IPC while waiting for a sync reply.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Did not crash
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/misc/testIncomingSyncIPCMessageWhileWaitingForSyncReply.html (0 => 227216)
--- trunk/LayoutTests/fast/misc/testIncomingSyncIPCMessageWhileWaitingForSyncReply.html (rev 0)
+++ trunk/LayoutTests/fast/misc/testIncomingSyncIPCMessageWhileWaitingForSyncReply.html 2018-01-19 18:38:51 UTC (rev 227216)
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description("Tests that the WebProcess does not process incoming sync IPC while waiting for a sync reply.");
+
+internals.testIncomingSyncIPCMessageWhileWaitingForSyncReply();
+testPassed("Did not crash");
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (227215 => 227216)
--- trunk/Source/WebCore/ChangeLog 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebCore/ChangeLog 2018-01-19 18:38:51 UTC (rev 227216)
@@ -1,3 +1,20 @@
+2018-01-19 Chris Dumez <[email protected]>
+
+ The WebContent process should not process incoming IPC while waiting for a sync IPC reply
+ https://bugs.webkit.org/show_bug.cgi?id=181560
+
+ Reviewed by Ryosuke Niwa.
+
+ Add internals API for testing purposes.
+
+ Test: fast/misc/testIncomingSyncIPCMessageWhileWaitingForSyncReply.html
+
+ * page/ChromeClient.h:
+ * testing/Internals.cpp:
+ (WebCore::Internals::testIncomingSyncIPCMessageWhileWaitingForSyncReply):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
2018-01-19 Keith Miller <[email protected]>
HaveInternalSDK includes should be "#include?"
Modified: trunk/Source/WebCore/page/ChromeClient.h (227215 => 227216)
--- trunk/Source/WebCore/page/ChromeClient.h 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebCore/page/ChromeClient.h 2018-01-19 18:38:51 UTC (rev 227216)
@@ -474,6 +474,8 @@
virtual void didInsertMenuItemElement(HTMLMenuItemElement&) { }
virtual void didRemoveMenuItemElement(HTMLMenuItemElement&) { }
+ virtual void testIncomingSyncIPCMessageWhileWaitingForSyncReply() { }
+
protected:
virtual ~ChromeClient() = default;
};
Modified: trunk/Source/WebCore/testing/Internals.cpp (227215 => 227216)
--- trunk/Source/WebCore/testing/Internals.cpp 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebCore/testing/Internals.cpp 2018-01-19 18:38:51 UTC (rev 227216)
@@ -4326,6 +4326,13 @@
timeline.setCurrentTime(Seconds::fromMilliseconds(currentTime));
}
+void Internals::testIncomingSyncIPCMessageWhileWaitingForSyncReply()
+{
+ ASSERT(contextDocument());
+ ASSERT(contextDocument()->page());
+ contextDocument()->page()->chrome().client().testIncomingSyncIPCMessageWhileWaitingForSyncReply();
+}
+
#if ENABLE(APPLE_PAY)
MockPaymentCoordinator& Internals::mockPaymentCoordinator() const
{
Modified: trunk/Source/WebCore/testing/Internals.h (227215 => 227216)
--- trunk/Source/WebCore/testing/Internals.h 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebCore/testing/Internals.h 2018-01-19 18:38:51 UTC (rev 227216)
@@ -638,6 +638,8 @@
void pauseTimeline(AnimationTimeline&);
void setTimelineCurrentTime(AnimationTimeline&, double);
+ void testIncomingSyncIPCMessageWhileWaitingForSyncReply();
+
private:
explicit Internals(Document&);
Document* contextDocument() const;
Modified: trunk/Source/WebCore/testing/Internals.idl (227215 => 227216)
--- trunk/Source/WebCore/testing/Internals.idl 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebCore/testing/Internals.idl 2018-01-19 18:38:51 UTC (rev 227216)
@@ -571,6 +571,8 @@
[Conditional=SERVICE_WORKER] Promise<boolean> hasServiceWorkerRegistration(DOMString scopeURL);
[Conditional=SERVICE_WORKER] void terminateServiceWorker(ServiceWorker worker);
+ void testIncomingSyncIPCMessageWhileWaitingForSyncReply();
+
[EnabledAtRuntime=WebAnimations] DOMString timelineDescription(AnimationTimeline timeline);
[EnabledAtRuntime=WebAnimations] void pauseTimeline(AnimationTimeline timeline);
[EnabledAtRuntime=WebAnimations] void setTimelineCurrentTime(AnimationTimeline timeline, double currentTime);
Modified: trunk/Source/WebKit/ChangeLog (227215 => 227216)
--- trunk/Source/WebKit/ChangeLog 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebKit/ChangeLog 2018-01-19 18:38:51 UTC (rev 227216)
@@ -1,3 +1,39 @@
+2018-01-19 Chris Dumez <[email protected]>
+
+ The WebContent process should not process incoming IPC while waiting for a sync IPC reply
+ https://bugs.webkit.org/show_bug.cgi?id=181560
+
+ Reviewed by Ryosuke Niwa.
+
+ The WebContent process should not process incoming IPC while waiting for a sync IPC reply.
+ This is causing hard-to-debug crashes because developers often assume the state does not
+ change during a sendSync() call.
+
+ * Platform/IPC/Connection.cpp:
+ (IPC::Connection::waitForSyncReply):
+ * Platform/IPC/Connection.h:
+ (IPC::Connection::setShouldProcessIncomingMessagesWhileWaitingForSyncReply):
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::testIncomingSyncIPCMessageWhileWaitingForSyncReply):
+ * UIProcess/WebProcessProxy.h:
+ * UIProcess/WebProcessProxy.messages.in:
+ * WebProcess/Network/NetworkProcessConnection.cpp:
+ (WebKit::NetworkProcessConnection::NetworkProcessConnection):
+ * WebProcess/Storage/WebToStorageProcessConnection.cpp:
+ (WebKit::WebToStorageProcessConnection::WebToStorageProcessConnection):
+ * WebProcess/WebConnectionToUIProcess.cpp:
+ (WebKit::WebConnectionToUIProcess::WebConnectionToUIProcess):
+ * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+ (WebKit::WebChromeClient::testIncomingSyncIPCMessageWhileWaitingForSyncReply):
+ * WebProcess/WebCoreSupport/WebChromeClient.h:
+ * WebProcess/WebCoreSupport/WebEditorClient.cpp:
+ (WebKit::WebEditorClient::undo):
+ (WebKit::WebEditorClient::redo):
+ * WebProcess/WebProcess.cpp:
+ (WebKit::WebProcess::syncIPCMessageWhileWaitingForSyncReplyForTesting):
+ * WebProcess/WebProcess.h:
+ * WebProcess/WebProcess.messages.in:
+
2018-01-19 Keith Miller <[email protected]>
HaveInternalSDK includes should be "#include?"
Modified: trunk/Source/WebKit/Platform/IPC/Connection.cpp (227215 => 227216)
--- trunk/Source/WebKit/Platform/IPC/Connection.cpp 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebKit/Platform/IPC/Connection.cpp 2018-01-19 18:38:51 UTC (rev 227216)
@@ -556,7 +556,8 @@
bool timedOut = false;
while (!timedOut) {
// First, check if we have any messages that we need to process.
- SyncMessageState::singleton().dispatchMessages(nullptr);
+ if (m_shouldProcessIncomingMessagesWhileWaitingForSyncReply || sendSyncOptions.contains(SendSyncOption::ProcessIncomingMessagesEvenWhenWaitingForSyncReply) || sendSyncOptions.contains(SendSyncOption::UseFullySynchronousModeForTesting) || m_inDispatchMessageMarkedToUseFullySynchronousModeForTesting)
+ SyncMessageState::singleton().dispatchMessages(nullptr);
{
LockHolder locker(m_syncReplyStateMutex);
Modified: trunk/Source/WebKit/Platform/IPC/Connection.h (227215 => 227216)
--- trunk/Source/WebKit/Platform/IPC/Connection.h 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebKit/Platform/IPC/Connection.h 2018-01-19 18:38:51 UTC (rev 227216)
@@ -64,6 +64,9 @@
// Use this to inform that this sync call will suspend this process until the user responds with input.
InformPlatformProcessWillSuspend = 1 << 0,
UseFullySynchronousModeForTesting = 1 << 1,
+
+ // This is the default except if setShouldProcessIncomingMessagesWhileWaitingForSyncReply(false) is called on the connection.
+ ProcessIncomingMessagesEvenWhenWaitingForSyncReply = 1 << 2,
};
enum class WaitForOption {
@@ -151,6 +154,8 @@
void setOnlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage(bool);
void setShouldExitOnSyncMessageSendFailure(bool);
+ void setShouldProcessIncomingMessagesWhileWaitingForSyncReply(bool value) { m_shouldProcessIncomingMessagesWhileWaitingForSyncReply = value; }
+
// The set callback will be called on the connection work queue when the connection is closed,
// before didCall is called on the client thread. Must be called before the connection is opened.
// In the future we might want a more generic way to handle sync or async messages directly
@@ -270,6 +275,8 @@
bool m_ignoreTimeoutsForTesting { false };
bool m_didReceiveInvalidMessage;
+ bool m_shouldProcessIncomingMessagesWhileWaitingForSyncReply { true };
+
// Incoming messages.
Lock m_incomingMessagesMutex;
Deque<std::unique_ptr<Decoder>> m_incomingMessages;
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (227215 => 227216)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2018-01-19 18:38:51 UTC (rev 227216)
@@ -811,6 +811,15 @@
}
}
+void WebProcessProxy::testIncomingSyncIPCMessageWhileWaitingForSyncReply(bool& handled)
+{
+ // Send Synchronous IPC back to the WebProcess while it is waiting for a sync reply from us.
+ // This should time out.
+ bool didSyncIPCsucceed = sendSync(Messages::WebProcess::SyncIPCMessageWhileWaitingForSyncReplyForTesting(), Messages::WebProcess::SyncIPCMessageWhileWaitingForSyncReplyForTesting::Reply(), 0, 100_ms);
+ RELEASE_ASSERT(!didSyncIPCsucceed);
+ handled = true;
+}
+
void WebProcessProxy::updateTextCheckerState()
{
if (canSendMessage())
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.h (227215 => 227216)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2018-01-19 18:38:51 UTC (rev 227216)
@@ -217,6 +217,7 @@
void didDestroyUserGestureToken(uint64_t);
void shouldTerminate(bool& shouldTerminate);
+ void testIncomingSyncIPCMessageWhileWaitingForSyncReply(bool& handled);
// Plugins
#if ENABLE(NETSCAPE_PLUGIN_API)
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.messages.in (227215 => 227216)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.messages.in 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.messages.in 2018-01-19 18:38:51 UTC (rev 227216)
@@ -54,4 +54,6 @@
MemoryPressureStatusChanged(bool isUnderMemoryPressure)
DidExceedInactiveMemoryLimitWhileActive()
+
+ TestIncomingSyncIPCMessageWhileWaitingForSyncReply() -> (bool handled)
}
Modified: trunk/Source/WebKit/WebProcess/Network/NetworkProcessConnection.cpp (227215 => 227216)
--- trunk/Source/WebKit/WebProcess/Network/NetworkProcessConnection.cpp 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebKit/WebProcess/Network/NetworkProcessConnection.cpp 2018-01-19 18:38:51 UTC (rev 227216)
@@ -54,6 +54,9 @@
NetworkProcessConnection::NetworkProcessConnection(IPC::Connection::Identifier connectionIdentifier)
: m_connection(IPC::Connection::createClientConnection(connectionIdentifier, *this))
{
+ // The WebProcess never processes incoming messages while waiting for a synchronous message reply as it would not be safe.
+ m_connection->setShouldProcessIncomingMessagesWhileWaitingForSyncReply(false);
+
m_connection->open();
}
Modified: trunk/Source/WebKit/WebProcess/Storage/WebToStorageProcessConnection.cpp (227215 => 227216)
--- trunk/Source/WebKit/WebProcess/Storage/WebToStorageProcessConnection.cpp 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebKit/WebProcess/Storage/WebToStorageProcessConnection.cpp 2018-01-19 18:38:51 UTC (rev 227216)
@@ -46,6 +46,9 @@
WebToStorageProcessConnection::WebToStorageProcessConnection(IPC::Connection::Identifier connectionIdentifier)
: m_connection(IPC::Connection::createClientConnection(connectionIdentifier, *this))
{
+ // The WebProcess never processes incoming messages while waiting for a synchronous message reply as it would not be safe.
+ m_connection->setShouldProcessIncomingMessagesWhileWaitingForSyncReply(false);
+
m_connection->open();
}
Modified: trunk/Source/WebKit/WebProcess/WebConnectionToUIProcess.cpp (227215 => 227216)
--- trunk/Source/WebKit/WebProcess/WebConnectionToUIProcess.cpp 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebKit/WebProcess/WebConnectionToUIProcess.cpp 2018-01-19 18:38:51 UTC (rev 227216)
@@ -42,6 +42,9 @@
: m_process(process)
{
m_process->addMessageReceiver(Messages::WebConnection::messageReceiverName(), *this);
+
+ // The WebProcess never processes incoming messages while waiting for a synchronous message reply as it would not be safe.
+ messageSenderConnection()->setShouldProcessIncomingMessagesWhileWaitingForSyncReply(false);
}
void WebConnectionToUIProcess::invalidate()
Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp (227215 => 227216)
--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp 2018-01-19 18:38:51 UTC (rev 227216)
@@ -712,6 +712,13 @@
m_page.sendSync(Messages::WebPageProxy::PrintFrame(webFrame->frameID()), Messages::WebPageProxy::PrintFrame::Reply(), Seconds::infinity(), IPC::SendSyncOption::InformPlatformProcessWillSuspend);
}
+void WebChromeClient::testIncomingSyncIPCMessageWhileWaitingForSyncReply()
+{
+ bool wasHandled = false;
+ WebProcess::singleton().parentProcessConnection()->sendSync(Messages::WebProcessProxy::TestIncomingSyncIPCMessageWhileWaitingForSyncReply(), Messages::WebProcessProxy::TestIncomingSyncIPCMessageWhileWaitingForSyncReply::Reply(wasHandled), 0);
+ RELEASE_ASSERT(wasHandled);
+}
+
void WebChromeClient::exceededDatabaseQuota(Frame& frame, const String& databaseName, DatabaseDetails details)
{
WebFrame* webFrame = WebFrame::fromCoreFrame(frame);
Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h (227215 => 227216)
--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h 2018-01-19 18:38:51 UTC (rev 227216)
@@ -135,6 +135,8 @@
void print(WebCore::Frame&) final;
+ void testIncomingSyncIPCMessageWhileWaitingForSyncReply() final;
+
void exceededDatabaseQuota(WebCore::Frame&, const String& databaseName, WebCore::DatabaseDetails) final;
void reachedMaxAppCacheSize(int64_t spaceNeeded) final;
Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.cpp (227215 => 227216)
--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.cpp 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebEditorClient.cpp 2018-01-19 18:38:51 UTC (rev 227216)
@@ -318,13 +318,13 @@
void WebEditorClient::undo()
{
bool result = false;
- m_page->sendSync(Messages::WebPageProxy::ExecuteUndoRedo(static_cast<uint32_t>(WebPageProxy::Undo)), Messages::WebPageProxy::ExecuteUndoRedo::Reply(result));
+ m_page->sendSync(Messages::WebPageProxy::ExecuteUndoRedo(static_cast<uint32_t>(WebPageProxy::Undo)), Messages::WebPageProxy::ExecuteUndoRedo::Reply(result), Seconds::infinity(), IPC::SendSyncOption::ProcessIncomingMessagesEvenWhenWaitingForSyncReply);
}
void WebEditorClient::redo()
{
bool result = false;
- m_page->sendSync(Messages::WebPageProxy::ExecuteUndoRedo(static_cast<uint32_t>(WebPageProxy::Redo)), Messages::WebPageProxy::ExecuteUndoRedo::Reply(result));
+ m_page->sendSync(Messages::WebPageProxy::ExecuteUndoRedo(static_cast<uint32_t>(WebPageProxy::Redo)), Messages::WebPageProxy::ExecuteUndoRedo::Reply(result), Seconds::infinity(), IPC::SendSyncOption::ProcessIncomingMessagesEvenWhenWaitingForSyncReply);
}
#if !PLATFORM(GTK) && !PLATFORM(COCOA) && !PLATFORM(WPE)
Modified: trunk/Source/WebKit/WebProcess/WebProcess.cpp (227215 => 227216)
--- trunk/Source/WebKit/WebProcess/WebProcess.cpp 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebKit/WebProcess/WebProcess.cpp 2018-01-19 18:38:51 UTC (rev 227216)
@@ -1050,6 +1050,10 @@
parentProcessConnection()->send(Messages::WebProcessProxy::DidReceiveBackgroundResponsivenessPing(), 0);
}
+void WebProcess::syncIPCMessageWhileWaitingForSyncReplyForTesting()
+{
+}
+
#if ENABLE(GAMEPAD)
void WebProcess::setInitialGamepads(const Vector<WebKit::GamepadData>& gamepadDatas)
Modified: trunk/Source/WebKit/WebProcess/WebProcess.h (227215 => 227216)
--- trunk/Source/WebKit/WebProcess/WebProcess.h 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebKit/WebProcess/WebProcess.h 2018-01-19 18:38:51 UTC (rev 227216)
@@ -292,6 +292,8 @@
void mainThreadPing();
void backgroundResponsivenessPing();
+ void syncIPCMessageWhileWaitingForSyncReplyForTesting();
+
#if ENABLE(GAMEPAD)
void setInitialGamepads(const Vector<GamepadData>&);
void gamepadConnected(const GamepadData&);
Modified: trunk/Source/WebKit/WebProcess/WebProcess.messages.in (227215 => 227216)
--- trunk/Source/WebKit/WebProcess/WebProcess.messages.in 2018-01-19 18:33:52 UTC (rev 227215)
+++ trunk/Source/WebKit/WebProcess/WebProcess.messages.in 2018-01-19 18:38:51 UTC (rev 227216)
@@ -103,6 +103,8 @@
MainThreadPing()
BackgroundResponsivenessPing()
+ SyncIPCMessageWhileWaitingForSyncReplyForTesting() -> ()
+
#if ENABLE(GAMEPAD)
SetInitialGamepads(Vector<WebKit::GamepadData> gamepadDatas)
GamepadConnected(WebKit::GamepadData gamepadData)
