Occasional hang under -[UIKeyboardTaskQueue lockWhenReadyForMainThread] when long...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Aug 2019 18:39:32 +0000 (18:39 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Aug 2019 18:39:32 +0000 (18:39 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/editing/selection/ios/tap-during-loupe-gesture-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/ios/tap-during-loupe-gesture.html [new file with mode: 0644]
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

index 21f6b6f..0cf3fb0 100644 (file)
@@ -1,3 +1,17 @@
+2019-08-15  Wenson Hsieh  <wenson_hsieh@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  <pecoraro@apple.com>
 
         Web Inspector: Update Esprima to trunk (minor fixes)
diff --git a/LayoutTests/editing/selection/ios/tap-during-loupe-gesture-expected.txt b/LayoutTests/editing/selection/ios/tap-during-loupe-gesture-expected.txt
new file mode 100644 (file)
index 0000000..c2dc479
--- /dev/null
@@ -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
+
diff --git a/LayoutTests/editing/selection/ios/tap-during-loupe-gesture.html b/LayoutTests/editing/selection/ios/tap-during-loupe-gesture.html
new file mode 100644 (file)
index 0000000..f2bb17a
--- /dev/null
@@ -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="../../../resources/js-test.js"></script>
+<script src="../../../resources/ui-helper.js"></script>
+<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>&nbsp;</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>
index 4a513b2..780ec25 100644 (file)
@@ -1,3 +1,40 @@
+2019-08-15  Wenson Hsieh  <wenson_hsieh@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-queue@webkit.org>
 
         Unreviewed, rolling out r248440.
index 127e352..f8b6477 100644 (file)
@@ -355,6 +355,7 @@ struct WKAutoCorrectionData {
 
     BOOL _hasSetUpInteractions;
     NSUInteger _ignoreSelectionCommandFadeCount;
+    NSInteger _suppressNonEditableSingleTapTextInteractionCount;
     CompletionHandler<void(WebCore::DOMPasteAccessResponse)> _domPasteRequestHandler;
     BlockPtr<void(UIWKAutocorrectionContext *)> _pendingAutocorrectionContextHandler;
 
index f2c1c0b..ceafd16 100644 (file)
@@ -2250,9 +2250,31 @@ static inline bool isSamePair(UIGestureRecognizer *a, UIGestureRecognizer *b, UI
     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])
@@ -3766,12 +3788,14 @@ static void selectionChangedWithTouch(WKContentView *view, const WebCore::IntPoi
 - (void)selectTextWithGranularity:(UITextGranularity)granularity atPoint:(CGPoint)point completionHandler:(void (^)(void))completionHandler
 {
     _usingGestureForSelection = YES;
+    ++_suppressNonEditableSingleTapTextInteractionCount;
     UIWKSelectionCompletionHandler selectionHandler = [completionHandler copy];
     RetainPtr<WKContentView> view = self;
 
     _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 @@ static void selectionChangedWithTouch(WKContentView *view, const WebCore::IntPoi
 {
     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;
     });
 }