Title: [251665] trunk/Source/WebKit
- Revision
- 251665
- Author
- [email protected]
- Date
- 2019-10-28 12:58:02 -0700 (Mon, 28 Oct 2019)
Log Message
[iOS] 3 editing/pasteboard/smart-paste-paragraph-* tests are flaky
https://bugs.webkit.org/show_bug.cgi?id=203264
<rdar://problem/56512107>
Reviewed by Tim Horton.
Fixes several flaky layout tests that exercise a corner case in our logic for caching position information
responses in the UI process. When focusing an element via a tap, we send a position information request for the
tap location in -_webTouchEventsRecognized:. After the web process computes the information and hands it back to
the UI process, we cache this in WKContentView's _positionInformation.
However, at the time of computing the request, the tapped element has not been focused yet, so the value of the
position information's nodeAtPositionIsFocusedElement flag is false. After the tap is recognized, we'll then
focus the element, such that if a subsequent position information request were to arrive at the same location,
it would have a nodeAtPositionIsFocusedElement flag set to true.
In this state, if the user taps _exactly_ at the same location again, UIKit (through text interaction gestures)
will ask us for information at the same point; we will end up using the cached information, for which
nodeAtPositionIsFocusedElement is false, causing us to incorrectly prevent the text interaction. In this
particular case, we fail to select text via a double tap gesture.
To address this, we invalidate the cached position information in the UI process whenever the focused element
rect changes (e.g. when the focused element changes); the only exception to this is when the previously cached
position information was not over the focused element, and the new focused element rect is empty, in which case
the value of nodeAtPositionIsFocusedElement is guaranteed to have not changed.
While this may potentially leads to an additional synchronous position information request when tapping at the
same location after focusing an element, this is very difficult to achieve in practice, since the tap location
would need to be _exactly_ at the same location.
No new test, since this is exercised by existing flaky layout tests.
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _elementDidFocus:userIsInteracting:blurPreviousNode:activityStateChanges:userObject:]):
(-[WKContentView _elementDidBlur]):
Also, add a FIXME about how we clear out surprisingly little of _focusedElementInformation when blurring the
focused element.
(-[WKContentView _didChangeFocusedElementRect:toRect:]):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (251664 => 251665)
--- trunk/Source/WebKit/ChangeLog 2019-10-28 19:52:54 UTC (rev 251664)
+++ trunk/Source/WebKit/ChangeLog 2019-10-28 19:58:02 UTC (rev 251665)
@@ -1,3 +1,46 @@
+2019-10-28 Wenson Hsieh <[email protected]>
+
+ [iOS] 3 editing/pasteboard/smart-paste-paragraph-* tests are flaky
+ https://bugs.webkit.org/show_bug.cgi?id=203264
+ <rdar://problem/56512107>
+
+ Reviewed by Tim Horton.
+
+ Fixes several flaky layout tests that exercise a corner case in our logic for caching position information
+ responses in the UI process. When focusing an element via a tap, we send a position information request for the
+ tap location in -_webTouchEventsRecognized:. After the web process computes the information and hands it back to
+ the UI process, we cache this in WKContentView's _positionInformation.
+
+ However, at the time of computing the request, the tapped element has not been focused yet, so the value of the
+ position information's nodeAtPositionIsFocusedElement flag is false. After the tap is recognized, we'll then
+ focus the element, such that if a subsequent position information request were to arrive at the same location,
+ it would have a nodeAtPositionIsFocusedElement flag set to true.
+
+ In this state, if the user taps _exactly_ at the same location again, UIKit (through text interaction gestures)
+ will ask us for information at the same point; we will end up using the cached information, for which
+ nodeAtPositionIsFocusedElement is false, causing us to incorrectly prevent the text interaction. In this
+ particular case, we fail to select text via a double tap gesture.
+
+ To address this, we invalidate the cached position information in the UI process whenever the focused element
+ rect changes (e.g. when the focused element changes); the only exception to this is when the previously cached
+ position information was not over the focused element, and the new focused element rect is empty, in which case
+ the value of nodeAtPositionIsFocusedElement is guaranteed to have not changed.
+
+ While this may potentially leads to an additional synchronous position information request when tapping at the
+ same location after focusing an element, this is very difficult to achieve in practice, since the tap location
+ would need to be _exactly_ at the same location.
+
+ No new test, since this is exercised by existing flaky layout tests.
+
+ * UIProcess/ios/WKContentViewInteraction.mm:
+ (-[WKContentView _elementDidFocus:userIsInteracting:blurPreviousNode:activityStateChanges:userObject:]):
+ (-[WKContentView _elementDidBlur]):
+
+ Also, add a FIXME about how we clear out surprisingly little of _focusedElementInformation when blurring the
+ focused element.
+
+ (-[WKContentView _didChangeFocusedElementRect:toRect:]):
+
2019-10-28 John Wilander <[email protected]>
Storage Access API: Make the API work with the experimental 3rd-party cookie blocking
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (251664 => 251665)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2019-10-28 19:52:54 UTC (rev 251664)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2019-10-28 19:58:02 UTC (rev 251665)
@@ -5556,6 +5556,7 @@
if (delegateImplementsWillStartInputSession)
[inputDelegate _webView:_webView willStartInputSession:_formInputSession.get()];
+ auto previousElementRect = _isChangingFocus ? _focusedElementInformation.elementRect : WebCore::IntRect();
BOOL isSelectable = mayContainSelectableText(information.elementType);
BOOL editableChanged = [self setIsEditable:isSelectable];
_focusedElementInformation = information;
@@ -5602,6 +5603,8 @@
[inputDelegate _webView:_webView didStartInputSession:_formInputSession.get()];
[_webView didStartFormControlInteraction];
+
+ [self _didChangeFocusedElementRect:previousElementRect toRect:_focusedElementInformation.elementRect];
}
- (void)_elementDidBlur
@@ -5625,7 +5628,8 @@
#endif
BOOL editableChanged = [self setIsEditable:NO];
-
+ auto previousElementRect = _focusedElementInformation.elementRect;
+ // FIXME: We should completely invalidate _focusedElementInformation here, instead of a subset of individual members.
_focusedElementInformation.elementType = WebKit::InputType::None;
_focusedElementInformation.shouldSynthesizeKeyEventsForEditing = false;
_focusedElementInformation.shouldAvoidResizingWhenInputViewBoundsChange = false;
@@ -5658,10 +5662,25 @@
_page->setIsShowingInputViewForFocusedElement(false);
}
- if (!_isChangingFocus)
+ if (!_isChangingFocus) {
_didAccessoryTabInitiateFocus = NO;
+ [self _didChangeFocusedElementRect:previousElementRect toRect:WebCore::IntRect()];
+ }
}
+- (void)_didChangeFocusedElementRect:(const WebCore::IntRect&)previousRect toRect:(const WebCore::IntRect&)newRect
+{
+ if (previousRect == newRect)
+ return;
+
+ if (newRect.isEmpty() && !_positionInformation.nodeAtPositionIsFocusedElement)
+ return;
+
+ // If the focused element rect changed, the cached position information's nodeAtPositionIsFocusedElement may be stale.
+ _hasValidPositionInformation = NO;
+ _positionInformation = { };
+}
+
- (void)_updateInputContextAfterBlurringAndRefocusingElement
{
if (!hasFocusedElement(_focusedElementInformation) || !_suppressSelectionAssistantReasons)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
