Changeset 247820 in webkit

Timestamp:

Jul 25, 2019 8:35:17 AM (5 years ago)

Author:

Truitt Savell

Message:

Unreviewed, rolling out r247817.

Broke 14 http/tests/paymentrequest/ tests. on iOS

Reverted changeset:

"Unable to tap/double tap to open files/folders in Google
Drive in Safari"
​https://bugs.webkit.org/show_bug.cgi?id=200096
​https://trac.webkit.org/changeset/247817

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/events/touch/ios/double-tap-for-double-click3-expected.txt (modified) (1 diff)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/Shared/WebPageCreationParameters.cpp (modified) (2 diffs)
• Source/WebKit/Shared/WebPageCreationParameters.h (modified) (1 diff)
• Source/WebKit/UIProcess/PageClient.h (modified) (1 diff)
• Source/WebKit/UIProcess/WebPageProxy.cpp (modified) (1 diff)
• Source/WebKit/UIProcess/WebPageProxy.h (modified) (1 diff)
• Source/WebKit/UIProcess/ios/PageClientImplIOS.h (modified) (1 diff)
• Source/WebKit/UIProcess/ios/PageClientImplIOS.mm (modified) (1 diff)
• Source/WebKit/UIProcess/ios/WKContentViewInteraction.h (modified) (2 diffs)
• Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (modified) (10 diffs)
• Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm (modified) (1 diff)
• Source/WebKit/WebProcess/WebPage/WebPage.cpp (modified) (1 diff)
• Source/WebKit/WebProcess/WebPage/WebPage.h (modified) (3 diffs)
• Source/WebKit/WebProcess/WebPage/WebPage.messages.in (modified) (1 diff)
• Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (modified) (7 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-07-25  Truitt Savell  <tsavell@apple.com>
+
+        Unreviewed, rolling out r247817.
+
+        Broke 14 http/tests/paymentrequest/ tests. on iOS
+
+        Reverted changeset:
+
+        "Unable to tap/double tap to open files/folders in Google
+        Drive in Safari"
+        https://bugs.webkit.org/show_bug.cgi?id=200096
+        https://trac.webkit.org/changeset/247817
+
 2019-07-24  Zalan Bujtas  <zalan@apple.com>
 

trunk/LayoutTests/fast/events/touch/ios/double-tap-for-double-click3-expected.txt

@@ -1 +1 @@
 PASS if 'double click' text is NOT shown below.
-[click]
+[click][click]

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-07-25  Truitt Savell  <tsavell@apple.com>
+
+        Unreviewed, rolling out r247817.
+
+        Broke 14 http/tests/paymentrequest/ tests. on iOS
+
+        Reverted changeset:
+
+        "Unable to tap/double tap to open files/folders in Google
+        Drive in Safari"
+        https://bugs.webkit.org/show_bug.cgi?id=200096
+        https://trac.webkit.org/changeset/247817
+
 2019-07-24  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebKit/Shared/WebPageCreationParameters.cpp

@@ -99 +99 @@
     encoder << keyboardIsAttached;
     encoder << canShowWhileLocked;
+    encoder << doubleTapForDoubleClickDelay;
+    encoder << doubleTapForDoubleClickRadius;
     encoder << overrideViewportArguments;
 #endif
@@ -291 +293 @@
     if (!decoder.decode(parameters.canShowWhileLocked))
         return WTF::nullopt;
+    if (!decoder.decode(parameters.doubleTapForDoubleClickDelay))
+        return WTF::nullopt;
+    if (!decoder.decode(parameters.doubleTapForDoubleClickRadius))
+        return WTF::nullopt;
 
     Optional<Optional<WebCore::ViewportArguments>> overrideViewportArguments;

trunk/Source/WebKit/Shared/WebPageCreationParameters.h

@@ -159 +159 @@
     bool keyboardIsAttached { false };
     bool canShowWhileLocked { false };
+    Seconds doubleTapForDoubleClickDelay { 350_ms };
+    float doubleTapForDoubleClickRadius { 45 };
     Optional<WebCore::ViewportArguments> overrideViewportArguments;
 #endif

trunk/Source/WebKit/UIProcess/PageClient.h

@@ -414 +414 @@
 
     virtual void handleAutocorrectionContext(const WebAutocorrectionContext&) = 0;
+
+    virtual Seconds doubleTapForDoubleClickDelay() = 0;
+    virtual float doubleTapForDoubleClickRadius() = 0;
 #endif
 

trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -7205 +7205 @@
     parameters.overrideViewportArguments = m_overrideViewportArguments;
     parameters.canShowWhileLocked = m_configuration->canShowWhileLocked();
+    parameters.doubleTapForDoubleClickDelay = pageClient().doubleTapForDoubleClickDelay();
+    parameters.doubleTapForDoubleClickRadius = pageClient().doubleTapForDoubleClickRadius();
 #endif
 

trunk/Source/WebKit/UIProcess/WebPageProxy.h

@@ -1226 +1226 @@
     void handleTap(const WebCore::FloatPoint&, OptionSet<WebKit::WebEvent::Modifier>, uint64_t layerTreeTransactionIdAtLastTouchStart);
     void didRecognizeLongPress();
-    void handleDoubleTapForDoubleClickAtPoint(const WebCore::IntPoint&, OptionSet<WebEvent::Modifier>, uint64_t layerTreeTransactionIdAtLastTouchStart);
 
     void inspectorNodeSearchMovedToPosition(const WebCore::FloatPoint&);

trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.h

@@ -244 +244 @@
     void handleAutocorrectionContext(const WebAutocorrectionContext&) final;
 
+    Seconds doubleTapForDoubleClickDelay() final;
+    float doubleTapForDoubleClickRadius() final;
+
     void didFinishProcessingAllPendingMouseEvents() final { }
 

trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.mm

@@ -843 +843 @@
 #endif
 
+Seconds PageClientImpl::doubleTapForDoubleClickDelay()
+{
+    return Seconds { [m_contentView _doubleTapForDoubleClickDelay] };
+}
+
+float PageClientImpl::doubleTapForDoubleClickRadius()
+{
+    return [m_contentView _doubleTapForDoubleClickRadius];
+}
+
 #if USE(QUICK_LOOK)
 void PageClientImpl::requestPasswordForQuickLookDocument(const String& fileName, WTF::Function<void(const String&)>&& completionHandler)

trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h

@@ -217 +217 @@
     RetainPtr<WKSyntheticTapGestureRecognizer> _doubleTapGestureRecognizer;
     RetainPtr<UITapGestureRecognizer> _nonBlockingDoubleTapGestureRecognizer;
-    RetainPtr<UITapGestureRecognizer> _doubleTapGestureRecognizerForDoubleClick;
     RetainPtr<UITapGestureRecognizer> _twoFingerDoubleTapGestureRecognizer;
     RetainPtr<UITapGestureRecognizer> _twoFingerSingleTapGestureRecognizer;
@@ -482 +481 @@
 - (void)_becomeFirstResponderWithSelectionMovingForward:(BOOL)selectingForward completionHandler:(void (^)(BOOL didBecomeFirstResponder))completionHandler;
 - (void)_setDoubleTapGesturesEnabled:(BOOL)enabled;
+- (double)_doubleTapForDoubleClickDelay;
+- (float)_doubleTapForDoubleClickRadius;
 #if ENABLE(DATA_DETECTION)
 - (NSArray *)_dataDetectionResults;

trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

@@ -689 +689 @@
 }
 
+- (void)_ensureNonBlockingDoubleTapGestureRecognizer
+{
+    if (_nonBlockingDoubleTapGestureRecognizer)
+        return;
+    _nonBlockingDoubleTapGestureRecognizer = adoptNS([[UITapGestureRecognizer alloc] initWithTarget:self action:@selector(_nonBlockingDoubleTapRecognized:)]);
+    [_nonBlockingDoubleTapGestureRecognizer setNumberOfTapsRequired:2];
+    [_nonBlockingDoubleTapGestureRecognizer setDelegate:self];
+    [_nonBlockingDoubleTapGestureRecognizer setEnabled:NO];
+}
+
 - (void)setupInteraction
 {
@@ -735 +745 @@
     [self addGestureRecognizer:_singleTapGestureRecognizer.get()];
 
-    _nonBlockingDoubleTapGestureRecognizer = adoptNS([[UITapGestureRecognizer alloc] initWithTarget:self action:@selector(_nonBlockingDoubleTapRecognized:)]);
-    [_nonBlockingDoubleTapGestureRecognizer setNumberOfTapsRequired:2];
-    [_nonBlockingDoubleTapGestureRecognizer setDelegate:self];
-    [_nonBlockingDoubleTapGestureRecognizer setEnabled:NO];
+    [self _ensureNonBlockingDoubleTapGestureRecognizer];
     [self addGestureRecognizer:_nonBlockingDoubleTapGestureRecognizer.get()];
-
-    _doubleTapGestureRecognizerForDoubleClick = adoptNS([[UITapGestureRecognizer alloc] initWithTarget:self action:@selector(_doubleTapRecognizedForDoubleClick:)]);
-    [_doubleTapGestureRecognizerForDoubleClick setNumberOfTapsRequired:2];
-    [_doubleTapGestureRecognizerForDoubleClick setDelegate:self];
-    [self addGestureRecognizer:_doubleTapGestureRecognizerForDoubleClick.get()];
 
     [self _createAndConfigureDoubleTapGestureRecognizer];
@@ -893 +895 @@
     [self removeGestureRecognizer:_nonBlockingDoubleTapGestureRecognizer.get()];
 
-    [_doubleTapGestureRecognizerForDoubleClick setDelegate:nil];
-    [self removeGestureRecognizer:_doubleTapGestureRecognizerForDoubleClick.get()];
-
     [_twoFingerDoubleTapGestureRecognizer setDelegate:nil];
     [self removeGestureRecognizer:_twoFingerDoubleTapGestureRecognizer.get()];
@@ -972 +971 @@
     [self removeGestureRecognizer:_doubleTapGestureRecognizer.get()];
     [self removeGestureRecognizer:_nonBlockingDoubleTapGestureRecognizer.get()];
-    [self removeGestureRecognizer:_doubleTapGestureRecognizerForDoubleClick.get()];
     [self removeGestureRecognizer:_twoFingerDoubleTapGestureRecognizer.get()];
     [self removeGestureRecognizer:_twoFingerSingleTapGestureRecognizer.get()];
@@ -992 +990 @@
     [self addGestureRecognizer:_doubleTapGestureRecognizer.get()];
     [self addGestureRecognizer:_nonBlockingDoubleTapGestureRecognizer.get()];
-    [self addGestureRecognizer:_doubleTapGestureRecognizerForDoubleClick.get()];
     [self addGestureRecognizer:_twoFingerDoubleTapGestureRecognizer.get()];
     [self addGestureRecognizer:_twoFingerSingleTapGestureRecognizer.get()];
@@ -1896 +1893 @@
         return YES;
 
-    if (isSamePair(gestureRecognizer, otherGestureRecognizer, _nonBlockingDoubleTapGestureRecognizer.get(), _doubleTapGestureRecognizerForDoubleClick.get()))
-        return YES;
-
-    if (isSamePair(gestureRecognizer, otherGestureRecognizer, _doubleTapGestureRecognizer.get(), _doubleTapGestureRecognizerForDoubleClick.get()))
-        return YES;
-
     return NO;
 }
@@ -2094 +2085 @@
         || gestureRecognizer == _doubleTapGestureRecognizer
         || gestureRecognizer == _nonBlockingDoubleTapGestureRecognizer
-        || gestureRecognizer == _doubleTapGestureRecognizerForDoubleClick
         || gestureRecognizer == _twoFingerDoubleTapGestureRecognizer) {
 
@@ -2331 +2321 @@
         break;
     }
-}
-
-- (void)_doubleTapRecognizedForDoubleClick:(UITapGestureRecognizer *)gestureRecognizer
-{
-    _page->handleDoubleTapForDoubleClickAtPoint(WebCore::IntPoint(gestureRecognizer.location), WebKit::webEventModifierFlags(gestureRecognizerModifierFlags(gestureRecognizer)), _layerTreeTransactionIdAtLastTouchStart);
 }
 
@@ -3914 +3899 @@
 - (void)_didStartProvisionalLoadForMainFrame
 {
-    // Reset the double tap gesture recognizer to prevent any double click that is in the process of being recognized.
-    [_doubleTapGestureRecognizerForDoubleClick setEnabled:NO];
-    [_doubleTapGestureRecognizerForDoubleClick setEnabled:YES];
-    // We also need to disable the double-tap gesture recognizers that are enabled for double-tap-to-zoom and which
+    // We need to disable the double-tap gesture recognizers that are enabled for double-tap-to-zoom and which
     // are enabled when a single tap is first recognized. This avoids tests running in sequence and simulating taps
     // in the same location to trigger double-tap recognition.
@@ -3985 +3967 @@
     [_nonBlockingDoubleTapGestureRecognizer setEnabled:!enabled];
     [self _resetIsDoubleTapPending];
+}
+
+- (double)_doubleTapForDoubleClickDelay
+{
+    static double doubleTapForDoubleClickDelay = 350;
+    static dispatch_once_t onceToken;
+    dispatch_once(&onceToken, ^{
+        [self _ensureNonBlockingDoubleTapGestureRecognizer];
+        doubleTapForDoubleClickDelay = [_nonBlockingDoubleTapGestureRecognizer maximumIntervalBetweenSuccessiveTaps];
+    });
+    return doubleTapForDoubleClickDelay;
+}
+
+- (float)_doubleTapForDoubleClickRadius
+{
+    static float doubleTapForDoubleClickRadius = 45;
+    static dispatch_once_t onceToken;
+    dispatch_once(&onceToken, ^{
+        [self _ensureNonBlockingDoubleTapGestureRecognizer];
+        doubleTapForDoubleClickRadius = [_nonBlockingDoubleTapGestureRecognizer allowableMovement];
+    });
+    return doubleTapForDoubleClickRadius;
 }
 

trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm

@@ -839 +839 @@
 }
 
-void WebPageProxy::handleDoubleTapForDoubleClickAtPoint(const WebCore::IntPoint& point, OptionSet<WebEvent::Modifier> modifiers, uint64_t layerTreeTransactionIdAtLastTouchStart)
-{
-    process().send(Messages::WebPage::HandleDoubleTapForDoubleClickAtPoint(point, modifiers, layerTreeTransactionIdAtLastTouchStart), m_pageID);
-}
-
 void WebPageProxy::inspectorNodeSearchMovedToPosition(const WebCore::FloatPoint& position)
 {

trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp

@@ -417 +417 @@
     , m_keyboardIsAttached(parameters.keyboardIsAttached)
     , m_canShowWhileLocked(parameters.canShowWhileLocked)
+    , m_doubleTapForDoubleClickDelay(parameters.doubleTapForDoubleClickDelay)
+    , m_doubleTapForDoubleClickRadius(parameters.doubleTapForDoubleClickRadius)
 #endif
     , m_layerVolatilityTimer(*this, &WebPage::layerVolatilityTimerFired)

trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

@@ -636 +636 @@
     void tapHighlightAtPosition(uint64_t requestID, const WebCore::FloatPoint&);
     void didRecognizeLongPress();
-    void handleDoubleTapForDoubleClickAtPoint(const WebCore::IntPoint&, OptionSet<WebKit::WebEvent::Modifier>, uint64_t lastLayerTreeTransactionId);
+    bool handlePotentialDoubleTapForDoubleClickAtPoint(OptionSet<WebKit::WebEvent::Modifier>, uint64_t lastLayerTreeTransactionId);
 
     void inspectorNodeSearchMovedToPosition(const WebCore::FloatPoint&);
@@ -1866 +1866 @@
     bool m_canShowWhileLocked { false };
     bool m_inDynamicSizeUpdate { false };
+    Seconds m_doubleTapForDoubleClickDelay { 350_ms };
+    float m_doubleTapForDoubleClickRadius { 45 };
     HashMap<std::pair<WebCore::IntSize, double>, WebCore::IntPoint> m_dynamicSizeUpdateHistory;
     RefPtr<WebCore::Node> m_pendingSyntheticClickNode;
@@ -1876 +1878 @@
     double m_lastTransactionPageScaleFactor { 0 };
     uint64_t m_lastTransactionIDWithScaleChange { 0 };
+    Optional<MonotonicTime> m_lastCommittedTapTimestamp;
+    Optional<WebCore::FloatPoint> m_lastCommittedTapLocation;
 
     CompletionHandler<void(InteractionInformationAtPosition&&)> m_pendingSynchronousPositionInformationReply;

trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in

@@ -58 +58 @@
     TapHighlightAtPosition(uint64_t requestID, WebCore::FloatPoint point)
     DidRecognizeLongPress()
-    HandleDoubleTapForDoubleClickAtPoint(WebCore::IntPoint point, OptionSet<WebKit::WebEvent::Modifier> modifiers, uint64_t lastLayerTreeTransactionId)
     InspectorNodeSearchMovedToPosition(WebCore::FloatPoint point)
     InspectorNodeSearchEndedAtPosition(WebCore::FloatPoint point)

trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

@@ -833 +833 @@
 }
 
-void WebPage::handleDoubleTapForDoubleClickAtPoint(const IntPoint& point, OptionSet<WebEvent::Modifier> modifiers, uint64_t lastLayerTreeTransactionId)
-{
+bool WebPage::handlePotentialDoubleTapForDoubleClickAtPoint(OptionSet<WebEvent::Modifier> modifiers, uint64_t lastLayerTreeTransactionId)
+{
+    if (!m_lastCommittedTapTimestamp || !m_lastCommittedTapLocation)
+        return false;
+
+    auto millisecondsSinceLastTap = (MonotonicTime::now() - *m_lastCommittedTapTimestamp).milliseconds();
+    if (millisecondsSinceLastTap > m_doubleTapForDoubleClickDelay.milliseconds())
+        return false;
+
+    auto distanceBetweenTaps = sqrtf(pow(m_potentialTapLocation.x() - m_lastCommittedTapLocation->x(), 2) + pow(m_potentialTapLocation.y() - m_lastCommittedTapLocation->y(), 2));
+    if (distanceBetweenTaps > m_doubleTapForDoubleClickRadius)
+        return false;
+
     FloatPoint adjustedPoint;
-    auto* nodeRespondingToDoubleClick = m_page->mainFrame().nodeRespondingToDoubleClickEvent(point, adjustedPoint);
+    auto* nodeRespondingToDoubleClick = m_page->mainFrame().nodeRespondingToDoubleClickEvent(m_potentialTapLocation, adjustedPoint);
     if (!nodeRespondingToDoubleClick)
-        return;
+        return false;
 
     auto* frameRespondingToDoubleClick = nodeRespondingToDoubleClick->document().frame();
     if (!frameRespondingToDoubleClick || lastLayerTreeTransactionId < WebFrame::fromCoreFrame(*frameRespondingToDoubleClick)->firstLayerTreeTransactionIDAfterDidCommitLoad())
-        return;
+        return false;
 
     bool shiftKey = modifiers.contains(WebEvent::Modifier::ShiftKey);
@@ -851 +862 @@
     nodeRespondingToDoubleClick->document().frame()->eventHandler().handleMousePressEvent(PlatformMouseEvent(roundedAdjustedPoint, roundedAdjustedPoint, LeftButton, PlatformEvent::MousePressed, 2, shiftKey, ctrlKey, altKey, metaKey, WallTime::now(), 0, WebCore::NoTap));
     if (m_isClosed)
-        return;
+        return false;
     nodeRespondingToDoubleClick->document().frame()->eventHandler().handleMouseReleaseEvent(PlatformMouseEvent(roundedAdjustedPoint, roundedAdjustedPoint, LeftButton, PlatformEvent::MouseReleased, 2, shiftKey, ctrlKey, altKey, metaKey, WallTime::now(), 0, WebCore::NoTap));
+    return true;
 }
 
@@ -1073 +1085 @@
 void WebPage::commitPotentialTap(OptionSet<WebEvent::Modifier> modifiers, uint64_t lastLayerTreeTransactionId, WebCore::PointerID pointerId)
 {
+    auto currentPotentialTapLocation = m_potentialTapLocation; 
+    auto updateLastCommittedLocationAndTimestamp = [&] {
+        m_lastCommittedTapTimestamp = MonotonicTime::now();
+        m_lastCommittedTapLocation = currentPotentialTapLocation;
+    };
+
     auto invalidTargetForSingleClick = !m_potentialTapNode;
     if (!invalidTargetForSingleClick) {
@@ -1081 +1099 @@
     }
     if (invalidTargetForSingleClick) {
+        // When the node has no click eventlistener, but it may have a dblclick one.
+        handlePotentialDoubleTapForDoubleClickAtPoint(modifiers, lastLayerTreeTransactionId);
         commitPotentialTapFailed();
-        return;
+        return updateLastCommittedLocationAndTimestamp();
     }
 
@@ -1091 +1111 @@
     if (!frameRespondingToClick || lastLayerTreeTransactionId < WebFrame::fromCoreFrame(*frameRespondingToClick)->firstLayerTreeTransactionIDAfterDidCommitLoad()) {
         commitPotentialTapFailed();
-        return;
+        return updateLastCommittedLocationAndTimestamp();
     }
 
@@ -1102 +1122 @@
         } else
 #endif
-            handleSyntheticClick(*nodeRespondingToClick, adjustedPoint, modifiers, pointerId);
+        {
+            if (handlePotentialDoubleTapForDoubleClickAtPoint(modifiers, lastLayerTreeTransactionId))
+                commitPotentialTapFailed();
+            else
+                handleSyntheticClick(*nodeRespondingToClick, adjustedPoint, modifiers, pointerId);
+        }
     } else
         commitPotentialTapFailed();
@@ -1109 +1134 @@
     m_potentialTapLocation = FloatPoint();
     m_potentialTapSecurityOrigin = nullptr;
+    return updateLastCommittedLocationAndTimestamp();
 }