- Revision
- 277169
- Author
- [email protected]
- Date
- 2021-05-07 05:05:42 -0700 (Fri, 07 May 2021)
Log Message
[iOS] Safari sometimes hangs underneath `WebKit::UIDelegate::UIClient::createNewPage`
https://bugs.webkit.org/show_bug.cgi?id=225481
rdar://77565282
Reviewed by Chris Dumez.
Consider the scenario where a webpage programmatically opens a window in a new tab on iOS while the keyboard is
visible. In certain configurations, Safari indirectly tells the `WKWebView` containing the original webpage to
`-resignFirstResponder` and then `-becomeFirstResponder` again, while creating a new web view and shuffling
around the view hierarchy under `-webView:createWebViewWithConfiguration:forNavigationAction:windowFeatures:`.
Since the keyboard is up, this causes UIKit to ask for an autocorrection context via `-[WKContentView
requestAutocorrectionContextWithCompletionHandler:]`, which blocks the main thread on a response from the web
process. Of course, since we're handling synchronous IPC, the web process isn't capable of sending a response
back to the UI process, so we deadlock until we exceed the IPC timeout, and then exit the call stack.
Instead, it's possible in this case to recognize that we're already handling a synchronous IPC message, and any
attempts to wait for a response will time out anyways. Instead of waiting for the entire timeout duration, we
can fail eagerly to avoid hanging the process that is waiting.
* Platform/IPC/Connection.cpp:
(IPC::Connection::Connection):
(IPC::Connection::sendMessage):
(IPC::Connection::waitForMessage):
(IPC::Connection::dispatchSyncMessage):
* Platform/IPC/Connection.h:
Add a new counter variable, `m_inDispatchSyncMessageCount`, that is incremented and decremented while we're
in the process of dispatching an incoming sync IPC message. If `m_inDispatchSyncMessageCount` is nonzero and
we have a non-infinite IPC timeout duration, then we should fail sooner inside `Connection::waitForMessage`,
instead of waiting to time out.
Drive-by refactoring: while we're here, let's also move initialization for some of the members on `Connection`
into the class declaration, instead of the constructor.
(IPC::Connection::WTF_GUARDED_BY_LOCK):
* Platform/IPC/Timeout.h:
(IPC::Timeout::isInfinity const):
* WebKit.xcodeproj/project.pbxproj:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (277168 => 277169)
--- trunk/Source/WebKit/ChangeLog 2021-05-07 11:46:48 UTC (rev 277168)
+++ trunk/Source/WebKit/ChangeLog 2021-05-07 12:05:42 UTC (rev 277169)
@@ -1,3 +1,45 @@
+2021-05-07 Wenson Hsieh <[email protected]>
+
+ [iOS] Safari sometimes hangs underneath `WebKit::UIDelegate::UIClient::createNewPage`
+ https://bugs.webkit.org/show_bug.cgi?id=225481
+ rdar://77565282
+
+ Reviewed by Chris Dumez.
+
+ Consider the scenario where a webpage programmatically opens a window in a new tab on iOS while the keyboard is
+ visible. In certain configurations, Safari indirectly tells the `WKWebView` containing the original webpage to
+ `-resignFirstResponder` and then `-becomeFirstResponder` again, while creating a new web view and shuffling
+ around the view hierarchy under `-webView:createWebViewWithConfiguration:forNavigationAction:windowFeatures:`.
+
+ Since the keyboard is up, this causes UIKit to ask for an autocorrection context via `-[WKContentView
+ requestAutocorrectionContextWithCompletionHandler:]`, which blocks the main thread on a response from the web
+ process. Of course, since we're handling synchronous IPC, the web process isn't capable of sending a response
+ back to the UI process, so we deadlock until we exceed the IPC timeout, and then exit the call stack.
+
+ Instead, it's possible in this case to recognize that we're already handling a synchronous IPC message, and any
+ attempts to wait for a response will time out anyways. Instead of waiting for the entire timeout duration, we
+ can fail eagerly to avoid hanging the process that is waiting.
+
+ * Platform/IPC/Connection.cpp:
+ (IPC::Connection::Connection):
+ (IPC::Connection::sendMessage):
+ (IPC::Connection::waitForMessage):
+ (IPC::Connection::dispatchSyncMessage):
+ * Platform/IPC/Connection.h:
+
+ Add a new counter variable, `m_inDispatchSyncMessageCount`, that is incremented and decremented while we're
+ in the process of dispatching an incoming sync IPC message. If `m_inDispatchSyncMessageCount` is nonzero and
+ we have a non-infinite IPC timeout duration, then we should fail sooner inside `Connection::waitForMessage`,
+ instead of waiting to time out.
+
+ Drive-by refactoring: while we're here, let's also move initialization for some of the members on `Connection`
+ into the class declaration, instead of the constructor.
+
+ (IPC::Connection::WTF_GUARDED_BY_LOCK):
+ * Platform/IPC/Timeout.h:
+ (IPC::Timeout::isInfinity const):
+ * WebKit.xcodeproj/project.pbxproj:
+
2021-05-06 Brent Fulgham <[email protected]>
[iOS] Allow file-read* and file-write-data for /dev/null and /dev/zero
Modified: trunk/Source/WebKit/Platform/IPC/Connection.cpp (277168 => 277169)
--- trunk/Source/WebKit/Platform/IPC/Connection.cpp 2021-05-07 11:46:48 UTC (rev 277168)
+++ trunk/Source/WebKit/Platform/IPC/Connection.cpp 2021-05-07 12:05:42 UTC (rev 277169)
@@ -36,6 +36,7 @@
#include <wtf/NeverDestroyed.h>
#include <wtf/ObjectIdentifier.h>
#include <wtf/RunLoop.h>
+#include <wtf/Scope.h>
#include <wtf/text/WTFString.h>
#include <wtf/threads/BinarySemaphore.h>
@@ -292,17 +293,7 @@
: m_client(client)
, m_uniqueID(UniqueID::generate())
, m_isServer(isServer)
- , m_onlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage(false)
- , m_shouldExitOnSyncMessageSendFailure(false)
- , m_didCloseOnConnectionWorkQueueCallback(0)
- , m_isConnected(false)
, m_connectionQueue(WorkQueue::create("com.apple.IPC.ReceiveQueue"))
- , m_inSendSyncCount(0)
- , m_inDispatchMessageCount(0)
- , m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount(0)
- , m_didReceiveInvalidMessage(false)
- , m_shouldWaitForSyncReplies(true)
- , m_shouldWaitForMessages(true)
{
ASSERT(RunLoop::isMain());
allConnections().add(m_uniqueID, this);
@@ -489,9 +480,6 @@
else if (sendOptions.contains(SendOption::DispatchMessageEvenWhenWaitingForUnboundedSyncReply))
encoder->setShouldDispatchMessageWhenWaitingForSyncReply(ShouldDispatchWhenWaitingForSyncReply::YesDuringUnboundedIPC);
-#if ENABLE(IPC_TESTING_API)
-#endif
-
{
auto locker = holdLock(m_outgoingMessagesMutex);
m_outgoingMessages.append(WTFMove(encoder));
@@ -583,6 +571,12 @@
break;
}
+ if (UNLIKELY(m_inDispatchSyncMessageCount && !timeout.isInfinity())) {
+ RELEASE_LOG_ERROR(IPC, "Connection::waitForMessage(%{public}s): Exiting immediately, since we're handling a sync message already", description(messageName));
+ m_waitingForMessage = nullptr;
+ break;
+ }
+
if (m_waitingForMessage->decoder) {
auto decoder = WTFMove(m_waitingForMessage->decoder);
m_waitingForMessage = nullptr;
@@ -935,6 +929,7 @@
void Connection::dispatchSyncMessage(Decoder& decoder)
{
+ ASSERT(isMainRunLoop());
ASSERT(decoder.isSyncMessage());
SyncRequestID syncRequestID;
@@ -943,6 +938,12 @@
return;
}
+ ++m_inDispatchSyncMessageCount;
+ auto decrementSyncMessageCount = makeScopeExit([&] {
+ ASSERT(m_inDispatchSyncMessageCount);
+ --m_inDispatchSyncMessageCount;
+ });
+
auto replyEncoder = makeUniqueRef<Encoder>(MessageName::SyncMessageReply, syncRequestID.toUInt64());
bool wasHandled = false;
Modified: trunk/Source/WebKit/Platform/IPC/Connection.h (277168 => 277169)
--- trunk/Source/WebKit/Platform/IPC/Connection.h 2021-05-07 11:46:48 UTC (rev 277168)
+++ trunk/Source/WebKit/Platform/IPC/Connection.h 2021-05-07 12:05:42 UTC (rev 277169)
@@ -388,20 +388,21 @@
bool m_isServer;
std::atomic<bool> m_isValid { true };
- bool m_onlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage;
- bool m_shouldExitOnSyncMessageSendFailure;
- DidCloseOnConnectionWorkQueueCallback m_didCloseOnConnectionWorkQueueCallback;
+ bool m_onlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage { false };
+ bool m_shouldExitOnSyncMessageSendFailure { false };
+ DidCloseOnConnectionWorkQueueCallback m_didCloseOnConnectionWorkQueueCallback { nullptr };
- bool m_isConnected;
+ bool m_isConnected { false };
Ref<WorkQueue> m_connectionQueue;
- unsigned m_inSendSyncCount;
- unsigned m_inDispatchMessageCount;
- unsigned m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount;
+ unsigned m_inSendSyncCount { 0 };
+ unsigned m_inDispatchMessageCount { 0 };
+ unsigned m_inDispatchSyncMessageCount { 0 };
+ unsigned m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount { 0 };
unsigned m_inDispatchMessageMarkedToUseFullySynchronousModeForTesting { 0 };
bool m_fullySynchronousModeIsAllowedForTesting { false };
bool m_ignoreTimeoutsForTesting { false };
- bool m_didReceiveInvalidMessage;
+ bool m_didReceiveInvalidMessage { false };
// Incoming messages.
Lock m_incomingMessagesMutex;
@@ -422,8 +423,8 @@
class SyncMessageState;
Lock m_syncReplyStateMutex;
- bool m_shouldWaitForSyncReplies;
- bool m_shouldWaitForMessages WTF_GUARDED_BY_LOCK(m_waitForMessageMutex);
+ bool m_shouldWaitForSyncReplies { true };
+ bool m_shouldWaitForMessages WTF_GUARDED_BY_LOCK(m_waitForMessageMutex) { true };
struct PendingSyncReply;
Vector<PendingSyncReply> m_pendingSyncReplies;
Modified: trunk/Source/WebKit/Platform/IPC/Timeout.h (277168 => 277169)
--- trunk/Source/WebKit/Platform/IPC/Timeout.h 2021-05-07 11:46:48 UTC (rev 277168)
+++ trunk/Source/WebKit/Platform/IPC/Timeout.h 2021-05-07 12:05:42 UTC (rev 277169)
@@ -37,16 +37,20 @@
: m_deadline(MonotonicTime::now() + timeDelta)
{
}
+
static constexpr Timeout infinity() { return Timeout { }; }
+ bool isInfinity() const { return std::isinf(m_deadline); }
static Timeout now() { return 0_s; }
Seconds secondsUntilDeadline() const { return std::max(m_deadline - MonotonicTime::now(), 0_s ); }
constexpr MonotonicTime deadline() const { return m_deadline; }
bool didTimeOut() const { return MonotonicTime::now() >= m_deadline; }
+
private:
explicit constexpr Timeout()
: m_deadline(MonotonicTime::infinity())
{
}
+
MonotonicTime m_deadline;
};
Modified: trunk/Source/WebKit/WebKit.xcodeproj/project.pbxproj (277168 => 277169)
--- trunk/Source/WebKit/WebKit.xcodeproj/project.pbxproj 2021-05-07 11:46:48 UTC (rev 277168)
+++ trunk/Source/WebKit/WebKit.xcodeproj/project.pbxproj 2021-05-07 12:05:42 UTC (rev 277169)
@@ -2026,6 +2026,7 @@
F44815642387820000982657 /* WKDeferringGestureRecognizer.h in Headers */ = {isa = PBXBuildFile; fileRef = F44815622387820000982657 /* WKDeferringGestureRecognizer.h */; };
F44DFEB21E9E752F0038D196 /* WebIconUtilities.h in Headers */ = {isa = PBXBuildFile; fileRef = F44DFEB01E9E752F0038D196 /* WebIconUtilities.h */; };
F4660BC225DEF08100E86598 /* PasteboardAccessIntent.h in Headers */ = {isa = PBXBuildFile; fileRef = F4660BC125DEF08100E86598 /* PasteboardAccessIntent.h */; };
+ F48570A32644BEC500C05F71 /* Timeout.h in Headers */ = {isa = PBXBuildFile; fileRef = F48570A22644BEC400C05F71 /* Timeout.h */; };
F48D2A8521583A7E00C6752B /* AppKitSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = F48D2A8421583A0200C6752B /* AppKitSPI.h */; };
F496A4311F58A272004C1757 /* DragDropInteractionState.h in Headers */ = {isa = PBXBuildFile; fileRef = F496A42F1F58A272004C1757 /* DragDropInteractionState.h */; };
F4975CF22624B80A003C626E /* WKImageExtractionPreviewController.h in Headers */ = {isa = PBXBuildFile; fileRef = F4975CF12624B80A003C626E /* WKImageExtractionPreviewController.h */; };
@@ -5975,6 +5976,7 @@
F44DFEB01E9E752F0038D196 /* WebIconUtilities.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = WebIconUtilities.h; path = ios/WebIconUtilities.h; sourceTree = "<group>"; };
F44DFEB11E9E752F0038D196 /* WebIconUtilities.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = WebIconUtilities.mm; path = ios/WebIconUtilities.mm; sourceTree = "<group>"; };
F4660BC125DEF08100E86598 /* PasteboardAccessIntent.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = PasteboardAccessIntent.h; sourceTree = "<group>"; };
+ F48570A22644BEC400C05F71 /* Timeout.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Timeout.h; sourceTree = "<group>"; };
F48D2A8421583A0200C6752B /* AppKitSPI.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = AppKitSPI.h; sourceTree = "<group>"; };
F496A42F1F58A272004C1757 /* DragDropInteractionState.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = DragDropInteractionState.h; path = ios/DragDropInteractionState.h; sourceTree = "<group>"; };
F496A4301F58A272004C1757 /* DragDropInteractionState.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; name = DragDropInteractionState.mm; path = ios/DragDropInteractionState.mm; sourceTree = "<group>"; };
@@ -7143,6 +7145,7 @@
7B73123825CC8524003B2796 /* StreamServerConnection.h */,
1AE00D6918327C1200087DD7 /* StringReference.cpp */,
1AE00D6A18327C1200087DD7 /* StringReference.h */,
+ F48570A22644BEC400C05F71 /* Timeout.h */,
);
path = IPC;
sourceTree = "<group>";
@@ -12311,6 +12314,7 @@
CE1A0BD71A48E6C60054EF74 /* TextInputSPI.h in Headers */,
1AAF263914687C39004A1E8A /* TiledCoreAnimationDrawingArea.h in Headers */,
1AF05D8714688348008B1E81 /* TiledCoreAnimationDrawingAreaProxy.h in Headers */,
+ F48570A32644BEC500C05F71 /* Timeout.h in Headers */,
2F8336861FA139DF00C6E080 /* TouchBarMenuData.h in Headers */,
46BEB6E322FBB21A00269867 /* TransientLocalStorageNamespace.h in Headers */,
57EB2E3A21E1983E00B89CDF /* U2fAuthenticator.h in Headers */,
