- Revision
- 264484
- Author
- [email protected]
- Date
- 2020-07-16 16:33:20 -0700 (Thu, 16 Jul 2020)
Log Message
iPad cursor is sometimes slow to change to I-beam (e.g. on reddit.com)
https://bugs.webkit.org/show_bug.cgi?id=214424
<rdar://problem/59503572>
Reviewed by Wenson Hsieh.
No new tests, just an optimization.
* Shared/ios/InteractionInformationRequest.h:
* Shared/ios/InteractionInformationRequest.cpp:
(WebKit::InteractionInformationRequest::encode const):
(WebKit::InteractionInformationRequest::decode):
(WebKit::InteractionInformationRequest::isValidForRequest):
(WebKit::InteractionInformationRequest::isApproximatelyValidForRequest):
'const'-ify these getters
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::positionInformation):
Make it possible to disable computation of the 'nodeAtPositionHasDoubleClickHandler' property,
because it can be quite expensive for some web content. We will still compute it by default,
but high-rate interaction information requests (like those for the cursor) can optionally disable it.
* UIProcess/ios/WKContentViewInteraction.h:
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView cleanUpInteraction]):
(-[WKContentView requestAsynchronousPositionInformationUpdate:]):
(-[WKContentView _hasValidOutstandingPositionInformationRequest:]):
(-[WKContentView _positionInformationDidChange:]):
When position information updates come in, only clear the outstanding request
if the reply is valid for that request; otherwise, the outstanding request is
definitely still outstanding, and we've probably just sent multiple requests.
A more formal mechanism that actually keeps track of all requests might be
more appropriate here in the future, but this helps us avoid re-requesting
position information for the same point repeatedly, just because old (stale)
updates are coming in.
(-[WKContentView _cursorInteraction:regionForLocation:defaultRegion:completion:]):
Instead of requesting an interaction information update for every cursor update,
and then dropping them on the floor when they return, if a new request was sent,
simply coalesce all requests that come in while we have an outstanding one.
This avoids a great deal of unnecessary (and ignored) work when the Web Content
process is responding to requests at a slower rate than the cursor interaction
is trying to retrieve new regions.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (264483 => 264484)
--- trunk/Source/WebKit/ChangeLog 2020-07-16 23:25:15 UTC (rev 264483)
+++ trunk/Source/WebKit/ChangeLog 2020-07-16 23:33:20 UTC (rev 264484)
@@ -1,3 +1,49 @@
+2020-07-16 Tim Horton <[email protected]>
+
+ iPad cursor is sometimes slow to change to I-beam (e.g. on reddit.com)
+ https://bugs.webkit.org/show_bug.cgi?id=214424
+ <rdar://problem/59503572>
+
+ Reviewed by Wenson Hsieh.
+
+ No new tests, just an optimization.
+
+ * Shared/ios/InteractionInformationRequest.h:
+ * Shared/ios/InteractionInformationRequest.cpp:
+ (WebKit::InteractionInformationRequest::encode const):
+ (WebKit::InteractionInformationRequest::decode):
+ (WebKit::InteractionInformationRequest::isValidForRequest):
+ (WebKit::InteractionInformationRequest::isApproximatelyValidForRequest):
+ 'const'-ify these getters
+
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::positionInformation):
+ Make it possible to disable computation of the 'nodeAtPositionHasDoubleClickHandler' property,
+ because it can be quite expensive for some web content. We will still compute it by default,
+ but high-rate interaction information requests (like those for the cursor) can optionally disable it.
+
+ * UIProcess/ios/WKContentViewInteraction.h:
+ * UIProcess/ios/WKContentViewInteraction.mm:
+ (-[WKContentView cleanUpInteraction]):
+ (-[WKContentView requestAsynchronousPositionInformationUpdate:]):
+ (-[WKContentView _hasValidOutstandingPositionInformationRequest:]):
+ (-[WKContentView _positionInformationDidChange:]):
+ When position information updates come in, only clear the outstanding request
+ if the reply is valid for that request; otherwise, the outstanding request is
+ definitely still outstanding, and we've probably just sent multiple requests.
+ A more formal mechanism that actually keeps track of all requests might be
+ more appropriate here in the future, but this helps us avoid re-requesting
+ position information for the same point repeatedly, just because old (stale)
+ updates are coming in.
+
+ (-[WKContentView _cursorInteraction:regionForLocation:defaultRegion:completion:]):
+ Instead of requesting an interaction information update for every cursor update,
+ and then dropping them on the floor when they return, if a new request was sent,
+ simply coalesce all requests that come in while we have an outstanding one.
+ This avoids a great deal of unnecessary (and ignored) work when the Web Content
+ process is responding to requests at a slower rate than the cursor interaction
+ is trying to retrieve new regions.
+
2020-07-16 Megan Gardner <[email protected]>
Selection is not always clearing when tapping.
Modified: trunk/Source/WebKit/Shared/ios/InteractionInformationAtPosition.h (264483 => 264484)
--- trunk/Source/WebKit/Shared/ios/InteractionInformationAtPosition.h 2020-07-16 23:25:15 UTC (rev 264483)
+++ trunk/Source/WebKit/Shared/ios/InteractionInformationAtPosition.h 2020-07-16 23:33:20 UTC (rev 264484)
@@ -52,7 +52,7 @@
InteractionInformationRequest request;
bool canBeValid { true };
- bool nodeAtPositionHasDoubleClickHandler { false };
+ Optional<bool> nodeAtPositionHasDoubleClickHandler;
bool isSelectable { false };
bool prefersDraggingOverTextSelection { false };
bool isNearMarkedText { false };
Modified: trunk/Source/WebKit/Shared/ios/InteractionInformationRequest.cpp (264483 => 264484)
--- trunk/Source/WebKit/Shared/ios/InteractionInformationRequest.cpp 2020-07-16 23:25:15 UTC (rev 264483)
+++ trunk/Source/WebKit/Shared/ios/InteractionInformationRequest.cpp 2020-07-16 23:33:20 UTC (rev 264484)
@@ -39,6 +39,7 @@
encoder << includeSnapshot;
encoder << includeLinkIndicator;
encoder << includeCaretContext;
+ encoder << includeHasDoubleClickHandler;
encoder << linkIndicatorShouldHaveLegacyMargins;
}
@@ -56,6 +57,9 @@
if (!decoder.decode(result.includeCaretContext))
return false;
+ if (!decoder.decode(result.includeHasDoubleClickHandler))
+ return false;
+
if (!decoder.decode(result.linkIndicatorShouldHaveLegacyMargins))
return false;
@@ -62,7 +66,7 @@
return true;
}
-bool InteractionInformationRequest::isValidForRequest(const InteractionInformationRequest& other, int radius)
+bool InteractionInformationRequest::isValidForRequest(const InteractionInformationRequest& other, int radius) const
{
if (other.includeSnapshot && !includeSnapshot)
return false;
@@ -73,6 +77,9 @@
if (other.includeCaretContext && !includeCaretContext)
return false;
+ if (other.includeHasDoubleClickHandler && !includeHasDoubleClickHandler)
+ return false;
+
if (other.linkIndicatorShouldHaveLegacyMargins != linkIndicatorShouldHaveLegacyMargins)
return false;
@@ -79,7 +86,7 @@
return (other.point - point).diagonalLengthSquared() <= radius * radius;
}
-bool InteractionInformationRequest::isApproximatelyValidForRequest(const InteractionInformationRequest& other, int radius)
+bool InteractionInformationRequest::isApproximatelyValidForRequest(const InteractionInformationRequest& other, int radius) const
{
return isValidForRequest(other, radius);
}
Modified: trunk/Source/WebKit/Shared/ios/InteractionInformationRequest.h (264483 => 264484)
--- trunk/Source/WebKit/Shared/ios/InteractionInformationRequest.h 2020-07-16 23:25:15 UTC (rev 264483)
+++ trunk/Source/WebKit/Shared/ios/InteractionInformationRequest.h 2020-07-16 23:33:20 UTC (rev 264484)
@@ -42,6 +42,7 @@
bool includeSnapshot { false };
bool includeLinkIndicator { false };
bool includeCaretContext { false };
+ bool includeHasDoubleClickHandler { true };
bool linkIndicatorShouldHaveLegacyMargins { false };
@@ -51,8 +52,8 @@
this->point = point;
}
- bool isValidForRequest(const InteractionInformationRequest&, int radius = 0);
- bool isApproximatelyValidForRequest(const InteractionInformationRequest&, int radius);
+ bool isValidForRequest(const InteractionInformationRequest&, int radius = 0) const;
+ bool isApproximatelyValidForRequest(const InteractionInformationRequest&, int radius) const;
void encode(IPC::Encoder&) const;
static WARN_UNUSED_RETURN bool decode(IPC::Decoder&, InteractionInformationRequest&);
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h (264483 => 264484)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h 2020-07-16 23:25:15 UTC (rev 264483)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h 2020-07-16 23:33:20 UTC (rev 264484)
@@ -124,8 +124,6 @@
typedef BlockPtr<void(WebKit::InteractionInformationAtPosition)> InteractionInformationCallback;
typedef std::pair<WebKit::InteractionInformationRequest, InteractionInformationCallback> InteractionInformationRequestAndCallback;
-typedef uint64_t CursorInteractionRequestID;
-
#define FOR_EACH_WKCONTENTVIEW_ACTION(M) \
M(_addShortcut) \
M(_define) \
@@ -249,7 +247,8 @@
#if HAVE(UI_CURSOR_INTERACTION)
RetainPtr<_UICursorInteraction> _cursorInteraction;
- CursorInteractionRequestID _currentCursorInteractionRequestID;
+ BOOL _hasOutstandingCursorInteractionRequest;
+ Optional<std::pair<WebKit::InteractionInformationRequest, BlockPtr<void(_UICursorRegion *)>>> _deferredCursorInteractionRequest;
#endif
RetainPtr<UIWKTextInteractionAssistant> _textInteractionAssistant;
@@ -324,7 +323,7 @@
WebKit::WKSelectionDrawingInfo _lastSelectionDrawingInfo;
- Optional<WebKit::InteractionInformationRequest> _outstandingPositionInformationRequest;
+ Optional<WebKit::InteractionInformationRequest> _lastOutstandingPositionInformationRequest;
uint64_t _positionInformationCallbackDepth;
Vector<Optional<InteractionInformationRequestAndCallback>> _pendingPositionInformationHandlers;
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (264483 => 264484)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2020-07-16 23:25:15 UTC (rev 264483)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2020-07-16 23:33:20 UTC (rev 264484)
@@ -918,7 +918,7 @@
[_formInputSession invalidate];
_formInputSession = nil;
[_highlightView removeFromSuperview];
- _outstandingPositionInformationRequest = WTF::nullopt;
+ _lastOutstandingPositionInformationRequest = WTF::nullopt;
_isWaitingOnPositionInformation = NO;
_focusRequiresStrongPasswordAssistance = NO;
@@ -2284,7 +2284,7 @@
if ([self _currentPositionInformationIsValidForRequest:request])
return;
- _outstandingPositionInformationRequest = request;
+ _lastOutstandingPositionInformationRequest = request;
_page->requestPositionInformation(request);
}
@@ -2296,7 +2296,7 @@
- (BOOL)_hasValidOutstandingPositionInformationRequest:(const WebKit::InteractionInformationRequest&)request
{
- return _outstandingPositionInformationRequest && _outstandingPositionInformationRequest->isValidForRequest(request);
+ return _lastOutstandingPositionInformationRequest && _lastOutstandingPositionInformationRequest->isValidForRequest(request);
}
- (BOOL)_currentPositionInformationIsApproximatelyValidForRequest:(const WebKit::InteractionInformationRequest&)request radiusForApproximation:(int)radius
@@ -2418,11 +2418,11 @@
if (gestureRecognizer == _doubleTapGestureRecognizerForDoubleClick) {
// Do not start the double-tap-for-double-click gesture recognizer unless we've got a dblclick event handler on the node at the tap location.
WebKit::InteractionInformationRequest request(WebCore::roundedIntPoint(point));
- if ([self _currentPositionInformationIsApproximatelyValidForRequest:request radiusForApproximation:[_doubleTapGestureRecognizerForDoubleClick allowableMovement]])
- return _positionInformation.nodeAtPositionHasDoubleClickHandler;
- if (![self ensurePositionInformationIsUpToDate:request])
- return NO;
- return _positionInformation.nodeAtPositionHasDoubleClickHandler;
+ if (![self _currentPositionInformationIsApproximatelyValidForRequest:request radiusForApproximation:[_doubleTapGestureRecognizerForDoubleClick allowableMovement]]) {
+ if (![self ensurePositionInformationIsUpToDate:request])
+ return NO;
+ }
+ return _positionInformation.nodeAtPositionHasDoubleClickHandler.valueOr(false);
}
if (gestureRecognizer == _highlightLongPressGestureRecognizer
@@ -2896,7 +2896,9 @@
- (void)_positionInformationDidChange:(const WebKit::InteractionInformationAtPosition&)info
{
- _outstandingPositionInformationRequest = WTF::nullopt;
+ if (_lastOutstandingPositionInformationRequest && info.request.isValidForRequest(*_lastOutstandingPositionInformationRequest))
+ _lastOutstandingPositionInformationRequest = WTF::nullopt;
+
_isWaitingOnPositionInformation = NO;
WebKit::InteractionInformationAtPosition newInfo = info;
@@ -8665,11 +8667,10 @@
- (void)_cursorInteraction:(_UICursorInteraction *)interaction regionForLocation:(CGPoint)location defaultRegion:(_UICursorRegion *)defaultRegion completion:(void(^)(_UICursorRegion *region))completion
{
- uint64_t interactionRequestID = ++_currentCursorInteractionRequestID;
-
WebKit::InteractionInformationRequest interactionInformationRequest;
interactionInformationRequest.point = WebCore::roundedIntPoint(location);
interactionInformationRequest.includeCaretContext = true;
+ interactionInformationRequest.includeHasDoubleClickHandler = false;
BOOL didSynchronouslyReplyWithApproximation = false;
if (![self _currentPositionInformationIsValidForRequest:interactionInformationRequest] && self.webView._editable && !_positionInformation.shouldNotUseIBeamInEditableContent) {
@@ -8677,10 +8678,20 @@
completion([_UICursorRegion regionWithIdentifier:editableCursorRegionIdentifier rect:self.bounds]);
}
- [self doAfterPositionInformationUpdate:^(WebKit::InteractionInformationAtPosition interactionInformation) {
- if (interactionRequestID < _currentCursorInteractionRequestID)
- return;
+ // If we already have an outstanding interaction information request, defer this one until
+ // we hear back, so that requests don't pile up if the Web Content process is slow.
+ if (_hasOutstandingCursorInteractionRequest) {
+ _deferredCursorInteractionRequest = std::make_pair(interactionInformationRequest, makeBlockPtr(completion));
+ return;
+ }
+ _hasOutstandingCursorInteractionRequest = YES;
+
+ __block BlockPtr<void(WebKit::InteractionInformationAtPosition, void(^)(_UICursorRegion *))> replyHandler;
+ replyHandler = ^(WebKit::InteractionInformationAtPosition interactionInformation, void(^completion)(_UICursorRegion *region)) {
+ if (!_deferredCursorInteractionRequest)
+ _hasOutstandingCursorInteractionRequest = NO;
+
if (didSynchronouslyReplyWithApproximation) {
[interaction invalidate];
return;
@@ -8687,6 +8698,18 @@
}
completion([self cursorRegionForPositionInformation:interactionInformation point:location]);
+
+ if (_deferredCursorInteractionRequest) {
+ auto deferredRequest = std::exchange(_deferredCursorInteractionRequest, WTF::nullopt);
+ [self doAfterPositionInformationUpdate:^(WebKit::InteractionInformationAtPosition interactionInformation) {
+ replyHandler(interactionInformation, deferredRequest->second.get());
+ } forRequest:deferredRequest->first];
+ return;
+ }
+ };
+
+ [self doAfterPositionInformationUpdate:^(WebKit::InteractionInformationAtPosition interactionInformation) {
+ replyHandler(interactionInformation, completion);
} forRequest:interactionInformationRequest];
}
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (264483 => 264484)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2020-07-16 23:25:15 UTC (rev 264483)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2020-07-16 23:33:20 UTC (rev 264484)
@@ -2945,8 +2945,10 @@
auto* nodeRespondingToClickEvents = m_page->mainFrame().nodeRespondingToClickEvents(request.point, adjustedPoint);
info.adjustedPointForNodeRespondingToClickEvents = adjustedPoint;
- info.nodeAtPositionHasDoubleClickHandler = m_page->mainFrame().nodeRespondingToDoubleClickEvent(request.point, adjustedPoint);
+ if (request.includeHasDoubleClickHandler)
+ info.nodeAtPositionHasDoubleClickHandler = m_page->mainFrame().nodeRespondingToDoubleClickEvent(request.point, adjustedPoint);
+
auto& eventHandler = m_page->mainFrame().eventHandler();
constexpr OptionSet<HitTestRequest::RequestType> hitType { HitTestRequest::ReadOnly, HitTestRequest::AllowFrameScrollbars, HitTestRequest::AllowVisibleChildFrameContentOnly };
HitTestResult hitTestResultForCursor = eventHandler.hitTestResultAtPoint(request.point, hitType);
