- Revision
- 241846
- Author
- [email protected]
- Date
- 2019-02-20 15:34:50 -0800 (Wed, 20 Feb 2019)
Log Message
REGRESSION: [ iOS ] Layout Test editing/input/ios/rtl-keyboard-input-on-focus.html is a Timeout
https://bugs.webkit.org/show_bug.cgi?id=194601
<rdar://problem/48080316>
Reviewed by Tim Horton.
Following r241311, if a web view becomes first responder and is then moved offscreen (or obscured, hidden, or in
the case of WebKitTestRunner, its UIWindow loses its status as keyWindow), we end up holding on to the input
view update deferral token indefinitely, waiting for the current focused element to be blurred or refocused.
This also manifests other user-facing bugs, the most common of which is the keyboard occasionally remaining
onscreen after typing a URL in the unified field in MobileSafari and hitting Return, in the case where there is
no autofocused element on the page.
To fix this, when becoming the first responder, additionally install a callback to detect when the page is
finished handling the activity state change, and invalidate the input deferral token then. This retains the
behavior where calling -becomeFirstResponder on the web view while a different view is focused will keep the
keyboard stable, since the focused element message from the web process should be dispatched when handling the
activity state change within the web process.
Of course, the web process may not be responsive at all while the web view is still in the view hierarchy, in
which case we may still end up deferring input view updates indefinitely. In this case, we maintain a separate
watchdog timer with a short delay, after which we unconditionally invalidate the token.
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::installActivityStateChangeCompletionHandler):
Move the implementation of installActivityStateChangeCompletionHandler into cross-platform code.
* UIProcess/WebPageProxy.h:
* UIProcess/ios/WKContentView.mm:
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView cleanupInteraction]):
(-[WKContentView _cancelPreviousResetInputViewDeferralRequest]):
(-[WKContentView _scheduleResetInputViewDeferralAfterBecomingFirstResponder]):
(-[WKContentView _resetInputViewDeferral]):
(-[WKContentView becomeFirstResponderForWebView]):
(-[WKContentView resignFirstResponderForWebView]):
(-[WKContentView _commitPotentialTapFailed]):
(-[WKContentView _didNotHandleTapAsClick:]):
(-[WKContentView _didCompleteSyntheticClick]):
Funnel all existing calls that reset _inputViewDeferralToken to nullptr, such that they go through a helper
method instead that also cancels any scheduled requests to clear the token.
* WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
(WebKit::RemoteLayerTreeDrawingArea::activityStateDidChange):
Respond to all pending callbacks after handling the activity state change.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (241845 => 241846)
--- trunk/Source/WebKit/ChangeLog 2019-02-20 23:30:03 UTC (rev 241845)
+++ trunk/Source/WebKit/ChangeLog 2019-02-20 23:34:50 UTC (rev 241846)
@@ -1,3 +1,55 @@
+2019-02-20 Wenson Hsieh <[email protected]>
+
+ REGRESSION: [ iOS ] Layout Test editing/input/ios/rtl-keyboard-input-on-focus.html is a Timeout
+ https://bugs.webkit.org/show_bug.cgi?id=194601
+ <rdar://problem/48080316>
+
+ Reviewed by Tim Horton.
+
+ Following r241311, if a web view becomes first responder and is then moved offscreen (or obscured, hidden, or in
+ the case of WebKitTestRunner, its UIWindow loses its status as keyWindow), we end up holding on to the input
+ view update deferral token indefinitely, waiting for the current focused element to be blurred or refocused.
+
+ This also manifests other user-facing bugs, the most common of which is the keyboard occasionally remaining
+ onscreen after typing a URL in the unified field in MobileSafari and hitting Return, in the case where there is
+ no autofocused element on the page.
+
+ To fix this, when becoming the first responder, additionally install a callback to detect when the page is
+ finished handling the activity state change, and invalidate the input deferral token then. This retains the
+ behavior where calling -becomeFirstResponder on the web view while a different view is focused will keep the
+ keyboard stable, since the focused element message from the web process should be dispatched when handling the
+ activity state change within the web process.
+
+ Of course, the web process may not be responsive at all while the web view is still in the view hierarchy, in
+ which case we may still end up deferring input view updates indefinitely. In this case, we maintain a separate
+ watchdog timer with a short delay, after which we unconditionally invalidate the token.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::installActivityStateChangeCompletionHandler):
+
+ Move the implementation of installActivityStateChangeCompletionHandler into cross-platform code.
+
+ * UIProcess/WebPageProxy.h:
+ * UIProcess/ios/WKContentView.mm:
+ * UIProcess/ios/WKContentViewInteraction.mm:
+ (-[WKContentView cleanupInteraction]):
+ (-[WKContentView _cancelPreviousResetInputViewDeferralRequest]):
+ (-[WKContentView _scheduleResetInputViewDeferralAfterBecomingFirstResponder]):
+ (-[WKContentView _resetInputViewDeferral]):
+ (-[WKContentView becomeFirstResponderForWebView]):
+ (-[WKContentView resignFirstResponderForWebView]):
+ (-[WKContentView _commitPotentialTapFailed]):
+ (-[WKContentView _didNotHandleTapAsClick:]):
+ (-[WKContentView _didCompleteSyntheticClick]):
+
+ Funnel all existing calls that reset _inputViewDeferralToken to nullptr, such that they go through a helper
+ method instead that also cancels any scheduled requests to clear the token.
+
+ * WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
+ (WebKit::RemoteLayerTreeDrawingArea::activityStateDidChange):
+
+ Respond to all pending callbacks after handling the activity state change.
+
2019-02-20 Chris Dumez <[email protected]>
Regression(PSON) "Reload without content extensions" does not work when the main resource is blocked
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (241845 => 241846)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-02-20 23:30:03 UTC (rev 241845)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-02-20 23:34:50 UTC (rev 241846)
@@ -8004,20 +8004,6 @@
return pageClient().immediateActionAnimationControllerForHitTestResult(hitTestResult, type, userData);
}
-void WebPageProxy::installActivityStateChangeCompletionHandler(WTF::Function<void ()>&& completionHandler)
-{
- if (!isValid()) {
- completionHandler();
- return;
- }
-
- auto voidCallback = VoidCallback::create([completionHandler = WTFMove(completionHandler)] (CallbackBase::Error) {
- completionHandler();
- }, m_process->throttler().backgroundActivityToken());
- auto callbackID = m_callbacks.put(WTFMove(voidCallback));
- m_nextActivityStateChangeCallbacks.append(callbackID);
-}
-
void WebPageProxy::handleAcceptedCandidate(WebCore::TextCheckingResult acceptedCandidate)
{
m_process->send(Messages::WebPage::HandleAcceptedCandidate(acceptedCandidate), m_pageID);
@@ -8052,6 +8038,20 @@
#endif
+void WebPageProxy::installActivityStateChangeCompletionHandler(Function<void()>&& completionHandler)
+{
+ if (!isValid()) {
+ completionHandler();
+ return;
+ }
+
+ auto voidCallback = VoidCallback::create([completionHandler = WTFMove(completionHandler)] (auto) {
+ completionHandler();
+ }, m_process->throttler().backgroundActivityToken());
+ auto callbackID = m_callbacks.put(WTFMove(voidCallback));
+ m_nextActivityStateChangeCallbacks.append(callbackID);
+}
+
void WebPageProxy::imageOrMediaDocumentSizeChanged(const WebCore::IntSize& newSize)
{
m_uiClient->imageOrMediaDocumentSizeChanged(newSize);
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (241845 => 241846)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.h 2019-02-20 23:30:03 UTC (rev 241845)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h 2019-02-20 23:34:50 UTC (rev 241846)
@@ -1288,8 +1288,6 @@
NSObject *immediateActionAnimationControllerForHitTestResult(RefPtr<API::HitTestResult>, uint64_t, RefPtr<API::Object>);
- void installActivityStateChangeCompletionHandler(WTF::Function<void ()>&&);
-
void handleAcceptedCandidate(WebCore::TextCheckingResult);
void didHandleAcceptedCandidate();
@@ -1297,6 +1295,8 @@
void setFooterBannerHeightForTesting(int);
#endif
+ void installActivityStateChangeCompletionHandler(Function<void()>&&);
+
#if USE(UNIFIED_TEXT_CHECKING)
void checkTextOfParagraph(const String& text, OptionSet<WebCore::TextCheckingType> checkingTypes, int32_t insertionPoint, Vector<WebCore::TextCheckingResult>& results);
#endif
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (241845 => 241846)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2019-02-20 23:30:03 UTC (rev 241845)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2019-02-20 23:34:50 UTC (rev 241846)
@@ -868,7 +868,7 @@
}
#endif
- _inputViewUpdateDeferrer = nullptr;
+ [self _resetInputViewDeferral];
_focusedElementInformation = { };
[_keyboardScrollingAnimator invalidate];
@@ -1072,6 +1072,25 @@
#endif
}
+- (void)_cancelPreviousResetInputViewDeferralRequest
+{
+ [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(_resetInputViewDeferral) object:nil];
+}
+
+- (void)_scheduleResetInputViewDeferralAfterBecomingFirstResponder
+{
+ [self _cancelPreviousResetInputViewDeferralRequest];
+
+ const NSTimeInterval inputViewDeferralWatchdogTimerDuration = 0.5;
+ [self performSelector:@selector(_resetInputViewDeferral) withObject:self afterDelay:inputViewDeferralWatchdogTimerDuration];
+}
+
+- (void)_resetInputViewDeferral
+{
+ [self _cancelPreviousResetInputViewDeferralRequest];
+ _inputViewUpdateDeferrer = nullptr;
+}
+
- (BOOL)canBecomeFirstResponder
{
return _becomingFirstResponder;
@@ -1106,12 +1125,22 @@
}
if (didBecomeFirstResponder) {
+ _page->installActivityStateChangeCompletionHandler([weakSelf = WeakObjCPtr<WKContentView>(self)] {
+ if (!weakSelf)
+ return;
+
+ auto strongSelf = weakSelf.get();
+ [strongSelf _resetInputViewDeferral];
+ });
+
_page->activityStateDidChange(WebCore::ActivityState::IsFocused);
if ([self canShowNonEmptySelectionView])
[_textSelectionAssistant activateSelection];
+
+ [self _scheduleResetInputViewDeferralAfterBecomingFirstResponder];
} else
- _inputViewUpdateDeferrer = nullptr;
+ [self _resetInputViewDeferral];
return didBecomeFirstResponder;
}
@@ -1137,7 +1166,7 @@
[self _cancelInteraction];
[_textSelectionAssistant deactivateSelection];
- _inputViewUpdateDeferrer = nullptr;
+ [self _resetInputViewDeferral];
// If the user explicitly dismissed the keyboard then we will lose first responder
// status only to gain it back again. Just don't resign in that case.
@@ -2177,12 +2206,12 @@
{
[self _cancelInteraction];
- _inputViewUpdateDeferrer = nullptr;
+ [self _resetInputViewDeferral];
}
- (void)_didNotHandleTapAsClick:(const WebCore::IntPoint&)point
{
- _inputViewUpdateDeferrer = nullptr;
+ [self _resetInputViewDeferral];
// FIXME: we should also take into account whether or not the UI delegate
// has handled this notification.
@@ -2202,7 +2231,7 @@
- (void)_didCompleteSyntheticClick
{
- _inputViewUpdateDeferrer = nullptr;
+ [self _resetInputViewDeferral];
}
- (void)_singleTapCommited:(UITapGestureRecognizer *)gestureRecognizer
Modified: trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm (241845 => 241846)
--- trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm 2019-02-20 23:30:03 UTC (rev 241845)
+++ trunk/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm 2019-02-20 23:34:50 UTC (rev 241846)
@@ -36,6 +36,7 @@
#import "RemoteScrollingCoordinator.h"
#import "RemoteScrollingCoordinatorTransaction.h"
#import "WebPage.h"
+#import "WebPageProxyMessages.h"
#import "WebProcess.h"
#import <QuartzCore/QuartzCore.h>
#import <WebCore/DebugPageOverlays.h>
@@ -499,7 +500,7 @@
m_connection->sendMessage(WTFMove(m_commitEncoder), { });
}
-void RemoteLayerTreeDrawingArea::activityStateDidChange(OptionSet<WebCore::ActivityState::Flag>, ActivityStateChangeID activityStateChangeID, const Vector<CallbackID>&)
+void RemoteLayerTreeDrawingArea::activityStateDidChange(OptionSet<WebCore::ActivityState::Flag>, ActivityStateChangeID activityStateChangeID, const Vector<CallbackID>& callbackIDs)
{
// FIXME: Should we suspend painting while not visible, like TiledCoreAnimationDrawingArea? Probably.
@@ -508,6 +509,11 @@
m_activityStateChangeID = activityStateChangeID;
scheduleCompositingLayerFlushImmediately();
}
+
+ // FIXME: We may want to match behavior in TiledCoreAnimationDrawingArea by firing these callbacks after the next compositing flush, rather than immediately after
+ // handling an activity state change.
+ for (auto& callbackID : callbackIDs)
+ m_webPage.send(Messages::WebPageProxy::VoidCallback(callbackID));
}
void RemoteLayerTreeDrawingArea::addTransactionCallbackID(CallbackID callbackID)
