[iOS] Keyboard input is severely delayed after switching away from unresponsive tab
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jan 2020 22:37:35 +0000 (22:37 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jan 2020 22:37:35 +0000 (22:37 +0000)
https://bugs.webkit.org/show_bug.cgi?id=206242
<rdar://problem/57132891>

Reviewed by Tim Horton.

Source/WebKit:

UIKit delivers key events to WKWebView using asynchronous SPI (-handleKeyWebEvent:withCompletionHandler:). The
completion handler is invoked when the web page has processed the event, and determines whether to proceed with
default behavior via the `BOOL handled` argument. Using UIKeyboardImpl's UIKeyboardTaskQueue, UIKit appends
subsequent key events to a queue, to be processed by the current first responder after the current key event
has been handled.

In the scenario where the web page is completely unresponsive, this means key events that come after an event
that has been dispatched to the unresponsive page will be stuck in the task queue; this manifests in behaviors
similar to the one in this bug:

- Using a hardware keyboard, press any key in an unresponsive page in Safari.
- Press CMD+T (to create a new tab and focus the unified field) or CMT+L (to just focus the unified field).
- Try to type in the unified field.

The result is that no characters are inserted in the unified field, because the hardware key events are stuck in
UIKeyboardTaskQueue waiting for the unresponsive page to finish handling the current key event. To fix this, we
introduce a mechanism for invoking the key event handler on WKContentView before the web page has actually
finished processing the event, but only in the case where the web view has resigned first responder (and
therefore won't receive subsequent key events anyways).

Tests:  KeyboardInputTests.ResigningFirstResponderCancelsKeyEvents
        KeyboardInputTests.WaitForKeyEventHandlerInFirstResponder

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::hasQueuedKeyEvent const):
(WebKit::WebPageProxy::firstQueuedKeyEvent const):
* UIProcess/WebPageProxy.h:
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView resignFirstResponderForWebView]):

After the content view has resigned first responder with a pending key event handler (and if it did not
immediately become first responder again in the same runloop), then invoke the key event handler early, passing
in `YES` for `handled` to prevent any default actions such as text insertion from being dispatched to the view.

Tools:

Add a couple of new API tests: (1) verify that the key event completion handler can still be invoked in an
unresponsive web view after resigning first responder, and (2) verify that we'll try to wait for the current
key event to be processed in a web view, if it remains the first responder.

* TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm

index 1d73b57..44d8361 100644 (file)
@@ -1,3 +1,45 @@
+2020-01-14  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Keyboard input is severely delayed after switching away from unresponsive tab
+        https://bugs.webkit.org/show_bug.cgi?id=206242
+        <rdar://problem/57132891>
+
+        Reviewed by Tim Horton.
+
+        UIKit delivers key events to WKWebView using asynchronous SPI (-handleKeyWebEvent:withCompletionHandler:). The
+        completion handler is invoked when the web page has processed the event, and determines whether to proceed with
+        default behavior via the `BOOL handled` argument. Using UIKeyboardImpl's UIKeyboardTaskQueue, UIKit appends
+        subsequent key events to a queue, to be processed by the current first responder after the current key event
+        has been handled.
+
+        In the scenario where the web page is completely unresponsive, this means key events that come after an event
+        that has been dispatched to the unresponsive page will be stuck in the task queue; this manifests in behaviors
+        similar to the one in this bug:
+
+        - Using a hardware keyboard, press any key in an unresponsive page in Safari.
+        - Press CMD+T (to create a new tab and focus the unified field) or CMT+L (to just focus the unified field).
+        - Try to type in the unified field.
+
+        The result is that no characters are inserted in the unified field, because the hardware key events are stuck in
+        UIKeyboardTaskQueue waiting for the unresponsive page to finish handling the current key event. To fix this, we
+        introduce a mechanism for invoking the key event handler on WKContentView before the web page has actually
+        finished processing the event, but only in the case where the web view has resigned first responder (and
+        therefore won't receive subsequent key events anyways).
+
+        Tests:  KeyboardInputTests.ResigningFirstResponderCancelsKeyEvents
+                KeyboardInputTests.WaitForKeyEventHandlerInFirstResponder
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::hasQueuedKeyEvent const):
+        (WebKit::WebPageProxy::firstQueuedKeyEvent const):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView resignFirstResponderForWebView]):
+
+        After the content view has resigned first responder with a pending key event handler (and if it did not
+        immediately become first responder again in the same runloop), then invoke the key event handler early, passing
+        in `YES` for `handled` to prevent any default actions such as text insertion from being dispatched to the view.
+
 2020-01-14  Jiewen Tan  <jiewen_tan@apple.com>
 
         Unreviewed, a build fix after r254533
index d919846..b39a6fe 100644 (file)
@@ -2627,6 +2627,16 @@ bool WebPageProxy::shouldProcessWheelEventNow(const WebWheelEvent& event) const
     return false;
 }
 
+bool WebPageProxy::hasQueuedKeyEvent() const
+{
+    return !m_keyEventQueue.isEmpty();
+}
+
+const NativeWebKeyboardEvent& WebPageProxy::firstQueuedKeyEvent() const
+{
+    return m_keyEventQueue.first();
+}
+
 void WebPageProxy::handleKeyboardEvent(const NativeWebKeyboardEvent& event)
 {
     if (!hasRunningProcess())
index 1203369..7fecfcd 100644 (file)
@@ -1663,6 +1663,9 @@ public:
 
     bool isHandlingPreventableTouchStart() const { return m_handlingPreventableTouchStartCount; }
 
+    bool hasQueuedKeyEvent() const;
+    const NativeWebKeyboardEvent& firstQueuedKeyEvent() const;
+
 private:
     WebPageProxy(PageClient&, WebProcessProxy&, Ref<API::PageConfiguration>&&);
     void platformInitialize();
index b3c744c..13bfcef 100644 (file)
@@ -1386,6 +1386,21 @@ typedef NS_ENUM(NSInteger, EndEditingReason) {
     if (superDidResign) {
         [self _handleDOMPasteRequestWithResult:WebCore::DOMPasteAccessResponse::DeniedForGesture];
         _page->activityStateDidChange(WebCore::ActivityState::IsFocused);
+
+        if (_keyWebEventHandler) {
+            dispatch_async(dispatch_get_main_queue(), [weakHandler = WeakObjCPtr<id>(_keyWebEventHandler.get()), weakSelf = WeakObjCPtr<WKContentView>(self)] {
+                if (!weakSelf || !weakHandler)
+                    return;
+
+                auto strongSelf = weakSelf.get();
+                if ([strongSelf isFirstResponder] || weakHandler.get().get() != strongSelf->_keyWebEventHandler.get())
+                    return;
+
+                auto keyEventHandler = std::exchange(strongSelf->_keyWebEventHandler, nil);
+                ASSERT(strongSelf->_page->hasQueuedKeyEvent());
+                keyEventHandler(strongSelf->_page->firstQueuedKeyEvent().nativeEvent(), YES);
+            });
+        }
     }
 
     return superDidResign;
index d24e85e..ef5c12b 100644 (file)
@@ -1,3 +1,17 @@
+2020-01-14  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Keyboard input is severely delayed after switching away from unresponsive tab
+        https://bugs.webkit.org/show_bug.cgi?id=206242
+        <rdar://problem/57132891>
+
+        Reviewed by Tim Horton.
+
+        Add a couple of new API tests: (1) verify that the key event completion handler can still be invoked in an
+        unresponsive web view after resigning first responder, and (2) verify that we'll try to wait for the current
+        key event to be processed in a web view, if it remains the first responder.
+
+        * TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:
+
 2020-01-10  Jiewen Tan  <jiewen_tan@apple.com>
 
         [WebAuthn] Implement SPI to tell UI clients to select assertion responses
index 45425b2..20d2aa1 100644 (file)
@@ -323,6 +323,46 @@ TEST(KeyboardInputTests, CanHandleKeyEventInCompletionHandler)
     EXPECT_WK_STREQ("a", [webView stringByEvaluatingJavaScript:@"document.querySelector('input').value"]);
 }
 
+TEST(KeyboardInputTests, ResigningFirstResponderCancelsKeyEvents)
+{
+    auto webView = webViewWithAutofocusedInput();
+    auto contentView = [webView textInputContentView];
+    auto keyDownEvent = adoptNS([[WebEvent alloc] initWithKeyEventType:WebEventKeyDown timeStamp:CFAbsoluteTimeGetCurrent() characters:@"a" charactersIgnoringModifiers:@"a" modifiers:0 isRepeating:NO withFlags:0 withInputManagerHint:nil keyCode:0 isTabKey:NO]);
+
+    [webView becomeFirstResponder];
+    [webView evaluateJavaScript:@"while(1);" completionHandler:nil];
+
+    bool doneWaiting = false;
+    [contentView handleKeyWebEvent:keyDownEvent.get() withCompletionHandler:[&] (WebEvent *event, BOOL handled) {
+        EXPECT_TRUE([event isEqual:keyDownEvent.get()]);
+        EXPECT_TRUE(handled);
+        doneWaiting = true;
+    }];
+
+    EXPECT_TRUE([webView resignFirstResponder]);
+    TestWebKitAPI::Util::run(&doneWaiting);
+}
+
+TEST(KeyboardInputTests, WaitForKeyEventHandlerInFirstResponder)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
+    auto contentView = [webView textInputContentView];
+    auto keyDownEvent = adoptNS([[WebEvent alloc] initWithKeyEventType:WebEventKeyDown timeStamp:CFAbsoluteTimeGetCurrent() characters:@"a" charactersIgnoringModifiers:@"a" modifiers:0 isRepeating:NO withFlags:0 withInputManagerHint:nil keyCode:0 isTabKey:NO]);
+
+    [webView becomeFirstResponder];
+    [webView synchronouslyLoadHTMLString:@"<body></body>"];
+    [webView evaluateJavaScript:@"start = Date.now(); while(Date.now() - start < 500);" completionHandler:nil];
+
+    bool doneWaiting = false;
+    [contentView handleKeyWebEvent:keyDownEvent.get() withCompletionHandler:[&] (WebEvent *event, BOOL handled) {
+        EXPECT_TRUE([event isEqual:keyDownEvent.get()]);
+        EXPECT_FALSE(handled);
+        doneWaiting = true;
+    }];
+
+    TestWebKitAPI::Util::run(&doneWaiting);
+}
+
 TEST(KeyboardInputTests, CaretSelectionRectAfterRestoringFirstResponderWithRetainActiveFocusedState)
 {
     // This difference in caret width is due to the fact that we don't zoom in to the input field on iPad, but do on iPhone.