REGRESSION (r236935): Layout test fast/events/ios/keyboard-scrolling-distance.html...
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Oct 2018 22:41:26 +0000 (22:41 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Oct 2018 22:41:26 +0000 (22:41 +0000)
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]):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@237020 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.h
Source/WebKit/UIProcess/ios/WKKeyboardScrollingAnimator.mm

index f3c405e..2bb52a2 100644 (file)
@@ -1,5 +1,42 @@
 2018-10-10  Tim Horton  <timothy_horton@apple.com>
 
+        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  <timothy_horton@apple.com>
+
         Share more WKShareSheet code between macOS and iOS, and fix a few bugs
         https://bugs.webkit.org/show_bug.cgi?id=190420
 
index b2b8815..aa5a812 100644 (file)
@@ -101,6 +101,7 @@ struct PrintInfo;
 - (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;
index ca3acd6..5e9e7c9 100644 (file)
@@ -3820,9 +3820,6 @@ static NSString *contentTypeFromFieldName(WebCore::AutofillFieldName fieldName)
 
 - (void)handleKeyWebEvent:(::WebEvent *)theEvent
 {
-    if ([_keyboardScrollingAnimator handleKeyEvent:theEvent])
-        return;
-
     _page->handleKeyboardEvent(NativeWebKeyboardEvent(theEvent));
 }
 
@@ -3834,6 +3831,8 @@ static NSString *contentTypeFromFieldName(WebCore::AutofillFieldName fieldName)
 
 - (void)_didHandleKeyEvent:(::WebEvent *)event eventWasHandled:(BOOL)eventWasHandled
 {
+    [_keyboardScrollingAnimator handleKeyEvent:event];
+    
     if (auto handler = WTFMove(_keyWebEventHandler)) {
         handler(event, eventWasHandled);
         return;
@@ -3952,6 +3951,11 @@ static NSString *contentTypeFromFieldName(WebCore::AutofillFieldName fieldName)
     [self willStartZoomOrScroll];
 }
 
+- (void)keyboardScrollViewAnimatorDidFinishScrolling:(WKKeyboardScrollViewAnimator *)animator
+{
+    [_webView _didFinishScrolling];
+}
+
 - (void)executeEditCommandWithCallback:(NSString *)commandName
 {
     [self beginSelectionChange];
index 0a7f0e1..76f8823 100644 (file)
@@ -53,7 +53,7 @@ enum class ScrollingIncrement : uint8_t {
 - (void)willStartInteractiveScroll;
 
 - (BOOL)beginWithEvent:(::WebEvent *)event;
-- (BOOL)handleKeyEvent:(::WebEvent *)event;
+- (void)handleKeyEvent:(::WebEvent *)event;
 
 @property (nonatomic, weak) id <WKKeyboardScrollViewAnimatorDelegate> delegate;
 
@@ -64,6 +64,7 @@ enum class ScrollingIncrement : uint8_t {
 - (BOOL)isScrollableForKeyboardScrollViewAnimator:(WKKeyboardScrollViewAnimator *)animator;
 - (CGFloat)keyboardScrollViewAnimator:(WKKeyboardScrollViewAnimator *)animator distanceForIncrement:(WebKit::ScrollingIncrement)increment;
 - (void)keyboardScrollViewAnimatorWillScroll:(WKKeyboardScrollViewAnimator *)animator;
+- (void)keyboardScrollViewAnimatorDidFinishScrolling:(WKKeyboardScrollViewAnimator *)animator;
 
 @end
 
index 65e922c..38d252e 100644 (file)
@@ -76,6 +76,7 @@ struct KeyboardScrollParameters {
 - (CGPoint)boundedContentOffset:(CGPoint)offset;
 - (WebCore::RectEdges<bool>)scrollableDirectionsFromOffset:(CGPoint)offset;
 - (WebCore::RectEdges<bool>)rubberbandableDirections;
+- (void)didFinishScrolling;
 
 @end
 
@@ -89,7 +90,7 @@ struct KeyboardScrollParameters {
 - (void)willStartInteractiveScroll;
 
 - (BOOL)beginWithEvent:(::WebEvent *)event;
-- (BOOL)handleKeyEvent:(::WebEvent *)event;
+- (void)handleKeyEvent:(::WebEvent *)event;
 
 @end
 
@@ -311,18 +312,16 @@ static WebCore::PhysicalBoxSide boxSide(WebKit::ScrollingDirection direction)
     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) {
         [self stopAnimatedScroll];
         _hasPressedScrollingKey = NO;
     }
-
-    return NO;
 }
 
 static WebCore::FloatPoint farthestPointInDirection(WebCore::FloatPoint a, WebCore::FloatPoint b, WebKit::ScrollingDirection direction)
@@ -437,6 +436,7 @@ static WebCore::FloatPoint farthestPointInDirection(WebCore::FloatPoint a, WebCo
     // 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 @@ static WebCore::FloatPoint farthestPointInDirection(WebCore::FloatPoint a, WebCo
     BOOL _delegateRespondsToIsKeyboardScrollable;
     BOOL _delegateRespondsToDistanceForIncrement;
     BOOL _delegateRespondsToWillScroll;
+    BOOL _delegateRespondsToDidFinishScrolling;
 }
 
 - (instancetype)init
@@ -494,6 +495,7 @@ static WebCore::FloatPoint farthestPointInDirection(WebCore::FloatPoint a, WebCo
     _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 @@ static WebCore::FloatPoint farthestPointInDirection(WebCore::FloatPoint a, WebCo
     return [_animator beginWithEvent:event];
 }
 
-- (BOOL)handleKeyEvent:(::WebEvent *)event
+- (void)handleKeyEvent:(::WebEvent *)event
 {
     return [_animator handleKeyEvent:event];
 }
@@ -627,6 +629,12 @@ static WebCore::FloatPoint farthestPointInDirection(WebCore::FloatPoint a, WebCo
     return edges;
 }
 
+- (void)didFinishScrolling
+{
+    if (_delegateRespondsToDidFinishScrolling)
+        [_delegate keyboardScrollViewAnimatorDidFinishScrolling:self];
+}
+
 @end
 
 #endif // PLATFORM(IOS)