Changeset 248733 in webkit


Ignore:
Timestamp:
Aug 15, 2019 11:39:32 AM (5 years ago)
Author:
Wenson Hsieh
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.
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r248732 r248733  
     12019-08-15  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        Occasional hang under -[UIKeyboardTaskQueue lockWhenReadyForMainThread] when long-pressing non-editable text
     4        https://bugs.webkit.org/show_bug.cgi?id=200731
     5        <rdar://problem/54315371>
     6
     7        Reviewed by Tim Horton.
     8
     9        Add a layout test to verify that tapping the page while handling a text loupe gesture doesn't cause the UI
     10        process to hang indefinitely.
     11
     12        * editing/selection/ios/tap-during-loupe-gesture-expected.txt: Added.
     13        * editing/selection/ios/tap-during-loupe-gesture.html: Added.
     14
    1152019-08-15  Joseph Pecoraro  <pecoraro@apple.com>
    216
  • trunk/Source/WebKit/ChangeLog

    r248731 r248733  
     12019-08-15  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        Occasional hang under -[UIKeyboardTaskQueue lockWhenReadyForMainThread] when long-pressing non-editable text
     4        https://bugs.webkit.org/show_bug.cgi?id=200731
     5        <rdar://problem/54315371>
     6
     7        Reviewed by Tim Horton.
     8
     9        When handling a single tap in non-editable content, keyboards logic in UIKit may attempt to wait for all
     10        pending tasks in UIKeyboardTaskQueue to finish executing (e.g. by calling -waitUntilAllTasksAreFinished]). If
     11        the task queue has a pending task at this moment - for example, a text selection update that is waiting for a
     12        response from the web process - this will result in a permanent deadlock, since the main thread will be blocked,
     13        and therefore cannot receive any IPC communication from the web process.
     14
     15        One way to trigger this is to activate both the loupe gesture and non-editable text tap gesture simultaneously,
     16        by tapping in a non-editable part of the web page, while an ongoing loupe gesture is driving selection updates
     17        (see the layout test for more details).
     18
     19        To avoid getting into this scenario, prevent the text tap gesture recognizer from firing in a few edge cases
     20        that could lead to hangs under keyboard code in UIKit. See comments below.
     21
     22        Test: editing/selection/ios/tap-during-loupe-gesture.html
     23
     24        * UIProcess/ios/WKContentViewInteraction.h:
     25        * UIProcess/ios/WKContentViewInteraction.mm:
     26        (-[WKContentView textInteractionGesture:shouldBeginAtPoint:]):
     27
     28        Don't allow the text tap gesture recognizer to fire if the user is actively modifying the text selection using
     29        the loupe gesture, or if there's other pending selection change updates that are pending responses from the web
     30        content process.
     31
     32        (-[WKContentView selectTextWithGranularity:atPoint:completionHandler:]):
     33        (-[WKContentView updateSelectionWithExtentPoint:withBoundary:completionHandler:]):
     34
     35        Increment and decrement _suppressNonEditableSingleTapTextInteractionCount while handling these selection
     36        updates.
     37
    1382019-08-15  Commit Queue  <commit-queue@webkit.org>
    239
  • trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h

    r248481 r248733  
    356356    BOOL _hasSetUpInteractions;
    357357    NSUInteger _ignoreSelectionCommandFadeCount;
     358    NSInteger _suppressNonEditableSingleTapTextInteractionCount;
    358359    CompletionHandler<void(WebCore::DOMPasteAccessResponse)> _domPasteRequestHandler;
    359360    BlockPtr<void(UIWKAutocorrectionContext *)> _pendingAutocorrectionContextHandler;
  • trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

    r248717 r248733  
    22512251        return NO;
    22522252   
    2253     // Don't allow double tap text gestures in noneditable content.
    2254     if (!self.isFocusingElement && gesture == UIWKGestureDoubleTap)
    2255         return NO;
     2253    if (!self.isFocusingElement) {
     2254        if (gesture == UIWKGestureDoubleTap) {
     2255            // Don't allow double tap text gestures in noneditable content.
     2256            return NO;
     2257        }
     2258
     2259        if (gesture == UIWKGestureOneFingerTap) {
     2260            ASSERT(_suppressNonEditableSingleTapTextInteractionCount >= 0);
     2261            if (_suppressNonEditableSingleTapTextInteractionCount > 0)
     2262                return NO;
     2263
     2264            switch ([_textSelectionAssistant loupeGesture].state) {
     2265            case UIGestureRecognizerStateBegan:
     2266            case UIGestureRecognizerStateChanged:
     2267            case UIGestureRecognizerStateEnded: {
     2268                // Avoid handling one-finger taps while the web process is processing certain selection changes.
     2269                // This works around a scenario where UIKeyboardImpl blocks the main thread while handling a one-
     2270                // finger tap, which subsequently prevents the UI process from handling any incoming IPC messages.
     2271                return NO;
     2272            }
     2273            default:
     2274                break;
     2275            }
     2276        }
     2277    }
    22562278
    22572279    WebKit::InteractionInformationRequest request(WebCore::roundedIntPoint(point));
     
    37673789{
    37683790    _usingGestureForSelection = YES;
     3791    ++_suppressNonEditableSingleTapTextInteractionCount;
    37693792    UIWKSelectionCompletionHandler selectionHandler = [completionHandler copy];
    37703793    RetainPtr<WKContentView> view = self;
     
    37733796        selectionHandler();
    37743797        view->_usingGestureForSelection = NO;
     3798        --view->_suppressNonEditableSingleTapTextInteractionCount;
    37753799        [selectionHandler release];
    37763800    });
     
    38013825    UIWKSelectionWithDirectionCompletionHandler selectionHandler = [completionHandler copy];
    38023826   
    3803     _page->updateSelectionWithExtentPointAndBoundary(WebCore::IntPoint(point), toWKTextGranularity(granularity), [self _isInteractingWithFocusedElement], [selectionHandler](bool endIsMoving, WebKit::CallbackBase::Error error) {
     3827    ++_suppressNonEditableSingleTapTextInteractionCount;
     3828    _page->updateSelectionWithExtentPointAndBoundary(WebCore::IntPoint(point), toWKTextGranularity(granularity), [self _isInteractingWithFocusedElement], [selectionHandler, protectedSelf = retainPtr(self)] (bool endIsMoving, WebKit::CallbackBase::Error error) {
    38043829        selectionHandler(endIsMoving);
    38053830        [selectionHandler release];
     3831        --protectedSelf->_suppressNonEditableSingleTapTextInteractionCount;
    38063832    });
    38073833}
Note: See TracChangeset for help on using the changeset viewer.