Title: [254538] trunk
Revision
254538
Author
wenson_hs...@apple.com
Date
2020-01-14 14:37:35 -0800 (Tue, 14 Jan 2020)

Log Message

[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.

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:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (254537 => 254538)


--- trunk/Source/WebKit/ChangeLog	2020-01-14 22:35:09 UTC (rev 254537)
+++ trunk/Source/WebKit/ChangeLog	2020-01-14 22:37:35 UTC (rev 254538)
@@ -1,3 +1,45 @@
+2020-01-14  Wenson Hsieh  <wenson_hs...@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_...@apple.com>
 
         Unreviewed, a build fix after r254533

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (254537 => 254538)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2020-01-14 22:35:09 UTC (rev 254537)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2020-01-14 22:37:35 UTC (rev 254538)
@@ -2627,6 +2627,16 @@
     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())

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (254537 => 254538)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.h	2020-01-14 22:35:09 UTC (rev 254537)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h	2020-01-14 22:37:35 UTC (rev 254538)
@@ -1663,6 +1663,9 @@
 
     bool isHandlingPreventableTouchStart() const { return m_handlingPreventableTouchStartCount; }
 
+    bool hasQueuedKeyEvent() const;
+    const NativeWebKeyboardEvent& firstQueuedKeyEvent() const;
+
 private:
     WebPageProxy(PageClient&, WebProcessProxy&, Ref<API::PageConfiguration>&&);
     void platformInitialize();

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (254537 => 254538)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2020-01-14 22:35:09 UTC (rev 254537)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2020-01-14 22:37:35 UTC (rev 254538)
@@ -1386,6 +1386,21 @@
     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;

Modified: trunk/Tools/ChangeLog (254537 => 254538)


--- trunk/Tools/ChangeLog	2020-01-14 22:35:09 UTC (rev 254537)
+++ trunk/Tools/ChangeLog	2020-01-14 22:37:35 UTC (rev 254538)
@@ -1,3 +1,17 @@
+2020-01-14  Wenson Hsieh  <wenson_hs...@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_...@apple.com>
 
         [WebAuthn] Implement SPI to tell UI clients to select assertion responses

Modified: trunk/Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm (254537 => 254538)


--- trunk/Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm	2020-01-14 22:35:09 UTC (rev 254537)
+++ trunk/Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm	2020-01-14 22:37:35 UTC (rev 254538)
@@ -323,6 +323,46 @@
     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.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to