- Revision
- 237020
- Author
- [email protected]
- Date
- 2018-10-10 15:41:26 -0700 (Wed, 10 Oct 2018)
Log Message
REGRESSION (r236935): Layout test fast/events/ios/keyboard-scrolling-distance.html is timing out
https://bugs.webkit.org/show_bug.cgi?id=190444
<rdar://problem/45110698>
Reviewed by Simon Fraser.
There's a race in WKKeyboardScrollingAnimator that's exacerbated by HIDEventGenerator
being much faster than a human finger. We get our "begin" events from interpretKeyEvent,
after the Web Content process has had its way with it, but currently the
back-channel "handle" events (e.g. for key up, which doesn't go to interpretKeyEvent)
are retrieved from handleKeyWebEvent in the UI process, which is *before*
the Web Content process has had a swing at it.
If you lose the race (an insanely short tap like you get from HIDEventGenerator,
or with a very busy Web Content process), we see handle(keyDown), handle(keyUp), begin(keyDown),
and get stuck scrolling!
Instead, retrieve the out-of-band "handle" events from _didHandleKeyEvent,
so that they're sensibly and strictly ordered with respect to the timing of interpretKeyEvent/"begin".
Also, hook up didFinishScrolling, so that UIScriptController's callbacks work correctly.
* UIProcess/API/Cocoa/WKWebViewInternal.h:
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView handleKeyWebEvent:]):
(-[WKContentView _didHandleKeyEvent:eventWasHandled:]):
(-[WKContentView keyboardScrollViewAnimatorDidFinishScrolling:]):
* UIProcess/ios/WKKeyboardScrollingAnimator.h:
* UIProcess/ios/WKKeyboardScrollingAnimator.mm:
(-[WKKeyboardScrollingAnimator handleKeyEvent:]):
(-[WKKeyboardScrollingAnimator displayLinkFired:]):
(-[WKKeyboardScrollViewAnimator setDelegate:]):
(-[WKKeyboardScrollViewAnimator handleKeyEvent:]):
(-[WKKeyboardScrollViewAnimator didFinishScrolling]):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (237019 => 237020)
--- trunk/Source/WebKit/ChangeLog 2018-10-10 22:41:25 UTC (rev 237019)
+++ trunk/Source/WebKit/ChangeLog 2018-10-10 22:41:26 UTC (rev 237020)
@@ -1,5 +1,42 @@
2018-10-10 Tim Horton <[email protected]>
+ REGRESSION (r236935): Layout test fast/events/ios/keyboard-scrolling-distance.html is timing out
+ https://bugs.webkit.org/show_bug.cgi?id=190444
+ <rdar://problem/45110698>
+
+ Reviewed by Simon Fraser.
+
+ There's a race in WKKeyboardScrollingAnimator that's exacerbated by HIDEventGenerator
+ being much faster than a human finger. We get our "begin" events from interpretKeyEvent,
+ after the Web Content process has had its way with it, but currently the
+ back-channel "handle" events (e.g. for key up, which doesn't go to interpretKeyEvent)
+ are retrieved from handleKeyWebEvent in the UI process, which is *before*
+ the Web Content process has had a swing at it.
+
+ If you lose the race (an insanely short tap like you get from HIDEventGenerator,
+ or with a very busy Web Content process), we see handle(keyDown), handle(keyUp), begin(keyDown),
+ and get stuck scrolling!
+
+ Instead, retrieve the out-of-band "handle" events from _didHandleKeyEvent,
+ so that they're sensibly and strictly ordered with respect to the timing of interpretKeyEvent/"begin".
+
+ Also, hook up didFinishScrolling, so that UIScriptController's callbacks work correctly.
+
+ * UIProcess/API/Cocoa/WKWebViewInternal.h:
+ * UIProcess/ios/WKContentViewInteraction.mm:
+ (-[WKContentView handleKeyWebEvent:]):
+ (-[WKContentView _didHandleKeyEvent:eventWasHandled:]):
+ (-[WKContentView keyboardScrollViewAnimatorDidFinishScrolling:]):
+ * UIProcess/ios/WKKeyboardScrollingAnimator.h:
+ * UIProcess/ios/WKKeyboardScrollingAnimator.mm:
+ (-[WKKeyboardScrollingAnimator handleKeyEvent:]):
+ (-[WKKeyboardScrollingAnimator displayLinkFired:]):
+ (-[WKKeyboardScrollViewAnimator setDelegate:]):
+ (-[WKKeyboardScrollViewAnimator handleKeyEvent:]):
+ (-[WKKeyboardScrollViewAnimator didFinishScrolling]):
+
+2018-10-10 Tim Horton <[email protected]>
+
Share more WKShareSheet code between macOS and iOS, and fix a few bugs
https://bugs.webkit.org/show_bug.cgi?id=190420
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h (237019 => 237020)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h 2018-10-10 22:41:25 UTC (rev 237019)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h 2018-10-10 22:41:26 UTC (rev 237020)
@@ -101,6 +101,7 @@
- (BOOL)_zoomToRect:(WebCore::FloatRect)targetRect withOrigin:(WebCore::FloatPoint)origin fitEntireRect:(BOOL)fitEntireRect minimumScale:(double)minimumScale maximumScale:(double)maximumScale minimumScrollDistance:(float)minimumScrollDistance;
- (void)_zoomOutWithOrigin:(WebCore::FloatPoint)origin animated:(BOOL)animated;
- (void)_zoomToInitialScaleWithOrigin:(WebCore::FloatPoint)origin animated:(BOOL)animated;
+- (void)_didFinishScrolling;
- (void)_setHasCustomContentView:(BOOL)hasCustomContentView loadedMIMEType:(const WTF::String&)mimeType;
- (void)_didFinishLoadingDataForCustomContentProviderWithSuggestedFilename:(const WTF::String&)suggestedFilename data:(NSData *)data;
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (237019 => 237020)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2018-10-10 22:41:25 UTC (rev 237019)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2018-10-10 22:41:26 UTC (rev 237020)
@@ -3820,9 +3820,6 @@
- (void)handleKeyWebEvent:(::WebEvent *)theEvent
{
- if ([_keyboardScrollingAnimator handleKeyEvent:theEvent])
- return;
-
_page->handleKeyboardEvent(NativeWebKeyboardEvent(theEvent));
}
@@ -3834,6 +3831,8 @@
- (void)_didHandleKeyEvent:(::WebEvent *)event eventWasHandled:(BOOL)eventWasHandled
{
+ [_keyboardScrollingAnimator handleKeyEvent:event];
+
if (auto handler = WTFMove(_keyWebEventHandler)) {
handler(event, eventWasHandled);
return;
@@ -3952,6 +3951,11 @@
[self willStartZoomOrScroll];
}
+- (void)keyboardScrollViewAnimatorDidFinishScrolling:(WKKeyboardScrollViewAnimator *)animator
+{
+ [_webView _didFinishScrolling];
+}
+
- (void)executeEditCommandWithCallback:(NSString *)commandName
{
[self beginSelectionChange];
Modified: trunk/Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.h (237019 => 237020)
--- trunk/Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.h 2018-10-10 22:41:25 UTC (rev 237019)
+++ trunk/Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.h 2018-10-10 22:41:26 UTC (rev 237020)
@@ -53,7 +53,7 @@
- (void)willStartInteractiveScroll;
- (BOOL)beginWithEvent:(::WebEvent *)event;
-- (BOOL)handleKeyEvent:(::WebEvent *)event;
+- (void)handleKeyEvent:(::WebEvent *)event;
@property (nonatomic, weak) id <WKKeyboardScrollViewAnimatorDelegate> delegate;
@@ -64,6 +64,7 @@
- (BOOL)isScrollableForKeyboardScrollViewAnimator:(WKKeyboardScrollViewAnimator *)animator;
- (CGFloat)keyboardScrollViewAnimator:(WKKeyboardScrollViewAnimator *)animator distanceForIncrement:(WebKit::ScrollingIncrement)increment;
- (void)keyboardScrollViewAnimatorWillScroll:(WKKeyboardScrollViewAnimator *)animator;
+- (void)keyboardScrollViewAnimatorDidFinishScrolling:(WKKeyboardScrollViewAnimator *)animator;
@end
Modified: trunk/Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm (237019 => 237020)
--- trunk/Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm 2018-10-10 22:41:25 UTC (rev 237019)
+++ trunk/Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm 2018-10-10 22:41:26 UTC (rev 237020)
@@ -76,6 +76,7 @@
- (CGPoint)boundedContentOffset:(CGPoint)offset;
- (WebCore::RectEdges<bool>)scrollableDirectionsFromOffset:(CGPoint)offset;
- (WebCore::RectEdges<bool>)rubberbandableDirections;
+- (void)didFinishScrolling;
@end
@@ -89,7 +90,7 @@
- (void)willStartInteractiveScroll;
- (BOOL)beginWithEvent:(::WebEvent *)event;
-- (BOOL)handleKeyEvent:(::WebEvent *)event;
+- (void)handleKeyEvent:(::WebEvent *)event;
@end
@@ -311,10 +312,10 @@
return YES;
}
-- (BOOL)handleKeyEvent:(::WebEvent *)event
+- (void)handleKeyEvent:(::WebEvent *)event
{
if (!_hasPressedScrollingKey)
- return NO;
+ return;
auto scroll = [self keyboardScrollForEvent:event];
if (!scroll || event.type == WebEventKeyUp) {
@@ -321,8 +322,6 @@
[self stopAnimatedScroll];
_hasPressedScrollingKey = NO;
}
-
- return NO;
}
static WebCore::FloatPoint farthestPointInDirection(WebCore::FloatPoint a, WebCore::FloatPoint b, WebKit::ScrollingDirection direction)
@@ -437,6 +436,7 @@
// If we've effectively stopped scrolling, and no key is pressed,
// shut down the display link.
if (!_hasPressedScrollingKey && _velocity.diagonalLengthSquared() < 1) {
+ [_scrollable didFinishScrolling];
[self stopDisplayLink];
_velocity = { };
}
@@ -454,6 +454,7 @@
BOOL _delegateRespondsToIsKeyboardScrollable;
BOOL _delegateRespondsToDistanceForIncrement;
BOOL _delegateRespondsToWillScroll;
+ BOOL _delegateRespondsToDidFinishScrolling;
}
- (instancetype)init
@@ -494,6 +495,7 @@
_delegateRespondsToIsKeyboardScrollable = [_delegate respondsToSelector:@selector(isScrollableForKeyboardScrollViewAnimator:)];
_delegateRespondsToDistanceForIncrement = [_delegate respondsToSelector:@selector(keyboardScrollViewAnimator:distanceForIncrement:)];
_delegateRespondsToWillScroll = [_delegate respondsToSelector:@selector(keyboardScrollViewAnimatorWillScroll:)];
+ _delegateRespondsToDidFinishScrolling = [_delegate respondsToSelector:@selector(keyboardScrollViewAnimatorDidFinishScrolling:)];
}
- (void)willStartInteractiveScroll
@@ -506,7 +508,7 @@
return [_animator beginWithEvent:event];
}
-- (BOOL)handleKeyEvent:(::WebEvent *)event
+- (void)handleKeyEvent:(::WebEvent *)event
{
return [_animator handleKeyEvent:event];
}
@@ -627,6 +629,12 @@
return edges;
}
+- (void)didFinishScrolling
+{
+ if (_delegateRespondsToDidFinishScrolling)
+ [_delegate keyboardScrollViewAnimatorDidFinishScrolling:self];
+}
+
@end
#endif // PLATFORM(IOS)
