Title: [178980] trunk/Source/WebKit2
Revision
178980
Author
benja...@webkit.org
Date
2015-01-22 20:17:11 -0800 (Thu, 22 Jan 2015)

Log Message

[iOS][WK2] Ignore synthetic click initiated on a previous page
https://bugs.webkit.org/show_bug.cgi?id=139556
rdar://problem/18586008

Patch by Benjamin Poulain <bpoul...@apple.com> on 2015-01-22
Reviewed by David Kilzer.

Under the right accumulation of bad luck, a synthetic click could be generated
on a page that never asked for it.

What happened was:
1) Page Foo listen to touch event.
2) In response to touch events, Foo decides to navigate to page Bar
   *but* it does not cancel the native gesture recognizers with preventDefault().
3) Page Bar loads.
4) The native gesture recognizer ask us to click, we forward that to the WebProcess
   and generate a synthetic mouse event.
-> Bar receives a click.
If you are unlucky enough, it looks like a single tap does a double click.

There are two ways this patch avoids the problem:

First, in the UIProcess, we just cancel the HighlightLongPressGestureRecognizer
on didCommitLoadForMainFrame. This prevents "fast clicks" from accidentally going through.
This is not bullet proof because we get didCommitLoadForMainFrame asynchronously but
killing the timer and gesture seems like a good idea.

Second, we skip any tap command at the WebProcess level when it was issued for
the previous page. This is using the same principle we used for VisibleContentRect:
any input generated before the current layer tree commit is useless for the current page.

We track the commit ID in the UIProcess when we decide to do the tracking or not.
Note that we keep the ID regardless of the tracking state, it does not matter if we have sent
touch events, what matters is what content was on screen when the touch events were handled.

If the user interaction result in a tap, we send the commit ID along the tap commands.
In the WebProcess, we know the actual layer tree commit associated with the current page.
If the tap was generated with a layer ID preceding the current page, we fail the command.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::WebPageProxy):
(WebKit::WebPageProxy::handleTouchEventSynchronously):
(WebKit::WebPageProxy::resetState):
* UIProcess/WebPageProxy.h:
* UIProcess/ios/WKContentView.mm:
(-[WKContentView _didCommitLoadForMainFrame]):
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _attemptClickAtLocation:]):
* UIProcess/ios/WebPageProxyIOS.mm:
(WebKit::WebPageProxy::commitPotentialTap):
(WebKit::WebPageProxy::handleTap):
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::didCommitLoad):
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/WebPage.messages.in:
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::handleTap):
(WebKit::WebPage::commitPotentialTap):

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (178979 => 178980)


--- trunk/Source/WebKit2/ChangeLog	2015-01-23 04:06:40 UTC (rev 178979)
+++ trunk/Source/WebKit2/ChangeLog	2015-01-23 04:17:11 UTC (rev 178980)
@@ -1,3 +1,63 @@
+2015-01-22  Benjamin Poulain  <bpoul...@apple.com>
+
+        [iOS][WK2] Ignore synthetic click initiated on a previous page
+        https://bugs.webkit.org/show_bug.cgi?id=139556
+        rdar://problem/18586008
+
+        Reviewed by David Kilzer.
+
+        Under the right accumulation of bad luck, a synthetic click could be generated
+        on a page that never asked for it.
+
+        What happened was:
+        1) Page Foo listen to touch event.
+        2) In response to touch events, Foo decides to navigate to page Bar
+           *but* it does not cancel the native gesture recognizers with preventDefault().
+        3) Page Bar loads.
+        4) The native gesture recognizer ask us to click, we forward that to the WebProcess
+           and generate a synthetic mouse event.
+        -> Bar receives a click.
+        If you are unlucky enough, it looks like a single tap does a double click.
+
+        There are two ways this patch avoids the problem:
+
+        First, in the UIProcess, we just cancel the HighlightLongPressGestureRecognizer
+        on didCommitLoadForMainFrame. This prevents "fast clicks" from accidentally going through.
+        This is not bullet proof because we get didCommitLoadForMainFrame asynchronously but
+        killing the timer and gesture seems like a good idea.
+
+        Second, we skip any tap command at the WebProcess level when it was issued for
+        the previous page. This is using the same principle we used for VisibleContentRect:
+        any input generated before the current layer tree commit is useless for the current page.
+
+        We track the commit ID in the UIProcess when we decide to do the tracking or not.
+        Note that we keep the ID regardless of the tracking state, it does not matter if we have sent
+        touch events, what matters is what content was on screen when the touch events were handled.
+
+        If the user interaction result in a tap, we send the commit ID along the tap commands.
+        In the WebProcess, we know the actual layer tree commit associated with the current page.
+        If the tap was generated with a layer ID preceding the current page, we fail the command.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::WebPageProxy):
+        (WebKit::WebPageProxy::handleTouchEventSynchronously):
+        (WebKit::WebPageProxy::resetState):
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/ios/WKContentView.mm:
+        (-[WKContentView _didCommitLoadForMainFrame]):
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView _attemptClickAtLocation:]):
+        * UIProcess/ios/WebPageProxyIOS.mm:
+        (WebKit::WebPageProxy::commitPotentialTap):
+        (WebKit::WebPageProxy::handleTap):
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::didCommitLoad):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::handleTap):
+        (WebKit::WebPage::commitPotentialTap):
+
 2015-01-22  Chris Dumez  <cdu...@apple.com>
 
         [WK2][Cocoa] Drop WKWebViewConfiguration._featureCounterEnabled SPI.

Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp (178979 => 178980)


--- trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp	2015-01-23 04:06:40 UTC (rev 178979)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp	2015-01-23 04:17:11 UTC (rev 178980)
@@ -286,6 +286,7 @@
     , m_dynamicViewportSizeUpdateWaitingForTarget(false)
     , m_dynamicViewportSizeUpdateWaitingForLayerTreeCommit(false)
     , m_dynamicViewportSizeUpdateLayerTreeTransactionID(0)
+    , m_layerTreeTransactionIdAtLastTouchStart(0)
 #endif
     , m_geolocationPermissionRequestManager(*this)
     , m_notificationPermissionRequestManager(*this)
@@ -1782,8 +1783,10 @@
     if (!isValid())
         return;
 
-    if (event.type() == WebEvent::TouchStart)
+    if (event.type() == WebEvent::TouchStart) {
         m_isTrackingTouchEvents = shouldStartTrackingTouchEvents(event);
+        m_layerTreeTransactionIdAtLastTouchStart = downcast<RemoteLayerTreeDrawingAreaProxy>(*drawingArea()).lastCommittedLayerTreeTransactionID();
+    }
 
     if (!m_isTrackingTouchEvents)
         return;
@@ -4578,6 +4581,7 @@
     m_dynamicViewportSizeUpdateWaitingForTarget = false;
     m_dynamicViewportSizeUpdateWaitingForLayerTreeCommit = false;
     m_dynamicViewportSizeUpdateLayerTreeTransactionID = 0;
+    m_layerTreeTransactionIdAtLastTouchStart = 0;
 #endif
 
     CallbackBase::Error error;

Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.h (178979 => 178980)


--- trunk/Source/WebKit2/UIProcess/WebPageProxy.h	2015-01-23 04:06:40 UTC (rev 178979)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.h	2015-01-23 04:17:11 UTC (rev 178980)
@@ -896,6 +896,7 @@
     void commitPotentialTap();
     void cancelPotentialTap();
     void tapHighlightAtPosition(const WebCore::FloatPoint&, uint64_t& requestID);
+    void handleTap(const WebCore::FloatPoint&);
 
     void inspectorNodeSearchMovedToPosition(const WebCore::FloatPoint&);
     void inspectorNodeSearchEndedAtPosition(const WebCore::FloatPoint&);
@@ -1422,6 +1423,7 @@
     bool m_dynamicViewportSizeUpdateWaitingForTarget;
     bool m_dynamicViewportSizeUpdateWaitingForLayerTreeCommit;
     uint64_t m_dynamicViewportSizeUpdateLayerTreeTransactionID;
+    uint64_t m_layerTreeTransactionIdAtLastTouchStart;
 #endif
 
 #if ENABLE(VIBRATION)

Modified: trunk/Source/WebKit2/UIProcess/ios/WKContentView.mm (178979 => 178980)


--- trunk/Source/WebKit2/UIProcess/ios/WKContentView.mm	2015-01-23 04:06:40 UTC (rev 178979)
+++ trunk/Source/WebKit2/UIProcess/ios/WKContentView.mm	2015-01-23 04:17:11 UTC (rev 178980)
@@ -461,6 +461,7 @@
 - (void)_didCommitLoadForMainFrame
 {
     [self _stopAssistingNode];
+    [self _cancelLongPressGestureRecognizer];
     [_webView _didCommitLoadForMainFrame];
 }
 

Modified: trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm (178979 => 178980)


--- trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm	2015-01-23 04:06:40 UTC (rev 178979)
+++ trunk/Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm	2015-01-23 04:17:11 UTC (rev 178980)
@@ -1071,7 +1071,7 @@
     if (![self isFirstResponder])
         [self becomeFirstResponder];
 
-    _page->process().send(Messages::WebPage::HandleTap(IntPoint(location)), _page->pageID());
+    _page->handleTap(location);
 }
 
 - (void)useSelectionAssistantWithMode:(UIWebSelectionMode)selectionMode

Modified: trunk/Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm (178979 => 178980)


--- trunk/Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm	2015-01-23 04:06:40 UTC (rev 178979)
+++ trunk/Source/WebKit2/UIProcess/ios/WebPageProxyIOS.mm	2015-01-23 04:17:11 UTC (rev 178980)
@@ -686,7 +686,7 @@
 
 void WebPageProxy::commitPotentialTap()
 {
-    process().send(Messages::WebPage::CommitPotentialTap(), m_pageID);
+    process().send(Messages::WebPage::CommitPotentialTap(m_layerTreeTransactionIdAtLastTouchStart), m_pageID);
 }
 
 void WebPageProxy::cancelPotentialTap()
@@ -699,6 +699,11 @@
     process().send(Messages::WebPage::TapHighlightAtPosition(requestID, position), m_pageID);
 }
 
+void WebPageProxy::handleTap(const FloatPoint& location)
+{
+    process().send(Messages::WebPage::HandleTap(roundedIntPoint(location), m_layerTreeTransactionIdAtLastTouchStart), m_pageID);
+}
+
 void WebPageProxy::inspectorNodeSearchMovedToPosition(const WebCore::FloatPoint& position)
 {
     process().send(Messages::WebPage::InspectorNodeSearchMovedToPosition(position), m_pageID);

Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp (178979 => 178980)


--- trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp	2015-01-23 04:06:40 UTC (rev 178979)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPage.cpp	2015-01-23 04:17:11 UTC (rev 178980)
@@ -4547,6 +4547,7 @@
     m_firstLayerTreeTransactionIDAfterDidCommitLoad = downcast<RemoteLayerTreeDrawingArea>(*m_drawingArea).nextTransactionID();
     m_userHasChangedPageScaleFactor = false;
     m_estimatedLatency = std::chrono::milliseconds(1000 / 60);
+    cancelPotentialTap();
 
 #if ENABLE(IOS_TOUCH_EVENTS)
     WebProcess::shared().eventDispatcher().clearQueuedTouchEventsForPage(*this);

Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h (178979 => 178980)


--- trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h	2015-01-23 04:06:40 UTC (rev 178979)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPage.h	2015-01-23 04:17:11 UTC (rev 178980)
@@ -473,9 +473,9 @@
     bool allowsUserScaling() const;
     bool hasStablePageScaleFactor() const { return m_hasStablePageScaleFactor; }
 
-    void handleTap(const WebCore::IntPoint&);
+    void handleTap(const WebCore::IntPoint&, uint64_t lastLayerTreeTranscationId);
     void potentialTapAtPosition(uint64_t requestID, const WebCore::FloatPoint&);
-    void commitPotentialTap();
+    void commitPotentialTap(uint64_t lastLayerTreeTranscationId);
     void commitPotentialTapFailed();
     void cancelPotentialTap();
     void tapHighlightAtPosition(uint64_t requestID, const WebCore::FloatPoint&);

Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPage.messages.in (178979 => 178980)


--- trunk/Source/WebKit2/WebProcess/WebPage/WebPage.messages.in	2015-01-23 04:06:40 UTC (rev 178979)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPage.messages.in	2015-01-23 04:17:11 UTC (rev 178980)
@@ -49,9 +49,9 @@
     DynamicViewportSizeUpdate(WebCore::FloatSize minimumLayoutSize, WebCore::FloatSize maximumUnobscuredSize, WebCore::FloatRect targetExposedContentRect, WebCore::FloatRect targetUnobscuredRect, WebCore::FloatRect targetUnobscuredRectInScrollViewCoordinates, double scale, int32_t deviceOrientation)
     SynchronizeDynamicViewportUpdate() -> (double newTargetScale, WebCore::FloatPoint newScrollPosition, uint64_t nextValidLayerTreeTransactionID)
 
-    HandleTap(WebCore::IntPoint point)
+    HandleTap(WebCore::IntPoint point, uint64_t lastLayerTreeTranscationId)
     PotentialTapAtPosition(uint64_t requestID, WebCore::FloatPoint point)
-    CommitPotentialTap()
+    CommitPotentialTap(uint64_t lastLayerTreeTranscationId)
     CancelPotentialTap()
     TapHighlightAtPosition(uint64_t requestID, WebCore::FloatPoint point)
     InspectorNodeSearchMovedToPosition(WebCore::FloatPoint point)

Modified: trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm (178979 => 178980)


--- trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm	2015-01-23 04:06:40 UTC (rev 178979)
+++ trunk/Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm	2015-01-23 04:17:11 UTC (rev 178980)
@@ -503,8 +503,13 @@
         send(Messages::WebPageProxy::DidNotHandleTapAsClick(roundedIntPoint(location)));
 }
 
-void WebPage::handleTap(const IntPoint& point)
+void WebPage::handleTap(const IntPoint& point, uint64_t lastLayerTreeTranscationId)
 {
+    if (lastLayerTreeTranscationId < m_firstLayerTreeTransactionIDAfterDidCommitLoad) {
+        send(Messages::WebPageProxy::DidNotHandleTapAsClick(roundedIntPoint(m_potentialTapLocation)));
+        return;
+    }
+
     FloatPoint adjustedPoint;
     Node* nodeRespondingToClick = m_page->mainFrame().nodeRespondingToClickEvents(point, adjustedPoint);
     handleSyntheticClick(nodeRespondingToClick, adjustedPoint);
@@ -552,9 +557,9 @@
     sendTapHighlightForNodeIfNecessary(requestID, m_potentialTapNode.get());
 }
 
-void WebPage::commitPotentialTap()
+void WebPage::commitPotentialTap(uint64_t lastLayerTreeTranscationId)
 {
-    if (!m_potentialTapNode || !m_potentialTapNode->renderer()) {
+    if (!m_potentialTapNode || !m_potentialTapNode->renderer() || lastLayerTreeTranscationId < m_firstLayerTreeTransactionIDAfterDidCommitLoad) {
         commitPotentialTapFailed();
         return;
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to