[iOS] WebPage::TouchEventSync() & WebPage::GetPositionInformation() sync IPC causes...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Jul 2019 18:27:28 +0000 (18:27 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Jul 2019 18:27:28 +0000 (18:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=200138
<rdar://problem/52698157>

Reviewed by Geoffrey Garen.

Source/WebKit:

Revert most of r247822 and use an alternative approach to address hangs. In this patch, the proposal
is to add a SendSyncOption::ForceDispatchWhenDestinationIsWaitingForUnboundedSyncReply flag on the
WebPage::TouchEventSync() & WebPage::GetPositionInformation() sendSync() calls in the UIProcess.
Those will cause this IPCs to get dispatched right away in the WebContent process, even if the
WebContent process is itself currently stuck on unbounded (i.e. potentially slow) synchronous IPC
(JS alerts / prompts & sync XHR). Because re-entering WebCore on sync IPC is generally unsafe, this
patch also updates the WebPage::getPositionInformation() & WebPage::touchEventSync() to return early
(i.e. cancelled) if they get called while the WebContent process is stuck on a slow sendSync.

This approach should address the UIProcess hangs caused by the WebPage::TouchEventSync() and
WebPage::GetPositionInformation() sync IPC messages when the WebContent process is busy on a slow XHR
or a JS prompt / alert. It should be safe because we do not re-enter WebCore. The only drawback is that
those IPCs will be cancelled (early return with default value) when the WebContent process is busy.
However, I am being told that this is likely acceptable in practice.

* Platform/IPC/Connection.cpp:
(IPC::Connection::SyncMessageState::processIncomingMessage):
(IPC::Connection::sendMessage):
(IPC::Connection::sendSyncMessage):
(IPC::Connection::dispatchMessage):
* Platform/IPC/Connection.h:
(IPC::UnboundedSynchronousIPCScope::UnboundedSynchronousIPCScope):
(IPC::UnboundedSynchronousIPCScope::~UnboundedSynchronousIPCScope):
(IPC::UnboundedSynchronousIPCScope::hasOngoingUnboundedSyncIPC):
* Platform/IPC/Decoder.cpp:
(IPC::Decoder::shouldDispatchMessageWhenWaitingForSyncReply const):
* Platform/IPC/Decoder.h:
* Platform/IPC/Encoder.cpp:
(IPC::Encoder::shouldDispatchMessageWhenWaitingForSyncReply const):
(IPC::Encoder::setShouldDispatchMessageWhenWaitingForSyncReply):
(IPC::Encoder::wrapForTesting):
* Platform/IPC/Encoder.h:
* Platform/IPC/MessageFlags.h:
* UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::testProcessIncomingSyncMessagesWhenWaitingForSyncReply):
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::handleTouchEventSynchronously):
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView ensurePositionInformationIsUpToDate:]):
* WebProcess/Network/WebLoaderStrategy.cpp:
(WebKit::WebLoaderStrategy::loadResourceSynchronously):
* WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::testProcessIncomingSyncMessagesWhenWaitingForSyncReply):
(WebKit::WebChromeClient::runJavaScriptAlert):
(WebKit::WebChromeClient::runJavaScriptConfirm):
(WebKit::WebChromeClient::runJavaScriptPrompt):
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::layerVolatilityTimerFired):
(WebKit::WebPage::markLayersVolatile):
(WebKit::WebPage::cancelMarkLayersVolatile):
(WebKit::WebPage::touchEventSync):
(WebKit::WebPage::didCompletePageTransition):
(WebKit::WebPage::updatePreferences):
(WebKit::WebPage::testProcessIncomingSyncMessagesWhenWaitingForSyncReply):
* WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::sendSyncWithDelayedReply):
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::getPositionInformation):

LayoutTests:

Update existing layout test accordingly.

* fast/misc/{testProcessIncomingSyncMessagesWhenWaitingForUnboundedReply-expected.txt: Renamed from LayoutTests/fast/misc/testProcessIncomingSyncMessagesWhenWaitingForSyncReply-expected.txt.
* fast/misc/{testProcessIncomingSyncMessagesWhenWaitingForUnboundedReply.html: Renamed from LayoutTests/fast/misc/testProcessIncomingSyncMessagesWhenWaitingForSyncReply.html.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247868 268f45cc-cd09-0410-ab3c-d52691b4dbfc

19 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/misc/{testProcessIncomingSyncMessagesWhenWaitingForUnboundedReply-expected.txt [moved from LayoutTests/fast/misc/testProcessIncomingSyncMessagesWhenWaitingForSyncReply-expected.txt with 67% similarity]
LayoutTests/fast/misc/{testProcessIncomingSyncMessagesWhenWaitingForUnboundedReply.html [moved from LayoutTests/fast/misc/testProcessIncomingSyncMessagesWhenWaitingForSyncReply.html with 63% similarity]
Source/WebKit/ChangeLog
Source/WebKit/Platform/IPC/Connection.cpp
Source/WebKit/Platform/IPC/Connection.h
Source/WebKit/Platform/IPC/Decoder.cpp
Source/WebKit/Platform/IPC/Decoder.h
Source/WebKit/Platform/IPC/Encoder.cpp
Source/WebKit/Platform/IPC/Encoder.h
Source/WebKit/Platform/IPC/MessageFlags.h
Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp
Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Source/WebKit/WebProcess/WebPage/WebPage.h
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

index 3719d31..6e00a09 100644 (file)
@@ -1,3 +1,16 @@
+2019-07-26  Chris Dumez  <cdumez@apple.com>
+
+        [iOS] WebPage::TouchEventSync() & WebPage::GetPositionInformation() sync IPC causes UIProcess hangs
+        https://bugs.webkit.org/show_bug.cgi?id=200138
+        <rdar://problem/52698157>
+
+        Reviewed by Geoffrey Garen.
+
+        Update existing layout test accordingly.
+
+        * fast/misc/{testProcessIncomingSyncMessagesWhenWaitingForUnboundedReply-expected.txt: Renamed from LayoutTests/fast/misc/testProcessIncomingSyncMessagesWhenWaitingForSyncReply-expected.txt.
+        * fast/misc/{testProcessIncomingSyncMessagesWhenWaitingForUnboundedReply.html: Renamed from LayoutTests/fast/misc/testProcessIncomingSyncMessagesWhenWaitingForSyncReply.html.
+
 2019-07-26  Zalan Bujtas  <zalan@apple.com>
 
         Unable to tap/double tap to open files/folders in Google Drive in Safari
@@ -1,4 +1,4 @@
-Test for the SendSyncOption::ProcessIncomingSyncMessagesWhenWaitingForSyncReply sendSync flag
+Test for the SendSyncOption::ForceDispatchWhenDestinationIsWaitingForUnboundedSyncReply sendSync flag
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
@@ -3,7 +3,7 @@
 <body>
 <script src="../../resources/js-test.js"></script>
 <script>
-description("Test for the SendSyncOption::ProcessIncomingSyncMessagesWhenWaitingForSyncReply sendSync flag");
+description("Test for the SendSyncOption::ForceDispatchWhenDestinationIsWaitingForUnboundedSyncReply sendSync flag");
 
 shouldBeTrue("internals.testProcessIncomingSyncMessagesWhenWaitingForSyncReply()");
 </script>
index 0b28f27..dfb98c7 100644 (file)
@@ -1,3 +1,70 @@
+2019-07-26  Chris Dumez  <cdumez@apple.com>
+
+        [iOS] WebPage::TouchEventSync() & WebPage::GetPositionInformation() sync IPC causes UIProcess hangs
+        https://bugs.webkit.org/show_bug.cgi?id=200138
+        <rdar://problem/52698157>
+
+        Reviewed by Geoffrey Garen.
+
+        Revert most of r247822 and use an alternative approach to address hangs. In this patch, the proposal
+        is to add a SendSyncOption::ForceDispatchWhenDestinationIsWaitingForUnboundedSyncReply flag on the
+        WebPage::TouchEventSync() & WebPage::GetPositionInformation() sendSync() calls in the UIProcess.
+        Those will cause this IPCs to get dispatched right away in the WebContent process, even if the
+        WebContent process is itself currently stuck on unbounded (i.e. potentially slow) synchronous IPC
+        (JS alerts / prompts & sync XHR). Because re-entering WebCore on sync IPC is generally unsafe, this
+        patch also updates the WebPage::getPositionInformation() & WebPage::touchEventSync() to return early
+        (i.e. cancelled) if they get called while the WebContent process is stuck on a slow sendSync.
+
+        This approach should address the UIProcess hangs caused by the WebPage::TouchEventSync() and
+        WebPage::GetPositionInformation() sync IPC messages when the WebContent process is busy on a slow XHR
+        or a JS prompt / alert. It should be safe because we do not re-enter WebCore. The only drawback is that
+        those IPCs will be cancelled (early return with default value) when the WebContent process is busy.
+        However, I am being told that this is likely acceptable in practice.        
+
+        * Platform/IPC/Connection.cpp:
+        (IPC::Connection::SyncMessageState::processIncomingMessage):
+        (IPC::Connection::sendMessage):
+        (IPC::Connection::sendSyncMessage):
+        (IPC::Connection::dispatchMessage):
+        * Platform/IPC/Connection.h:
+        (IPC::UnboundedSynchronousIPCScope::UnboundedSynchronousIPCScope):
+        (IPC::UnboundedSynchronousIPCScope::~UnboundedSynchronousIPCScope):
+        (IPC::UnboundedSynchronousIPCScope::hasOngoingUnboundedSyncIPC):
+        * Platform/IPC/Decoder.cpp:
+        (IPC::Decoder::shouldDispatchMessageWhenWaitingForSyncReply const):
+        * Platform/IPC/Decoder.h:
+        * Platform/IPC/Encoder.cpp:
+        (IPC::Encoder::shouldDispatchMessageWhenWaitingForSyncReply const):
+        (IPC::Encoder::setShouldDispatchMessageWhenWaitingForSyncReply):
+        (IPC::Encoder::wrapForTesting):
+        * Platform/IPC/Encoder.h:
+        * Platform/IPC/MessageFlags.h:
+        * UIProcess/Network/NetworkProcessProxy.cpp:
+        (WebKit::NetworkProcessProxy::testProcessIncomingSyncMessagesWhenWaitingForSyncReply):
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::handleTouchEventSynchronously):
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView ensurePositionInformationIsUpToDate:]):
+        * WebProcess/Network/WebLoaderStrategy.cpp:
+        (WebKit::WebLoaderStrategy::loadResourceSynchronously):
+        * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+        (WebKit::WebChromeClient::testProcessIncomingSyncMessagesWhenWaitingForSyncReply):
+        (WebKit::WebChromeClient::runJavaScriptAlert):
+        (WebKit::WebChromeClient::runJavaScriptConfirm):
+        (WebKit::WebChromeClient::runJavaScriptPrompt):
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::layerVolatilityTimerFired):
+        (WebKit::WebPage::markLayersVolatile):
+        (WebKit::WebPage::cancelMarkLayersVolatile):
+        (WebKit::WebPage::touchEventSync):
+        (WebKit::WebPage::didCompletePageTransition):
+        (WebKit::WebPage::updatePreferences):
+        (WebKit::WebPage::testProcessIncomingSyncMessagesWhenWaitingForSyncReply):
+        * WebProcess/WebPage/WebPage.h:
+        (WebKit::WebPage::sendSyncWithDelayedReply):
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::getPositionInformation):
+
 2019-07-26  Zalan Bujtas  <zalan@apple.com>
 
         Unable to tap/double tap to open files/folders in Google Drive in Safari
index 6cbeba9..14afea0 100644 (file)
@@ -27,6 +27,7 @@
 #include "Connection.h"
 
 #include "Logging.h"
+#include "MessageFlags.h"
 #include <memory>
 #include <wtf/HashSet.h>
 #include <wtf/NeverDestroyed.h>
@@ -49,6 +50,8 @@ namespace IPC {
 const size_t maxPendingIncomingMessagesKillingThreshold { 50000 };
 #endif
 
+std::atomic<unsigned> UnboundedSynchronousIPCScope::unboundedSynchronousIPCCount = 0;
+
 struct Connection::ReplyHandler {
     RefPtr<FunctionDispatcher> dispatcher;
     Function<void (std::unique_ptr<Decoder>)> handler;
@@ -98,9 +101,6 @@ public:
     // from that connection and put the other messages back in the queue.
     void dispatchMessages(Connection* allowedConnection);
 
-    void incrementProcessIncomingSyncMessagesWhenWaitingForSyncReplyCount() { ++m_processIncomingSyncMessagesWhenWaitingForSyncReplyCount; }
-    void decrementProcessIncomingSyncMessagesWhenWaitingForSyncReplyCount() { --m_processIncomingSyncMessagesWhenWaitingForSyncReplyCount; }
-
 private:
     void dispatchMessageAndResetDidScheduleDispatchMessagesForConnection(Connection&);
 
@@ -117,8 +117,6 @@ private:
         std::unique_ptr<Decoder> message;
     };
     Vector<ConnectionAndIncomingMessage> m_messagesToDispatchWhileWaitingForSyncReply;
-
-    std::atomic<unsigned> m_processIncomingSyncMessagesWhenWaitingForSyncReplyCount { 0 };
 };
 
 Connection::SyncMessageState& Connection::SyncMessageState::singleton()
@@ -139,17 +137,16 @@ Connection::SyncMessageState::SyncMessageState()
 
 bool Connection::SyncMessageState::processIncomingMessage(Connection& connection, std::unique_ptr<Decoder>& message)
 {
-    bool shouldDispatchMessageWhenWaitingForSyncReply = message->shouldDispatchMessageWhenWaitingForSyncReply();
-
-    // We dispatch synchronous messages even if shouldDispatchMessageWhenWaitingForSyncReply returns false if the
-    // sendSync() used SendSyncOption::ProcessIncomingSyncMessagesWhenWaitingForSyncReply. This is used for some messages
-    // in the WebContent process (which normally does not dispatch messages when waiting for a sync reply), to avoid
-    // hangs.
-    if (!shouldDispatchMessageWhenWaitingForSyncReply && message->isSyncMessage() && m_processIncomingSyncMessagesWhenWaitingForSyncReplyCount.load())
-        shouldDispatchMessageWhenWaitingForSyncReply = true;
-
-    if (!shouldDispatchMessageWhenWaitingForSyncReply)
+    switch (message->shouldDispatchMessageWhenWaitingForSyncReply()) {
+    case ShouldDispatchWhenWaitingForSyncReply::No:
         return false;
+    case ShouldDispatchWhenWaitingForSyncReply::YesDuringUnboundedIPC:
+        if (!UnboundedSynchronousIPCScope::hasOngoingUnboundedSyncIPC())
+            return false;
+        break;
+    case ShouldDispatchWhenWaitingForSyncReply::Yes:
+        break;
+    }
 
     ConnectionAndIncomingMessage connectionAndIncomingMessage { connection, WTFMove(message) };
 
@@ -433,7 +430,9 @@ bool Connection::sendMessage(std::unique_ptr<Encoder> encoder, OptionSet<SendOpt
     if (sendOptions.contains(SendOption::DispatchMessageEvenWhenWaitingForSyncReply)
         && (!m_onlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage
             || m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount))
-        encoder->setShouldDispatchMessageWhenWaitingForSyncReply(true);
+        encoder->setShouldDispatchMessageWhenWaitingForSyncReply(ShouldDispatchWhenWaitingForSyncReply::Yes);
+    else if (sendOptions.contains(SendOption::DispatchMessageEvenWhenWaitingForUnboundedSyncReply))
+        encoder->setShouldDispatchMessageWhenWaitingForSyncReply(ShouldDispatchWhenWaitingForSyncReply::YesDuringUnboundedIPC);
 
     {
         std::lock_guard<Lock> lock(m_outgoingMessagesMutex);
@@ -577,19 +576,17 @@ std::unique_ptr<Decoder> Connection::sendSyncMessage(uint64_t syncRequestID, std
     ++m_inSendSyncCount;
 
     // First send the message.
-    sendMessage(WTFMove(encoder), IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply);
+    OptionSet<SendOption> sendOptions = IPC::SendOption::DispatchMessageEvenWhenWaitingForSyncReply;
+    if (sendSyncOptions.contains(SendSyncOption::ForceDispatchWhenDestinationIsWaitingForUnboundedSyncReply))
+        sendOptions = sendOptions | IPC::SendOption::DispatchMessageEvenWhenWaitingForUnboundedSyncReply;
 
-    if (sendSyncOptions.contains(SendSyncOption::ProcessIncomingSyncMessagesWhenWaitingForSyncReply))
-        SyncMessageState::singleton().incrementProcessIncomingSyncMessagesWhenWaitingForSyncReplyCount();
+    sendMessage(WTFMove(encoder), sendOptions);
 
     // Then wait for a reply. Waiting for a reply could involve dispatching incoming sync messages, so
     // keep an extra reference to the connection here in case it's invalidated.
     Ref<Connection> protect(*this);
     std::unique_ptr<Decoder> reply = waitForSyncReply(syncRequestID, timeout, sendSyncOptions);
 
-    if (sendSyncOptions.contains(SendSyncOption::ProcessIncomingSyncMessagesWhenWaitingForSyncReply))
-        SyncMessageState::singleton().decrementProcessIncomingSyncMessagesWhenWaitingForSyncReplyCount();
-
     --m_inSendSyncCount;
 
     // Finally, pop the pending sync reply information.
@@ -1040,8 +1037,11 @@ void Connection::dispatchMessage(std::unique_ptr<Decoder> message)
     }
 
     m_inDispatchMessageCount++;
+    
+    bool isDispatchingMessageWhileWaitingForSyncReply = (message->shouldDispatchMessageWhenWaitingForSyncReply() == ShouldDispatchWhenWaitingForSyncReply::Yes)
+        || (message->shouldDispatchMessageWhenWaitingForSyncReply() == ShouldDispatchWhenWaitingForSyncReply::YesDuringUnboundedIPC && UnboundedSynchronousIPCScope::hasOngoingUnboundedSyncIPC());
 
-    if (message->shouldDispatchMessageWhenWaitingForSyncReply())
+    if (isDispatchingMessageWhileWaitingForSyncReply)
         m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount++;
 
     bool oldDidReceiveInvalidMessage = m_didReceiveInvalidMessage;
@@ -1057,7 +1057,7 @@ void Connection::dispatchMessage(std::unique_ptr<Decoder> message)
 
     // FIXME: For synchronous messages, we should not decrement the counter until we send a response.
     // Otherwise, we would deadlock if processing the message results in a sync message back after we exit this function.
-    if (message->shouldDispatchMessageWhenWaitingForSyncReply())
+    if (isDispatchingMessageWhileWaitingForSyncReply)
         m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount--;
 
     if (message->shouldUseFullySynchronousModeForTesting())
index a4907d1..3577e04 100644 (file)
@@ -61,14 +61,15 @@ enum class SendOption {
     // Whether this message should be dispatched when waiting for a sync reply.
     // This is the default for synchronous messages.
     DispatchMessageEvenWhenWaitingForSyncReply = 1 << 0,
-    IgnoreFullySynchronousMode = 1 << 1,
+    DispatchMessageEvenWhenWaitingForUnboundedSyncReply = 1 << 1,
+    IgnoreFullySynchronousMode = 1 << 2,
 };
 
 enum class SendSyncOption {
     // Use this to inform that this sync call will suspend this process until the user responds with input.
     InformPlatformProcessWillSuspend = 1 << 0,
     UseFullySynchronousModeForTesting = 1 << 1,
-    ProcessIncomingSyncMessagesWhenWaitingForSyncReply = 1 << 2,
+    ForceDispatchWhenDestinationIsWaitingForUnboundedSyncReply = 1 << 2,
 };
 
 enum class WaitForOption {
@@ -554,4 +555,28 @@ template<typename T> bool Connection::waitForAndDispatchImmediately(uint64_t des
     return true;
 }
 
+class UnboundedSynchronousIPCScope {
+public:
+    UnboundedSynchronousIPCScope()
+    {
+        ASSERT(RunLoop::isMain());
+        ++unboundedSynchronousIPCCount;
+    }
+
+    ~UnboundedSynchronousIPCScope()
+    {
+        ASSERT(RunLoop::isMain());
+        ASSERT(unboundedSynchronousIPCCount);
+        --unboundedSynchronousIPCCount;
+    }
+
+    static bool hasOngoingUnboundedSyncIPC()
+    {
+        return unboundedSynchronousIPCCount.load() > 0;
+    }
+
+private:
+    static std::atomic<unsigned> unboundedSynchronousIPCCount;
+};
+
 } // namespace IPC
index 154c3d9..cca0739 100644 (file)
@@ -88,9 +88,13 @@ bool Decoder::isSyncMessage() const
     return m_messageFlags & SyncMessage;
 }
 
-bool Decoder::shouldDispatchMessageWhenWaitingForSyncReply() const
+ShouldDispatchWhenWaitingForSyncReply Decoder::shouldDispatchMessageWhenWaitingForSyncReply() const
 {
-    return m_messageFlags & DispatchMessageWhenWaitingForSyncReply;
+    if (m_messageFlags & DispatchMessageWhenWaitingForSyncReply)
+        return ShouldDispatchWhenWaitingForSyncReply::Yes;
+    if (m_messageFlags & DispatchMessageWhenWaitingForUnboundedSyncReply)
+        return ShouldDispatchWhenWaitingForSyncReply::YesDuringUnboundedIPC;
+    return ShouldDispatchWhenWaitingForSyncReply::No;
 }
 
 bool Decoder::shouldUseFullySynchronousModeForTesting() const
index 4e0154b..b3e641d 100644 (file)
@@ -39,6 +39,7 @@ namespace IPC {
 
 class DataReference;
 class ImportanceAssertion;
+enum class ShouldDispatchWhenWaitingForSyncReply;
 
 class Decoder {
     WTF_MAKE_FAST_ALLOCATED;
@@ -54,7 +55,7 @@ public:
     uint64_t destinationID() const { return m_destinationID; }
 
     bool isSyncMessage() const;
-    bool shouldDispatchMessageWhenWaitingForSyncReply() const;
+    ShouldDispatchWhenWaitingForSyncReply shouldDispatchMessageWhenWaitingForSyncReply() const;
     bool shouldUseFullySynchronousModeForTesting() const;
 
 #if PLATFORM(MAC)
index b586162..1c45bc7 100644 (file)
@@ -85,9 +85,13 @@ bool Encoder::isSyncMessage() const
     return *buffer() & SyncMessage;
 }
 
-bool Encoder::shouldDispatchMessageWhenWaitingForSyncReply() const
+ShouldDispatchWhenWaitingForSyncReply Encoder::shouldDispatchMessageWhenWaitingForSyncReply() const
 {
-    return *buffer() & DispatchMessageWhenWaitingForSyncReply;
+    if (*buffer() & DispatchMessageWhenWaitingForSyncReply)
+        return ShouldDispatchWhenWaitingForSyncReply::Yes;
+    if (*buffer() & DispatchMessageWhenWaitingForUnboundedSyncReply)
+        return ShouldDispatchWhenWaitingForSyncReply::YesDuringUnboundedIPC;
+    return ShouldDispatchWhenWaitingForSyncReply::No;
 }
 
 void Encoder::setIsSyncMessage(bool isSyncMessage)
@@ -98,12 +102,21 @@ void Encoder::setIsSyncMessage(bool isSyncMessage)
         *buffer() &= ~SyncMessage;
 }
 
-void Encoder::setShouldDispatchMessageWhenWaitingForSyncReply(bool shouldDispatchMessageWhenWaitingForSyncReply)
+void Encoder::setShouldDispatchMessageWhenWaitingForSyncReply(ShouldDispatchWhenWaitingForSyncReply shouldDispatchWhenWaitingForSyncReply)
 {
-    if (shouldDispatchMessageWhenWaitingForSyncReply)
+    switch (shouldDispatchWhenWaitingForSyncReply) {
+    case ShouldDispatchWhenWaitingForSyncReply::No:
+        *buffer() &= ~(DispatchMessageWhenWaitingForSyncReply | DispatchMessageWhenWaitingForUnboundedSyncReply);
+        break;
+    case ShouldDispatchWhenWaitingForSyncReply::Yes:
         *buffer() |= DispatchMessageWhenWaitingForSyncReply;
-    else
+        *buffer() &= ~DispatchMessageWhenWaitingForUnboundedSyncReply;
+        break;
+    case ShouldDispatchWhenWaitingForSyncReply::YesDuringUnboundedIPC:
+        *buffer() |= DispatchMessageWhenWaitingForUnboundedSyncReply;
         *buffer() &= ~DispatchMessageWhenWaitingForSyncReply;
+        break;
+    }
 }
 
 void Encoder::setFullySynchronousModeForTesting()
@@ -116,7 +129,7 @@ void Encoder::wrapForTesting(std::unique_ptr<Encoder> original)
     ASSERT(isSyncMessage());
     ASSERT(!original->isSyncMessage());
 
-    original->setShouldDispatchMessageWhenWaitingForSyncReply(true);
+    original->setShouldDispatchMessageWhenWaitingForSyncReply(ShouldDispatchWhenWaitingForSyncReply::Yes);
 
     encodeVariableLengthByteArray(DataReference(original->buffer(), original->bufferSize()));
 
index 294796b..813a27f 100644 (file)
@@ -34,6 +34,7 @@
 namespace IPC {
 
 class DataReference;
+enum class ShouldDispatchWhenWaitingForSyncReply;
 
 class Encoder final {
     WTF_MAKE_FAST_ALLOCATED;
@@ -48,8 +49,8 @@ public:
     void setIsSyncMessage(bool);
     bool isSyncMessage() const;
 
-    void setShouldDispatchMessageWhenWaitingForSyncReply(bool);
-    bool shouldDispatchMessageWhenWaitingForSyncReply() const;
+    void setShouldDispatchMessageWhenWaitingForSyncReply(ShouldDispatchWhenWaitingForSyncReply);
+    ShouldDispatchWhenWaitingForSyncReply shouldDispatchMessageWhenWaitingForSyncReply() const;
 
     void setFullySynchronousModeForTesting();
 
index e8d3e5e..97f0782 100644 (file)
@@ -31,9 +31,12 @@ namespace IPC {
 enum MessageFlags {
     SyncMessage = 1 << 0,
     DispatchMessageWhenWaitingForSyncReply = 1 << 1,
-    UseFullySynchronousModeForTesting = 1 << 2,
+    DispatchMessageWhenWaitingForUnboundedSyncReply = 1 << 2,
+    UseFullySynchronousModeForTesting = 1 << 3,
 };
 
+enum class ShouldDispatchWhenWaitingForSyncReply { No, Yes, YesDuringUnboundedIPC };
+
 } // namespace IPC
 
 #endif // MessageFlags_h
index a9dc327..6d666c9 100644 (file)
@@ -1211,7 +1211,7 @@ void NetworkProcessProxy::testProcessIncomingSyncMessagesWhenWaitingForSyncReply
         return reply(false);
 
     bool handled = false;
-    if (!page->sendSync(Messages::WebPage::TestProcessIncomingSyncMessagesWhenWaitingForSyncReply(), Messages::WebPage::TestProcessIncomingSyncMessagesWhenWaitingForSyncReply::Reply(handled)))
+    if (!page->sendSync(Messages::WebPage::TestProcessIncomingSyncMessagesWhenWaitingForSyncReply(), Messages::WebPage::TestProcessIncomingSyncMessagesWhenWaitingForSyncReply::Reply(handled), Seconds::infinity(), IPC::SendSyncOption::ForceDispatchWhenDestinationIsWaitingForUnboundedSyncReply))
         return reply(false);
     reply(handled);
 }
index ba636fc..e4133c6 100644 (file)
@@ -2669,7 +2669,7 @@ void WebPageProxy::handleTouchEventSynchronously(NativeWebTouchEvent& event)
 
     m_process->responsivenessTimer().start();
     bool handled = false;
-    bool replyReceived = m_process->sendSync(Messages::WebPage::TouchEventSync(event), Messages::WebPage::TouchEventSync::Reply(handled), m_pageID, 1_s);
+    bool replyReceived = m_process->sendSync(Messages::WebPage::TouchEventSync(event), Messages::WebPage::TouchEventSync::Reply(handled), m_pageID, 1_s, IPC::SendSyncOption::ForceDispatchWhenDestinationIsWaitingForUnboundedSyncReply);
     // If the sync request has timed out, we should consider the event handled. The Web Process is too busy to answer any questions, so the default action is also likely to have issues.
     if (!replyReceived)
         handled = true;
index 8fa7a14..6222f6d 100644 (file)
@@ -1999,7 +1999,7 @@ static inline bool isSamePair(UIGestureRecognizer *a, UIGestureRecognizer *b, UI
     if ([self _hasValidOutstandingPositionInformationRequest:request])
         return connection->waitForAndDispatchImmediately<Messages::WebPageProxy::DidReceivePositionInformation>(_page->pageID(), 1_s, IPC::WaitForOption::InterruptWaitingIfSyncMessageArrives);
 
-    bool receivedResponse = _page->process().sendSync(Messages::WebPage::GetPositionInformation(request), Messages::WebPage::GetPositionInformation::Reply(_positionInformation), _page->pageID(), 1_s);
+    bool receivedResponse = _page->process().sendSync(Messages::WebPage::GetPositionInformation(request), Messages::WebPage::GetPositionInformation::Reply(_positionInformation), _page->pageID(), 1_s, IPC::SendSyncOption::ForceDispatchWhenDestinationIsWaitingForUnboundedSyncReply);
     _hasValidPositionInformation = receivedResponse && _positionInformation.canBeValid;
     
     // FIXME: We need to clean up these handlers in the event that we are not able to collect data, or if the WebProcess crashes.
index e21f4a1..5026954 100644 (file)
@@ -564,6 +564,7 @@ void WebLoaderStrategy::loadResourceSynchronously(FrameLoader& frameLoader, unsi
     data.shrink(0);
 
     HangDetectionDisabler hangDetectionDisabler;
+    IPC::UnboundedSynchronousIPCScope unboundedSynchronousIPCScope;
 
     bool shouldNotifyOfUpload = request.hasUpload() && m_loadersWithUploads.isEmpty();
     if (shouldNotifyOfUpload)
index 9921dfb..1a024f7 100644 (file)
@@ -307,8 +307,9 @@ Page* WebChromeClient::createWindow(Frame& frame, const FrameLoadRequest& reques
 
 bool WebChromeClient::testProcessIncomingSyncMessagesWhenWaitingForSyncReply()
 {
+    IPC::UnboundedSynchronousIPCScope unboundedSynchronousIPCScope;
     bool handled = false;
-    if (!WebProcess::singleton().ensureNetworkProcessConnection().connection().sendSync(Messages::NetworkConnectionToWebProcess::TestProcessIncomingSyncMessagesWhenWaitingForSyncReply(m_page.pageID()), Messages::NetworkConnectionToWebProcess::TestProcessIncomingSyncMessagesWhenWaitingForSyncReply::Reply(handled), 0, Seconds::infinity(), IPC::SendSyncOption::ProcessIncomingSyncMessagesWhenWaitingForSyncReply))
+    if (!WebProcess::singleton().ensureNetworkProcessConnection().connection().sendSync(Messages::NetworkConnectionToWebProcess::TestProcessIncomingSyncMessagesWhenWaitingForSyncReply(m_page.pageID()), Messages::NetworkConnectionToWebProcess::TestProcessIncomingSyncMessagesWhenWaitingForSyncReply::Reply(handled), 0))
         return false;
     return handled;
 }
@@ -467,8 +468,9 @@ void WebChromeClient::runJavaScriptAlert(Frame& frame, const String& alertText)
     m_page.injectedBundleUIClient().willRunJavaScriptAlert(&m_page, alertText, webFrame);
 
     HangDetectionDisabler hangDetectionDisabler;
+    IPC::UnboundedSynchronousIPCScope unboundedSynchronousIPCScope;
 
-    m_page.sendSyncWithDelayedReply(Messages::WebPageProxy::RunJavaScriptAlert(webFrame->frameID(), SecurityOriginData::fromFrame(&frame), alertText), Messages::WebPageProxy::RunJavaScriptAlert::Reply(), IPC::SendSyncOption::ProcessIncomingSyncMessagesWhenWaitingForSyncReply);
+    m_page.sendSyncWithDelayedReply(Messages::WebPageProxy::RunJavaScriptAlert(webFrame->frameID(), SecurityOriginData::fromFrame(&frame), alertText), Messages::WebPageProxy::RunJavaScriptAlert::Reply());
 }
 
 bool WebChromeClient::runJavaScriptConfirm(Frame& frame, const String& message)
@@ -483,9 +485,10 @@ bool WebChromeClient::runJavaScriptConfirm(Frame& frame, const String& message)
     m_page.injectedBundleUIClient().willRunJavaScriptConfirm(&m_page, message, webFrame);
 
     HangDetectionDisabler hangDetectionDisabler;
+    IPC::UnboundedSynchronousIPCScope unboundedSynchronousIPCScope;
 
     bool result = false;
-    if (!m_page.sendSyncWithDelayedReply(Messages::WebPageProxy::RunJavaScriptConfirm(webFrame->frameID(), SecurityOriginData::fromFrame(&frame), message), Messages::WebPageProxy::RunJavaScriptConfirm::Reply(result), IPC::SendSyncOption::ProcessIncomingSyncMessagesWhenWaitingForSyncReply))
+    if (!m_page.sendSyncWithDelayedReply(Messages::WebPageProxy::RunJavaScriptConfirm(webFrame->frameID(), SecurityOriginData::fromFrame(&frame), message), Messages::WebPageProxy::RunJavaScriptConfirm::Reply(result)))
         return false;
 
     return result;
@@ -503,8 +506,9 @@ bool WebChromeClient::runJavaScriptPrompt(Frame& frame, const String& message, c
     m_page.injectedBundleUIClient().willRunJavaScriptPrompt(&m_page, message, defaultValue, webFrame);
 
     HangDetectionDisabler hangDetectionDisabler;
+    IPC::UnboundedSynchronousIPCScope unboundedSynchronousIPCScope;
 
-    if (!m_page.sendSyncWithDelayedReply(Messages::WebPageProxy::RunJavaScriptPrompt(webFrame->frameID(), SecurityOriginData::fromFrame(&frame), message, defaultValue), Messages::WebPageProxy::RunJavaScriptPrompt::Reply(result), IPC::SendSyncOption::ProcessIncomingSyncMessagesWhenWaitingForSyncReply))
+    if (!m_page.sendSyncWithDelayedReply(Messages::WebPageProxy::RunJavaScriptPrompt(webFrame->frameID(), SecurityOriginData::fromFrame(&frame), message, defaultValue), Messages::WebPageProxy::RunJavaScriptPrompt::Reply(result)))
         return false;
 
     return !result.isNull();
index 9b5c403..85c4b99 100644 (file)
@@ -316,8 +316,8 @@ static const Seconds pageScrollHysteresisDuration { 300_ms };
 static const Seconds initialLayerVolatilityTimerInterval { 20_ms };
 static const Seconds maximumLayerVolatilityTimerInterval { 2_s };
 
-#define RELEASE_LOG_IF_ALLOWED(...) RELEASE_LOG_IF(isAlwaysOnLoggingAllowed(), Layers, __VA_ARGS__)
-#define RELEASE_LOG_ERROR_IF_ALLOWED(...) RELEASE_LOG_ERROR_IF(isAlwaysOnLoggingAllowed(), Layers, __VA_ARGS__)
+#define RELEASE_LOG_IF_ALLOWED(channel, fmt, ...) RELEASE_LOG_IF(isAlwaysOnLoggingAllowed(), channel, "%p - WebPage::" fmt, this, ##__VA_ARGS__)
+#define RELEASE_LOG_ERROR_IF_ALLOWED(channel, fmt, ...) RELEASE_LOG_ERROR_IF(isAlwaysOnLoggingAllowed(), channel, "%p - WebPage::" fmt, this, ##__VA_ARGS__)
 
 class SendStopResponsivenessTimer {
 public:
@@ -2556,14 +2556,14 @@ void WebPage::layerVolatilityTimerFired()
     if (didSucceed || newInterval > maximumLayerVolatilityTimerInterval) {
         m_layerVolatilityTimer.stop();
         if (didSucceed)
-            RELEASE_LOG_IF_ALLOWED("%p - WebPage - Succeeded in marking layers as volatile", this);
+            RELEASE_LOG_IF_ALLOWED(Layers, "layerVolatilityTimerFired: Succeeded in marking layers as volatile");
         else
-            RELEASE_LOG_IF_ALLOWED("%p - WebPage - Failed to mark layers as volatile within %gms", this, maximumLayerVolatilityTimerInterval.milliseconds());
+            RELEASE_LOG_IF_ALLOWED(Layers, "layerVolatilityTimerFired: Failed to mark layers as volatile within %gms", maximumLayerVolatilityTimerInterval.milliseconds());
         callVolatilityCompletionHandlers(didSucceed);
         return;
     }
 
-    RELEASE_LOG_ERROR_IF_ALLOWED("%p - WebPage - Failed to mark all layers as volatile, will retry in %g ms", this, newInterval.milliseconds());
+    RELEASE_LOG_ERROR_IF_ALLOWED(Layers, "layerVolatilityTimerFired: Failed to mark all layers as volatile, will retry in %g ms", newInterval.milliseconds());
     m_layerVolatilityTimer.startRepeating(newInterval);
 }
 
@@ -2574,7 +2574,7 @@ bool WebPage::markLayersVolatileImmediatelyIfPossible()
 
 void WebPage::markLayersVolatile(WTF::Function<void (bool)>&& completionHandler)
 {
-    RELEASE_LOG_IF_ALLOWED("%p - WebPage::markLayersVolatile()", this);
+    RELEASE_LOG_IF_ALLOWED(Layers, "markLayersVolatile");
 
     if (m_layerVolatilityTimer.isActive())
         m_layerVolatilityTimer.stop();
@@ -2585,22 +2585,22 @@ void WebPage::markLayersVolatile(WTF::Function<void (bool)>&& completionHandler)
     bool didSucceed = markLayersVolatileImmediatelyIfPossible();
     if (didSucceed || m_isSuspendedUnderLock) {
         if (didSucceed)
-            RELEASE_LOG_IF_ALLOWED("%p - WebPage - Successfully marked layers as volatile", this);
+            RELEASE_LOG_IF_ALLOWED(Layers, "markLayersVolatile: Successfully marked layers as volatile");
         else {
             // If we get suspended when locking the screen, it is expected that some IOSurfaces cannot be marked as purgeable so we do not keep retrying.
-            RELEASE_LOG_IF_ALLOWED("%p - WebPage - Did what we could to mark IOSurfaces as purgeable after locking the screen", this);
+            RELEASE_LOG_IF_ALLOWED(Layers, "markLayersVolatile: Did what we could to mark IOSurfaces as purgeable after locking the screen");
         }
         callVolatilityCompletionHandlers(didSucceed);
         return;
     }
 
-    RELEASE_LOG_IF_ALLOWED("%p - Failed to mark all layers as volatile, will retry in %g ms", this, initialLayerVolatilityTimerInterval.milliseconds());
+    RELEASE_LOG_IF_ALLOWED(Layers, "markLayersVolatile: Failed to mark all layers as volatile, will retry in %g ms", initialLayerVolatilityTimerInterval.milliseconds());
     m_layerVolatilityTimer.startRepeating(initialLayerVolatilityTimerInterval);
 }
 
 void WebPage::cancelMarkLayersVolatile()
 {
-    RELEASE_LOG_IF_ALLOWED("%p - WebPage::cancelMarkLayersVolatile()", this);
+    RELEASE_LOG_IF_ALLOWED(Layers, "cancelMarkLayersVolatile");
     m_layerVolatilityTimer.stop();
     m_markLayersAsVolatileCompletionHandlers.clear();
 }
@@ -2910,6 +2910,12 @@ void WebPage::dispatchTouchEvent(const WebTouchEvent& touchEvent, bool& handled)
 
 void WebPage::touchEventSync(const WebTouchEvent& touchEvent, CompletionHandler<void(bool)>&& reply)
 {
+    // Avoid UIProcess hangs when the WebContent process is stuck on a sync IPC.
+    if (IPC::UnboundedSynchronousIPCScope::hasOngoingUnboundedSyncIPC()) {
+        RELEASE_LOG_ERROR_IF_ALLOWED(Process, "touchEventSync - Not processing because the process is stuck on unbounded sync IPC");
+        return reply(true);
+    }
+
     m_pendingSynchronousTouchEventReply = WTFMove(reply);
 
     EventDispatcher::TouchEventQueue queuedEvents;
@@ -3279,7 +3285,7 @@ void WebPage::didCompletePageTransition()
 {
     unfreezeLayerTree(LayerTreeFreezeReason::PageTransition);
 
-    RELEASE_LOG_IF_ALLOWED("%p - WebPage - Did complete page transition", this);
+    RELEASE_LOG_IF_ALLOWED(Layers, "didCompletePageTransition: Did complete page transition");
 
     bool isInitialEmptyDocument = !m_mainFrame;
     if (!isInitialEmptyDocument)
@@ -3544,7 +3550,7 @@ void WebPage::updatePreferences(const WebPreferencesStore& store)
 
 #if !PLATFORM(GTK) && !PLATFORM(WIN)
     if (!settings.acceleratedCompositingEnabled()) {
-        RELEASE_LOG_IF_ALLOWED("%p - WebPage - acceleratedCompositingEnabled setting was false. WebKit cannot function in this mode; changing setting to true", this);
+        RELEASE_LOG_IF_ALLOWED(Layers, "updatePreferences: acceleratedCompositingEnabled setting was false. WebKit cannot function in this mode; changing setting to true");
         settings.setAcceleratedCompositingEnabled(true);
     }
 #endif
@@ -5885,6 +5891,7 @@ void WebPage::didRemoveMenuItemElement(HTMLMenuItemElement& element)
 
 void WebPage::testProcessIncomingSyncMessagesWhenWaitingForSyncReply(Messages::WebPage::TestProcessIncomingSyncMessagesWhenWaitingForSyncReply::DelayedReply&& reply)
 {
+    RELEASE_ASSERT(IPC::UnboundedSynchronousIPCScope::hasOngoingUnboundedSyncIPC());
     reply(true);
 }
 
index c6254a7..95f2e05 100644 (file)
@@ -1167,10 +1167,10 @@ public:
     void didReceiveWebPageMessage(IPC::Connection&, IPC::Decoder&);
 
     template<typename T>
-    bool sendSyncWithDelayedReply(T&& message, typename T::Reply&& reply, OptionSet<IPC::SendSyncOption> sendSyncOptions = { })
+    bool sendSyncWithDelayedReply(T&& message, typename T::Reply&& reply)
     {
         cancelGesturesBlockedOnSynchronousReplies();
-        return sendSync(WTFMove(message), WTFMove(reply), Seconds::infinity(), sendSyncOptions | IPC::SendSyncOption::InformPlatformProcessWillSuspend);
+        return sendSync(WTFMove(message), WTFMove(reply), Seconds::infinity(), IPC::SendSyncOption::InformPlatformProcessWillSuspend);
     }
 
     WebCore::DOMPasteAccessResponse requestDOMPasteAccess(const String& originIdentifier);
index 617df8e..22952e3 100644 (file)
 #import <wtf/cocoa/Entitlements.h>
 #import <wtf/text/TextStream.h>
 
+#define RELEASE_LOG_IF_ALLOWED(channel, fmt, ...) RELEASE_LOG_IF(isAlwaysOnLoggingAllowed(), channel, "%p - WebPage::" fmt, this, ##__VA_ARGS__)
+#define RELEASE_LOG_ERROR_IF_ALLOWED(channel, fmt, ...) RELEASE_LOG_ERROR_IF(isAlwaysOnLoggingAllowed(), channel, "%p - WebPage::" fmt, this, ##__VA_ARGS__)
+
 namespace WebKit {
 using namespace WebCore;
 
@@ -2516,6 +2519,12 @@ static inline bool isAssistableElement(Element& element)
 
 void WebPage::getPositionInformation(const InteractionInformationRequest& request, CompletionHandler<void(InteractionInformationAtPosition&&)>&& reply)
 {
+    // Avoid UIProcess hangs when the WebContent process is stuck on a sync IPC.
+    if (IPC::UnboundedSynchronousIPCScope::hasOngoingUnboundedSyncIPC()) {
+        RELEASE_LOG_ERROR_IF_ALLOWED(Process, "getPositionInformation - Not processing because the process is stuck on unbounded sync IPC");
+        return reply({ });
+    }
+
     m_pendingSynchronousPositionInformationReply = WTFMove(reply);
 
     auto information = positionInformation(request);
@@ -4015,4 +4024,7 @@ void WebPage::requestDocumentEditingContext(DocumentEditingContextRequest reques
 
 } // namespace WebKit
 
+#undef RELEASE_LOG_IF_ALLOWED
+#undef RELEASE_LOG_ERROR_IF_ALLOWED
+
 #endif // PLATFORM(IOS_FAMILY)