Title: [261480] trunk
Revision
261480
Author
[email protected]
Date
2020-05-11 09:22:56 -0700 (Mon, 11 May 2020)

Log Message

REGRESSION (r253267): issues on touchstart/touchend/touchmove (pointerdown/pointerup/pointermove) events
https://bugs.webkit.org/show_bug.cgi?id=211521
<rdar://problem/62942374>

Reviewed by Darin Adler.

Source/WebKit:

As a brief refresher, deferring gesture recognizers allow us to handle otherwise blocking touch events
asynchronously by having all preventable native gesture recognizers require the deferring gesture recognizer to
fail; we only fail the deferring gesture recognizer once the web process has handled the touch event, and did
not call `preventDefault()`.

These additional failure requirements can cause preventable gestures to be linked together in the same gesture
dependency subgraph; since each subgraph is reset only once all gestures in the subgraph have failed or ended,
this might cause some gestures to be reset after a delay (rather than being reset immediately). To mitigate
this, we divide the set of preventable gestures into multiple (currently, 2) subgraphs: one for gestures that
are reset after a delay, and another for gestures that are immediately resettable. This way, immediately
resettable gestures are able to reset and recognize again, without having to wait for other slower preventable
gesture recognizers to reset.

When fast-clicking is disabled (e.g. when loading a desktop web page on a mobile form factor, or when the
viewport has been zoomed in), the blocking synthetic double tap gesture recognizer (that is, `WKContentView`'s
`_doubleTapGestureRecognizer`) is enabled, and adds itself as a dynamic failure requirement to the content
view's synthetic single tap gesture recognizer (`_singleTapGestureRecognizer`). In terms of the gesture
dependency graph, this causes the single tap gesture to form an edge with the double tap gesture, which ends up
uniting both deferring gesture recognizers under the same subgraph. This means UIWebTouchEventsGestureRecognizer,
which should be one of the gestures in the immediately resettable subgraph, is now connected to the rest of the
delayed resettable gestures, meaning that it cannot recognize until "slowly resettable" gestures such as the
tap-and-half text selection gesture have also been reset. This delay causes touch events to be dropped, as is
the case in this bug.

To fix this, simply quarantine the single tap and double tap gestures inside their own subgraph by introducing a
separate deferring gesture recognizer for them. When fast-clicking is enabled, this does not hinder the ability
for the single tap gesture to fire in rapid succession, since the double tap gesture is disabled (and thus, not
a part of the graph at all). when fast-clicking is disabled, then the double tap gesture will prevent the single
tap gesture from being immediately reset anyways, due to the direct failure requirement between the double and
single tap gesture.

Doing this ensures that no other immediately resettable gesture (`UIWebTouchEventsGestureRecognizer` included)
is accidentally blocked from immediately resetting due to being linked to the delayed resettable gestures by way
of the synthetic single and double tap gestures.

Test: fast/events/touch/ios/tap-and-half-when-viewport-is-not-responsive.html

* UIProcess/ios/WKContentViewInteraction.h:

Add a dedicated deferring gesture recognizer for the synthetic single tap and double tap gesture recognizers.

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView setUpInteraction]):
(-[WKContentView cleanUpInteraction]):

Use -_deferringGestureRecognizers instead of hard-coding logic for each deferring gesture.

(-[WKContentView _removeDefaultGestureRecognizers]): Ditto.
(-[WKContentView _addDefaultGestureRecognizers]): Ditto.
(-[WKContentView _deferringGestureRecognizers]):

We now have 3 distinct deferring gestures; instead of handling the three deferring gestures individually in
various places in this file, group them all behind a getter that returns an array of deferring gestures, and use
this instead.

(-[WKContentView _doneDeferringNativeGestures:]): Ditto.
(-[WKContentView gestureRecognizer:shouldRecognizeSimultaneouslyWithGestureRecognizer:]): Ditto.
(-[WKContentView deferringGestureRecognizer:shouldDeferOtherGestureRecognizer:]):

Partition the synthetic single tap and double tap gestures into their own subgraph.

LayoutTests:

Add a layout test that synthesizes a tap-and-half gesture over an element with active touch event listeners, and
verifies that the second half of the gesture (i.e. the pan gesture) dispatches touchstart, touchmove, and
touchend events.

* fast/events/touch/ios/tap-and-half-when-viewport-is-not-responsive-expected.txt: Added.
* fast/events/touch/ios/tap-and-half-when-viewport-is-not-responsive.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (261479 => 261480)


--- trunk/LayoutTests/ChangeLog	2020-05-11 15:43:00 UTC (rev 261479)
+++ trunk/LayoutTests/ChangeLog	2020-05-11 16:22:56 UTC (rev 261480)
@@ -1,3 +1,18 @@
+2020-05-11  Wenson Hsieh  <[email protected]>
+
+        REGRESSION (r253267): issues on touchstart/touchend/touchmove (pointerdown/pointerup/pointermove) events
+        https://bugs.webkit.org/show_bug.cgi?id=211521
+        <rdar://problem/62942374>
+
+        Reviewed by Darin Adler.
+
+        Add a layout test that synthesizes a tap-and-half gesture over an element with active touch event listeners, and
+        verifies that the second half of the gesture (i.e. the pan gesture) dispatches touchstart, touchmove, and
+        touchend events.
+
+        * fast/events/touch/ios/tap-and-half-when-viewport-is-not-responsive-expected.txt: Added.
+        * fast/events/touch/ios/tap-and-half-when-viewport-is-not-responsive.html: Added.
+
 2020-05-11  Jason Lawrence  <[email protected]>
 
         [ Mac wk1 ] http/tests/security/_javascript_URL/xss-DENIED-to-_javascript_-url-in-foreign-domain-subframe.html is flaky failing.

Added: trunk/LayoutTests/fast/events/touch/ios/tap-and-half-when-viewport-is-not-responsive-expected.txt (0 => 261480)


--- trunk/LayoutTests/fast/events/touch/ios/tap-and-half-when-viewport-is-not-responsive-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/tap-and-half-when-viewport-is-not-responsive-expected.txt	2020-05-11 16:22:56 UTC (rev 261480)
@@ -0,0 +1,15 @@
+This test verifies that a tap-and-half gesture dispatches touch events when the page is zoomed in. To manually run the test, tap the red square and then immediately perform a pan gesture starting in the same location as the tap. The resulting events should consist of 'touchstart', 'touchend', 'touchstart', one or more 'touchmove', and finally a 'touchend'.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Observed touchstart
+PASS Observed touchend
+PASS Observed touchstart
+PASS Observed touchmove
+PASS Observed touchend
+PASS touchendCount became 2
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/events/touch/ios/tap-and-half-when-viewport-is-not-responsive.html (0 => 261480)


--- trunk/LayoutTests/fast/events/touch/ios/tap-and-half-when-viewport-is-not-responsive.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/tap-and-half-when-viewport-is-not-responsive.html	2020-05-11 16:22:56 UTC (rev 261480)
@@ -0,0 +1,54 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<meta name="viewport" content="initial-scale=0.6">
+<script src=""
+<script src=""
+<script>
+jsTestIsAsync = true;
+lastEventType = null;
+touchendCount = 0;
+
+function logEvent(event) {
+    if (event.type === lastEventType)
+        return;
+
+    lastEventType = event.type;
+    testPassed(`Observed ${event.type}`);
+    if (event.type === "touchend")
+        touchendCount++;
+}
+
+addEventListener("load", async () => {
+    const target = document.getElementById("target");
+    target.addEventListener("touchstart", logEvent, { passive : false });
+    target.addEventListener("touchmove", logEvent, { passive : false });
+    target.addEventListener("touchend", logEvent, { passive : false });
+
+    description("This test verifies that a tap-and-half gesture dispatches touch events when the page is zoomed in. To manually run the test, tap the red square and then immediately perform a pan gesture starting in the same location as the tap. The resulting events should consist of 'touchstart', 'touchend', 'touchstart', one or more 'touchmove', and finally a 'touchend'.");
+
+    await UIHelper.activateAt(100, 100);
+    await UIHelper.sendEventStream(new UIHelper.EventStreamBuilder()
+        .begin(100, 100)
+        .move(100, 200, 0.5)
+        .end()
+        .takeResult());
+
+    await new Promise(resolve => shouldBecomeEqual("touchendCount", "2", resolve));
+    finishJSTest();
+});
+</script>
+<style>
+#target {
+    width: 400px;
+    height: 400px;
+    background-color: red;
+}
+</style>
+</head>
+<body>
+<div id="target"></div>
+<p id="description"></p>
+<p id="console"></p>
+</body>
+</html>

Modified: trunk/Source/WebKit/ChangeLog (261479 => 261480)


--- trunk/Source/WebKit/ChangeLog	2020-05-11 15:43:00 UTC (rev 261479)
+++ trunk/Source/WebKit/ChangeLog	2020-05-11 16:22:56 UTC (rev 261480)
@@ -1,3 +1,72 @@
+2020-05-11  Wenson Hsieh  <[email protected]>
+
+        REGRESSION (r253267): issues on touchstart/touchend/touchmove (pointerdown/pointerup/pointermove) events
+        https://bugs.webkit.org/show_bug.cgi?id=211521
+        <rdar://problem/62942374>
+
+        Reviewed by Darin Adler.
+
+        As a brief refresher, deferring gesture recognizers allow us to handle otherwise blocking touch events
+        asynchronously by having all preventable native gesture recognizers require the deferring gesture recognizer to
+        fail; we only fail the deferring gesture recognizer once the web process has handled the touch event, and did
+        not call `preventDefault()`.
+
+        These additional failure requirements can cause preventable gestures to be linked together in the same gesture
+        dependency subgraph; since each subgraph is reset only once all gestures in the subgraph have failed or ended,
+        this might cause some gestures to be reset after a delay (rather than being reset immediately). To mitigate
+        this, we divide the set of preventable gestures into multiple (currently, 2) subgraphs: one for gestures that
+        are reset after a delay, and another for gestures that are immediately resettable. This way, immediately
+        resettable gestures are able to reset and recognize again, without having to wait for other slower preventable
+        gesture recognizers to reset.
+
+        When fast-clicking is disabled (e.g. when loading a desktop web page on a mobile form factor, or when the
+        viewport has been zoomed in), the blocking synthetic double tap gesture recognizer (that is, `WKContentView`'s
+        `_doubleTapGestureRecognizer`) is enabled, and adds itself as a dynamic failure requirement to the content
+        view's synthetic single tap gesture recognizer (`_singleTapGestureRecognizer`). In terms of the gesture
+        dependency graph, this causes the single tap gesture to form an edge with the double tap gesture, which ends up
+        uniting both deferring gesture recognizers under the same subgraph. This means UIWebTouchEventsGestureRecognizer,
+        which should be one of the gestures in the immediately resettable subgraph, is now connected to the rest of the
+        delayed resettable gestures, meaning that it cannot recognize until "slowly resettable" gestures such as the
+        tap-and-half text selection gesture have also been reset. This delay causes touch events to be dropped, as is
+        the case in this bug.
+
+        To fix this, simply quarantine the single tap and double tap gestures inside their own subgraph by introducing a
+        separate deferring gesture recognizer for them. When fast-clicking is enabled, this does not hinder the ability
+        for the single tap gesture to fire in rapid succession, since the double tap gesture is disabled (and thus, not
+        a part of the graph at all). when fast-clicking is disabled, then the double tap gesture will prevent the single
+        tap gesture from being immediately reset anyways, due to the direct failure requirement between the double and
+        single tap gesture.
+
+        Doing this ensures that no other immediately resettable gesture (`UIWebTouchEventsGestureRecognizer` included)
+        is accidentally blocked from immediately resetting due to being linked to the delayed resettable gestures by way
+        of the synthetic single and double tap gestures.
+
+        Test: fast/events/touch/ios/tap-and-half-when-viewport-is-not-responsive.html
+
+        * UIProcess/ios/WKContentViewInteraction.h:
+
+        Add a dedicated deferring gesture recognizer for the synthetic single tap and double tap gesture recognizers.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView setUpInteraction]):
+        (-[WKContentView cleanUpInteraction]):
+
+        Use -_deferringGestureRecognizers instead of hard-coding logic for each deferring gesture.
+
+        (-[WKContentView _removeDefaultGestureRecognizers]): Ditto.
+        (-[WKContentView _addDefaultGestureRecognizers]): Ditto.
+        (-[WKContentView _deferringGestureRecognizers]):
+
+        We now have 3 distinct deferring gestures; instead of handling the three deferring gestures individually in
+        various places in this file, group them all behind a getter that returns an array of deferring gestures, and use
+        this instead.
+
+        (-[WKContentView _doneDeferringNativeGestures:]): Ditto.
+        (-[WKContentView gestureRecognizer:shouldRecognizeSimultaneouslyWithGestureRecognizer:]): Ditto.
+        (-[WKContentView deferringGestureRecognizer:shouldDeferOtherGestureRecognizer:]):
+
+        Partition the synthetic single tap and double tap gestures into their own subgraph.
+
 2020-05-11  Per Arne Vollan  <[email protected]>
 
         Unreviewed, reverting r261296.

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h (261479 => 261480)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h	2020-05-11 15:43:00 UTC (rev 261479)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h	2020-05-11 16:22:56 UTC (rev 261480)
@@ -211,6 +211,7 @@
 #if ENABLE(IOS_TOUCH_EVENTS)
     RetainPtr<WKDeferringGestureRecognizer> _deferringGestureRecognizerForImmediatelyResettableGestures;
     RetainPtr<WKDeferringGestureRecognizer> _deferringGestureRecognizerForDelayedResettableGestures;
+    RetainPtr<WKDeferringGestureRecognizer> _deferringGestureRecognizerForSyntheticTapGestures;
 #endif
     RetainPtr<UIWebTouchEventsGestureRecognizer> _touchEventGestureRecognizer;
 

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (261479 => 261480)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2020-05-11 15:43:00 UTC (rev 261479)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2020-05-11 16:22:56 UTC (rev 261480)
@@ -761,13 +761,17 @@
 #if ENABLE(IOS_TOUCH_EVENTS)
     _deferringGestureRecognizerForImmediatelyResettableGestures = adoptNS([[WKDeferringGestureRecognizer alloc] initWithDeferringGestureDelegate:self]);
     [_deferringGestureRecognizerForImmediatelyResettableGestures setName:@"Touch event deferrer (immediate reset)"];
-    [_deferringGestureRecognizerForImmediatelyResettableGestures setDelegate:self];
-    [self addGestureRecognizer:_deferringGestureRecognizerForImmediatelyResettableGestures.get()];
 
     _deferringGestureRecognizerForDelayedResettableGestures = adoptNS([[WKDeferringGestureRecognizer alloc] initWithDeferringGestureDelegate:self]);
     [_deferringGestureRecognizerForDelayedResettableGestures setName:@"Touch event deferrer (delayed reset)"];
-    [_deferringGestureRecognizerForDelayedResettableGestures setDelegate:self];
-    [self addGestureRecognizer:_deferringGestureRecognizerForDelayedResettableGestures.get()];
+
+    _deferringGestureRecognizerForSyntheticTapGestures = adoptNS([[WKDeferringGestureRecognizer alloc] initWithDeferringGestureDelegate:self]);
+    [_deferringGestureRecognizerForSyntheticTapGestures setName:@"Touch event deferrer (synthetic tap)"];
+
+    for (WKDeferringGestureRecognizer *gesture in self._deferringGestureRecognizers) {
+        gesture.delegate = self;
+        [self addGestureRecognizer:gesture];
+    }
 #endif
 
     _touchEventGestureRecognizer = adoptNS([[UIWebTouchEventsGestureRecognizer alloc] initWithTarget:self action:@selector(_webTouchEventsRecognized:) touchDelegate:self]);
@@ -934,11 +938,10 @@
     [self removeGestureRecognizer:_touchEventGestureRecognizer.get()];
 
 #if ENABLE(IOS_TOUCH_EVENTS)
-    [_deferringGestureRecognizerForImmediatelyResettableGestures setDelegate:nil];
-    [self removeGestureRecognizer:_deferringGestureRecognizerForImmediatelyResettableGestures.get()];
-
-    [_deferringGestureRecognizerForDelayedResettableGestures setDelegate:nil];
-    [self removeGestureRecognizer:_deferringGestureRecognizerForDelayedResettableGestures.get()];
+    for (WKDeferringGestureRecognizer *gesture in self._deferringGestureRecognizers) {
+        gesture.delegate = nil;
+        [self removeGestureRecognizer:gesture];
+    }
 #endif
 
 #if HAVE(UIKIT_WITH_MOUSE_SUPPORT)
@@ -1053,8 +1056,8 @@
 - (void)_removeDefaultGestureRecognizers
 {
 #if ENABLE(IOS_TOUCH_EVENTS)
-    [self removeGestureRecognizer:_deferringGestureRecognizerForImmediatelyResettableGestures.get()];
-    [self removeGestureRecognizer:_deferringGestureRecognizerForDelayedResettableGestures.get()];
+    for (WKDeferringGestureRecognizer *gesture in self._deferringGestureRecognizers)
+        [self removeGestureRecognizer:gesture];
 #endif
     [self removeGestureRecognizer:_touchEventGestureRecognizer.get()];
     [self removeGestureRecognizer:_singleTapGestureRecognizer.get()];
@@ -1081,8 +1084,8 @@
 - (void)_addDefaultGestureRecognizers
 {
 #if ENABLE(IOS_TOUCH_EVENTS)
-    [self addGestureRecognizer:_deferringGestureRecognizerForImmediatelyResettableGestures.get()];
-    [self addGestureRecognizer:_deferringGestureRecognizerForDelayedResettableGestures.get()];
+    for (WKDeferringGestureRecognizer *gesture in self._deferringGestureRecognizers)
+        [self addGestureRecognizer:gesture];
 #endif
     [self addGestureRecognizer:_touchEventGestureRecognizer.get()];
     [self addGestureRecognizer:_singleTapGestureRecognizer.get()];
@@ -1656,13 +1659,27 @@
 
 #if ENABLE(IOS_TOUCH_EVENTS)
 
+- (NSArray<WKDeferringGestureRecognizer *> *)_deferringGestureRecognizers
+{
+    WKDeferringGestureRecognizer *recognizers[3];
+    NSUInteger count = 0;
+    auto add = [&] (const RetainPtr<WKDeferringGestureRecognizer>& recognizer) {
+        if (recognizer)
+            recognizers[count++] = recognizer.get();
+    };
+    add(_deferringGestureRecognizerForImmediatelyResettableGestures);
+    add(_deferringGestureRecognizerForDelayedResettableGestures);
+    add(_deferringGestureRecognizerForSyntheticTapGestures);
+    return [NSArray arrayWithObjects:recognizers count:count];
+}
+
 - (void)_doneDeferringNativeGestures:(BOOL)preventNativeGestures
 {
-    [_deferringGestureRecognizerForImmediatelyResettableGestures setDefaultPrevented:preventNativeGestures];
-    [_deferringGestureRecognizerForDelayedResettableGestures setDefaultPrevented:preventNativeGestures];
+    for (WKDeferringGestureRecognizer *gesture in self._deferringGestureRecognizers)
+        [gesture setDefaultPrevented:preventNativeGestures];
 }
 
-#endif
+#endif // ENABLE(IOS_TOUCH_EVENTS)
 
 static NSValue *nsSizeForTapHighlightBorderRadius(WebCore::IntSize borderRadius, CGFloat borderRadiusScale)
 {
@@ -2041,11 +2058,10 @@
 - (BOOL)gestureRecognizer:(UIGestureRecognizer *)gestureRecognizer shouldRecognizeSimultaneouslyWithGestureRecognizer:(UIGestureRecognizer*)otherGestureRecognizer
 {
 #if ENABLE(IOS_TOUCH_EVENTS)
-    if (isSamePair(gestureRecognizer, otherGestureRecognizer, _touchEventGestureRecognizer.get(), _deferringGestureRecognizerForImmediatelyResettableGestures.get()))
-        return YES;
-
-    if (isSamePair(gestureRecognizer, otherGestureRecognizer, _touchEventGestureRecognizer.get(), _deferringGestureRecognizerForDelayedResettableGestures.get()))
-        return YES;
+    for (WKDeferringGestureRecognizer *gesture in self._deferringGestureRecognizers) {
+        if (isSamePair(gestureRecognizer, otherGestureRecognizer, _touchEventGestureRecognizer.get(), gesture))
+            return YES;
+    }
 #endif
 
     if ([gestureRecognizer isKindOfClass:WKDeferringGestureRecognizer.class] && [otherGestureRecognizer isKindOfClass:WKDeferringGestureRecognizer.class])
@@ -7100,6 +7116,9 @@
         return NO;
     };
 
+    if (gestureRecognizer == _doubleTapGestureRecognizer || gestureRecognizer == _singleTapGestureRecognizer)
+        return deferringGestureRecognizer == _deferringGestureRecognizerForSyntheticTapGestures;
+
     if (mayDelayResetOfContainingSubgraph(gestureRecognizer))
         return deferringGestureRecognizer == _deferringGestureRecognizerForDelayedResettableGestures;
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to