Log Message
Occasional hang under -[UIKeyboardTaskQueue lockWhenReadyForMainThread] when long-pressing non-editable text https://bugs.webkit.org/show_bug.cgi?id=200731 <rdar://problem/54315371>
Reviewed by Tim Horton. Source/WebKit: When handling a single tap in non-editable content, keyboards logic in UIKit may attempt to wait for all pending tasks in UIKeyboardTaskQueue to finish executing (e.g. by calling -waitUntilAllTasksAreFinished]). If the task queue has a pending task at this moment - for example, a text selection update that is waiting for a response from the web process - this will result in a permanent deadlock, since the main thread will be blocked, and therefore cannot receive any IPC communication from the web process. One way to trigger this is to activate both the loupe gesture and non-editable text tap gesture simultaneously, by tapping in a non-editable part of the web page, while an ongoing loupe gesture is driving selection updates (see the layout test for more details). To avoid getting into this scenario, prevent the text tap gesture recognizer from firing in a few edge cases that could lead to hangs under keyboard code in UIKit. See comments below. Test: editing/selection/ios/tap-during-loupe-gesture.html * UIProcess/ios/WKContentViewInteraction.h: * UIProcess/ios/WKContentViewInteraction.mm: (-[WKContentView textInteractionGesture:shouldBeginAtPoint:]): Don't allow the text tap gesture recognizer to fire if the user is actively modifying the text selection using the loupe gesture, or if there's other pending selection change updates that are pending responses from the web content process. (-[WKContentView selectTextWithGranularity:atPoint:completionHandler:]): (-[WKContentView updateSelectionWithExtentPoint:withBoundary:completionHandler:]): Increment and decrement _suppressNonEditableSingleTapTextInteractionCount while handling these selection updates. LayoutTests: Add a layout test to verify that tapping the page while handling a text loupe gesture doesn't cause the UI process to hang indefinitely. * editing/selection/ios/tap-during-loupe-gesture-expected.txt: Added. * editing/selection/ios/tap-during-loupe-gesture.html: Added.
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/Source/WebKit/ChangeLog
- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (248732 => 248733)
--- trunk/LayoutTests/ChangeLog 2019-08-15 18:14:45 UTC (rev 248732)
+++ trunk/LayoutTests/ChangeLog 2019-08-15 18:39:32 UTC (rev 248733)
@@ -1,3 +1,17 @@
+2019-08-15 Wenson Hsieh <wenson_hs...@apple.com>
+
+ Occasional hang under -[UIKeyboardTaskQueue lockWhenReadyForMainThread] when long-pressing non-editable text
+ https://bugs.webkit.org/show_bug.cgi?id=200731
+ <rdar://problem/54315371>
+
+ Reviewed by Tim Horton.
+
+ Add a layout test to verify that tapping the page while handling a text loupe gesture doesn't cause the UI
+ process to hang indefinitely.
+
+ * editing/selection/ios/tap-during-loupe-gesture-expected.txt: Added.
+ * editing/selection/ios/tap-during-loupe-gesture.html: Added.
+
2019-08-15 Joseph Pecoraro <pecor...@apple.com>
Web Inspector: Update Esprima to trunk (minor fixes)
Added: trunk/LayoutTests/editing/selection/ios/tap-during-loupe-gesture-expected.txt (0 => 248733)
--- trunk/LayoutTests/editing/selection/ios/tap-during-loupe-gesture-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/tap-during-loupe-gesture-expected.txt 2019-08-15 18:39:32 UTC (rev 248733)
@@ -0,0 +1,10 @@
+
+This test verifies that the UI process doesn't hang if the user taps during a loupe gesture if focus is inside editable content in a same-origin child frame. To run the test manually, long press and drag on the page with one finger, and tap the page several times with the other finger at the same time. The UI process should not permanently hang as a result.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+PASS Did not hang.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/editing/selection/ios/tap-during-loupe-gesture.html (0 => 248733)
--- trunk/LayoutTests/editing/selection/ios/tap-during-loupe-gesture.html (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/tap-during-loupe-gesture.html 2019-08-15 18:39:32 UTC (rev 248733)
@@ -0,0 +1,255 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+<script src=""
+<script src=""
+<html>
+<head>
+ <style>
+ body {
+ width: 100%;
+ height: 100%;
+ font-family: system-ui;
+ font-size: 18px;
+ pointer-events: none;
+ }
+
+ iframe {
+ border: red 2px solid;
+ top: -100px;
+ width: 100px;
+ height: 100px;
+ position: fixed;
+ box-sizing: border-box;
+ }
+
+ .vertical-space {
+ width: 100%;
+ height: 2000px;
+ }
+
+ .text {
+ text-align: center;
+ -webkit-user-select: none;
+ }
+ </style>
+</head>
+<body>
+ <iframe srcdoc="
+ <body>
+ <a> </a>
+ <script>
+ getSelection().setPosition(document.body, 0);
+ document.designMode = 'on';
+ </script>
+ </body>
+ "></iframe>
+ <body id="lorem-body">
+ <div id="description" class="text"></div>
+ <div class="vertical-space"></div>
+ <div id="console"></div>
+ </body>
+ <script>
+ jsTestIsAsync = true;
+
+ description("This test verifies that the UI process doesn't hang if the user taps during a loupe gesture if focus is inside editable content in a same-origin child frame. To run the test manually, long press and drag on the page with one finger, and tap the page several times with the other finger at the same time. The UI process should not permanently hang as a result.");
+
+ function performLoupeGestureAndSingleTap()
+ {
+ return new Promise(resolve => {
+ testRunner.runUIScript(`
+ (function() {
+ const eventStream = {
+ events : [
+ {
+ // First finger down.
+ interpolate : "linear",
+ timestep: 0.016,
+ coordinateSpace : "content",
+ startEvent : {
+ inputType : "hand",
+ timeOffset : 0,
+ touches : [
+ {
+ inputType : "finger",
+ phase : "began",
+ id : 1,
+ x : 100,
+ y : 100,
+ pressure : 0
+ }
+ ]
+ },
+ endEvent : {
+ inputType : "hand",
+ timeOffset : 0.5,
+ touches : [
+ {
+ inputType : "finger",
+ phase : "moved",
+ id : 1,
+ x : 100,
+ y : 100,
+ pressure : 0
+ }
+ ]
+ }
+ },
+ {
+ // First finger drag.
+ interpolate : "linear",
+ timestep: 0.016,
+ coordinateSpace : "content",
+ startEvent : {
+ inputType : "hand",
+ timeOffset : 0.5,
+ touches : [
+ {
+ inputType : "finger",
+ phase : "moved",
+ id : 1,
+ x : 100,
+ y : 100,
+ pressure : 0
+ }
+ ]
+ },
+ endEvent : {
+ inputType : "hand",
+ timeOffset : 1,
+ touches : [
+ {
+ inputType : "finger",
+ phase : "moved",
+ id : 1,
+ x : 100,
+ y : 450,
+ pressure : 0
+ }
+ ]
+ }
+ },
+ {
+ // Second finger down.
+ interpolate : "linear",
+ timestep: 0.016,
+ coordinateSpace : "content",
+ startEvent : {
+ inputType : "hand",
+ timeOffset : 1,
+ touches : [
+ {
+ inputType : "finger",
+ phase : "began",
+ id : 2,
+ x : 200,
+ y : 200,
+ pressure : 0
+ }
+ ]
+ },
+ endEvent : {
+ inputType : "hand",
+ timeOffset : 1,
+ touches : [
+ {
+ inputType : "finger",
+ phase : "stationary",
+ id : 2,
+ x : 200,
+ y : 200,
+ pressure : 0
+ }
+ ]
+ }
+ },
+ {
+ // Second finger up.
+ interpolate : "linear",
+ timestep: 0.016,
+ coordinateSpace : "content",
+ startEvent : {
+ inputType : "hand",
+ timeOffset : 1,
+ touches : [
+ {
+ inputType : "finger",
+ phase : "stationary",
+ id : 2,
+ x : 200,
+ y : 200,
+ pressure : 0
+ }
+ ]
+ },
+ endEvent : {
+ inputType : "hand",
+ timeOffset : 1,
+ touches : [
+ {
+ inputType : "finger",
+ phase : "ended",
+ id : 2,
+ x : 200,
+ y : 200,
+ pressure : 0
+ }
+ ]
+ }
+ },
+ {
+ // First finger up.
+ interpolate : "linear",
+ timestep: 0.016,
+ coordinateSpace : "content",
+ startEvent : {
+ inputType : "hand",
+ timeOffset : 1,
+ touches : [
+ {
+ inputType : "finger",
+ phase : "moved",
+ id : 1,
+ x : 100,
+ y : 450,
+ pressure : 0
+ }
+ ]
+ },
+ endEvent : {
+ inputType : "hand",
+ timeOffset : 1,
+ touches : [
+ {
+ inputType : "finger",
+ phase : "ended",
+ id : 1,
+ x : 100,
+ y : 450,
+ pressure : 0
+ }
+ ]
+ }
+ }
+ ]
+ };
+
+ uiController.sendEventStream(JSON.stringify(eventStream), () => {
+ uiController.uiScriptComplete();
+ });
+ })();
+ `, resolve);
+ });
+ }
+
+ addEventListener("load", async () => {
+ const subframe = document.querySelector("iframe");
+ subframe.focus();
+ subframe.blur();
+
+ await performLoupeGestureAndSingleTap();
+ testPassed("Did not hang.");
+ finishJSTest();
+ });
+ </script>
+</body>
+</html>
Modified: trunk/Source/WebKit/ChangeLog (248732 => 248733)
--- trunk/Source/WebKit/ChangeLog 2019-08-15 18:14:45 UTC (rev 248732)
+++ trunk/Source/WebKit/ChangeLog 2019-08-15 18:39:32 UTC (rev 248733)
@@ -1,3 +1,40 @@
+2019-08-15 Wenson Hsieh <wenson_hs...@apple.com>
+
+ Occasional hang under -[UIKeyboardTaskQueue lockWhenReadyForMainThread] when long-pressing non-editable text
+ https://bugs.webkit.org/show_bug.cgi?id=200731
+ <rdar://problem/54315371>
+
+ Reviewed by Tim Horton.
+
+ When handling a single tap in non-editable content, keyboards logic in UIKit may attempt to wait for all
+ pending tasks in UIKeyboardTaskQueue to finish executing (e.g. by calling -waitUntilAllTasksAreFinished]). If
+ the task queue has a pending task at this moment - for example, a text selection update that is waiting for a
+ response from the web process - this will result in a permanent deadlock, since the main thread will be blocked,
+ and therefore cannot receive any IPC communication from the web process.
+
+ One way to trigger this is to activate both the loupe gesture and non-editable text tap gesture simultaneously,
+ by tapping in a non-editable part of the web page, while an ongoing loupe gesture is driving selection updates
+ (see the layout test for more details).
+
+ To avoid getting into this scenario, prevent the text tap gesture recognizer from firing in a few edge cases
+ that could lead to hangs under keyboard code in UIKit. See comments below.
+
+ Test: editing/selection/ios/tap-during-loupe-gesture.html
+
+ * UIProcess/ios/WKContentViewInteraction.h:
+ * UIProcess/ios/WKContentViewInteraction.mm:
+ (-[WKContentView textInteractionGesture:shouldBeginAtPoint:]):
+
+ Don't allow the text tap gesture recognizer to fire if the user is actively modifying the text selection using
+ the loupe gesture, or if there's other pending selection change updates that are pending responses from the web
+ content process.
+
+ (-[WKContentView selectTextWithGranularity:atPoint:completionHandler:]):
+ (-[WKContentView updateSelectionWithExtentPoint:withBoundary:completionHandler:]):
+
+ Increment and decrement _suppressNonEditableSingleTapTextInteractionCount while handling these selection
+ updates.
+
2019-08-15 Commit Queue <commit-qu...@webkit.org>
Unreviewed, rolling out r248440.
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h (248732 => 248733)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h 2019-08-15 18:14:45 UTC (rev 248732)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h 2019-08-15 18:39:32 UTC (rev 248733)
@@ -355,6 +355,7 @@
BOOL _hasSetUpInteractions;
NSUInteger _ignoreSelectionCommandFadeCount;
+ NSInteger _suppressNonEditableSingleTapTextInteractionCount;
CompletionHandler<void(WebCore::DOMPasteAccessResponse)> _domPasteRequestHandler;
BlockPtr<void(UIWKAutocorrectionContext *)> _pendingAutocorrectionContextHandler;
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (248732 => 248733)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2019-08-15 18:14:45 UTC (rev 248732)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2019-08-15 18:39:32 UTC (rev 248733)
@@ -2250,10 +2250,32 @@
if (_suppressSelectionAssistantReasons)
return NO;
- // Don't allow double tap text gestures in noneditable content.
- if (!self.isFocusingElement && gesture == UIWKGestureDoubleTap)
- return NO;
+ if (!self.isFocusingElement) {
+ if (gesture == UIWKGestureDoubleTap) {
+ // Don't allow double tap text gestures in noneditable content.
+ return NO;
+ }
+ if (gesture == UIWKGestureOneFingerTap) {
+ ASSERT(_suppressNonEditableSingleTapTextInteractionCount >= 0);
+ if (_suppressNonEditableSingleTapTextInteractionCount > 0)
+ return NO;
+
+ switch ([_textSelectionAssistant loupeGesture].state) {
+ case UIGestureRecognizerStateBegan:
+ case UIGestureRecognizerStateChanged:
+ case UIGestureRecognizerStateEnded: {
+ // Avoid handling one-finger taps while the web process is processing certain selection changes.
+ // This works around a scenario where UIKeyboardImpl blocks the main thread while handling a one-
+ // finger tap, which subsequently prevents the UI process from handling any incoming IPC messages.
+ return NO;
+ }
+ default:
+ break;
+ }
+ }
+ }
+
WebKit::InteractionInformationRequest request(WebCore::roundedIntPoint(point));
if (![self ensurePositionInformationIsUpToDate:request])
return NO;
@@ -3766,6 +3788,7 @@
- (void)selectTextWithGranularity:(UITextGranularity)granularity atPoint:(CGPoint)point completionHandler:(void (^)(void))completionHandler
{
_usingGestureForSelection = YES;
+ ++_suppressNonEditableSingleTapTextInteractionCount;
UIWKSelectionCompletionHandler selectionHandler = [completionHandler copy];
RetainPtr<WKContentView> view = self;
@@ -3772,6 +3795,7 @@
_page->selectTextWithGranularityAtPoint(WebCore::IntPoint(point), toWKTextGranularity(granularity), [self _isInteractingWithFocusedElement], [view, selectionHandler](WebKit::CallbackBase::Error error) {
selectionHandler();
view->_usingGestureForSelection = NO;
+ --view->_suppressNonEditableSingleTapTextInteractionCount;
[selectionHandler release];
});
}
@@ -3800,9 +3824,11 @@
{
UIWKSelectionWithDirectionCompletionHandler selectionHandler = [completionHandler copy];
- _page->updateSelectionWithExtentPointAndBoundary(WebCore::IntPoint(point), toWKTextGranularity(granularity), [self _isInteractingWithFocusedElement], [selectionHandler](bool endIsMoving, WebKit::CallbackBase::Error error) {
+ ++_suppressNonEditableSingleTapTextInteractionCount;
+ _page->updateSelectionWithExtentPointAndBoundary(WebCore::IntPoint(point), toWKTextGranularity(granularity), [self _isInteractingWithFocusedElement], [selectionHandler, protectedSelf = retainPtr(self)] (bool endIsMoving, WebKit::CallbackBase::Error error) {
selectionHandler(endIsMoving);
[selectionHandler release];
+ --protectedSelf->_suppressNonEditableSingleTapTextInteractionCount;
});
}
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes
