WebKit Bugzilla
Attachment 362531 Details for
Bug 194601
: REGRESSION: [ iOS ] Layout Test editing/input/ios/rtl-keyboard-input-on-focus.html is a Timeout
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-194601-20190220132202.patch (text/plain), 11.25 KB, created by
Wenson Hsieh
on 2019-02-20 13:22:02 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Wenson Hsieh
Created:
2019-02-20 13:22:02 PST
Size:
11.25 KB
patch
obsolete
>Subversion Revision: 241754 >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index 6256c8af0edd9565666e9df373d167b55e74ad0c..5713cf94e26db5f2b69accfd6c1a311e39dfe343 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,55 @@ >+2019-02-20 Wenson Hsieh <wenson_hsieh@apple.com> >+ >+ 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 NOBODY (OOPS!). >+ >+ 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-18 Alex Christensen <achristensen@webkit.org> > > Revert functional part of r241451 >diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp >index 52b4c8e227a3bd3d7e457bf6c5dcd0e36b187bda..43d5603809ffd89059361b49c22323dc902fe28e 100644 >--- a/Source/WebKit/UIProcess/WebPageProxy.cpp >+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp >@@ -7984,20 +7984,6 @@ NSObject *WebPageProxy::immediateActionAnimationControllerForHitTestResult(RefPt > 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); >@@ -8032,6 +8018,20 @@ void WebPageProxy::setFooterBannerHeightForTesting(int height) > > #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); >diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h >index ac920260a54141ae31a693d7a89546e42befa7ab..868f861cd958beb40eadcbde96f48c3c4460c0b0 100644 >--- a/Source/WebKit/UIProcess/WebPageProxy.h >+++ b/Source/WebKit/UIProcess/WebPageProxy.h >@@ -1289,8 +1289,6 @@ public: > > NSObject *immediateActionAnimationControllerForHitTestResult(RefPtr<API::HitTestResult>, uint64_t, RefPtr<API::Object>); > >- void installActivityStateChangeCompletionHandler(WTF::Function<void ()>&&); >- > void handleAcceptedCandidate(WebCore::TextCheckingResult); > void didHandleAcceptedCandidate(); > >@@ -1298,6 +1296,8 @@ public: > 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 >diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm >index 1667efc669c39a5b60bb4bc41c510d1d53a9ab13..01ffc1935ac10616e10345e311f00d2cd40e4abb 100644 >--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm >+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm >@@ -868,7 +868,7 @@ - (void)cleanupInteraction > } > #endif > >- _inputViewUpdateDeferrer = nullptr; >+ [self _resetInputViewDeferral]; > _focusedElementInformation = { }; > > [_keyboardScrollingAnimator invalidate]; >@@ -1072,6 +1072,25 @@ - (void)_endEditing > #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 @@ - (BOOL)becomeFirstResponderForWebView > } > > 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 @@ - (BOOL)resignFirstResponderForWebView > [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. >@@ -2181,12 +2210,12 @@ - (void)_commitPotentialTapFailed > { > [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. >@@ -2206,7 +2235,7 @@ - (void)_didNotHandleTapAsClick:(const WebCore::IntPoint&)point > > - (void)_didCompleteSyntheticClick > { >- _inputViewUpdateDeferrer = nullptr; >+ [self _resetInputViewDeferral]; > } > > - (void)_singleTapCommited:(UITapGestureRecognizer *)gestureRecognizer >diff --git a/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm b/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm >index ce2d5cbdece8d9dc5177168df056109a30ee89b2..06148bf5e348c6ea19b14038ea52cf14172eec21 100644 >--- a/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm >+++ b/Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm >@@ -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 @@ void RemoteLayerTreeDrawingArea::BackingStoreFlusher::flush() > 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,9 @@ void RemoteLayerTreeDrawingArea::activityStateDidChange(OptionSet<WebCore::Activ > m_activityStateChangeID = activityStateChangeID; > scheduleCompositingLayerFlushImmediately(); > } >+ >+ for (const auto& callbackID : callbackIDs) >+ m_webPage.send(Messages::WebPageProxy::VoidCallback(callbackID)); > } > > void RemoteLayerTreeDrawingArea::addTransactionCallbackID(CallbackID callbackID)
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
thorton
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 194601
:
362478
| 362531 |
362548