Changeset 142808 in webkit

Timestamp:

Feb 13, 2013 2:54:39 PM (11 years ago)

Author:

aelias@chromium.org

Message:

[chromium] Fix scaling in WebViewImpl::handleGestureEvent
​https://bugs.webkit.org/show_bug.cgi?id=109671

Reviewed by James Robinson.

My last patch broke a bunch of things in handleGestureEvent that
assumed the event came in scaled, most notably tap highlight and
double-tap zoom. Switch those to PlatformGestureEvent.

• src/WebViewImpl.cpp:

(WebKit::WebViewImpl::handleGestureEvent):
(WebKit::WebViewImpl::bestTapNode):
(WebKit::WebViewImpl::enableTapHighlight):

• src/WebViewImpl.h:

(WebViewImpl):

• tests/LinkHighlightTest.cpp:

(WebCore::TEST):

Location:

trunk/Source/WebKit/chromium

Files:

• ChangeLog (modified) (1 diff)
• src/WebViewImpl.cpp (modified) (14 diffs)
• src/WebViewImpl.h (modified) (1 diff)
• tests/LinkHighlightTest.cpp (modified) (4 diffs)

trunk/Source/WebKit/chromium/ChangeLog

@@ +1 @@
+2013-02-13  Alexandre Elias  <aelias@chromium.org>
+
+        [chromium] Fix scaling in WebViewImpl::handleGestureEvent
+        https://bugs.webkit.org/show_bug.cgi?id=109671
+
+        Reviewed by James Robinson.
+
+        My last patch broke a bunch of things in handleGestureEvent that
+        assumed the event came in scaled, most notably tap highlight and
+        double-tap zoom. Switch those to PlatformGestureEvent.
+
+        * src/WebViewImpl.cpp:
+        (WebKit::WebViewImpl::handleGestureEvent):
+        (WebKit::WebViewImpl::bestTapNode):
+        (WebKit::WebViewImpl::enableTapHighlight):
+        * src/WebViewImpl.h:
+        (WebViewImpl):
+        * tests/LinkHighlightTest.cpp:
+        (WebCore::TEST):
+
 2013-02-13  Eberhard Graether  <egraether@google.com>
 

trunk/Source/WebKit/chromium/src/WebViewImpl.cpp

@@ -697 +697 @@
     bool eventCancelled = false; // for disambiguation
 
+    PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
+
     // Handle link highlighting outside the main switch to avoid getting lost in the
     // complicated set of cases handled below.
@@ -704 +706 @@
 #if OS(LINUX)
         if (settingsImpl()->gestureTapHighlightEnabled())
-            enableTouchHighlight(event);
+            enableTapHighlight(platformEvent);
 #endif
         break;
@@ -722 +724 @@
             break;
         m_client->cancelScheduledContentIntents();
-        m_positionOnFlingStart = WebPoint(event.x, event.y);
-        m_globalPositionOnFlingStart = WebPoint(event.globalX, event.globalY);
+        m_positionOnFlingStart = platformEvent.position();
+        m_globalPositionOnFlingStart = platformEvent.globalPosition();
         m_flingModifier = event.modifiers;
         m_flingSourceDevice = event.sourceDevice;
@@ -742 +744 @@
     case WebInputEvent::GestureTap: {
         m_client->cancelScheduledContentIntents();
-        if (detectContentOnTouch(WebPoint(event.x, event.y))) {
+        if (detectContentOnTouch(platformEvent.position())) {
             eventSwallowed = true;
             break;
@@ -757 +759 @@
             // FIXME: didTapMultipleTargets should just take a rect instead of
             // an event.
-            WebGestureEvent scaledEvent;
+            WebGestureEvent scaledEvent = event;
             scaledEvent.x = event.x / pageScaleFactor();
             scaledEvent.y = event.y / pageScaleFactor();
@@ -774 +776 @@
         }
 
-        PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
         eventSwallowed = mainFrameImpl()->frame()->eventHandler()->handleGestureEvent(platformEvent);
 
@@ -796 +797 @@
         m_page->contextMenuController()->clearContextMenu();
         m_contextMenuAllowed = true;
-        PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
         eventSwallowed = mainFrameImpl()->frame()->eventHandler()->handleGestureEvent(platformEvent);
         m_contextMenuAllowed = false;
@@ -804 +804 @@
     case WebInputEvent::GestureTapDown: {
         m_client->cancelScheduledContentIntents();
-        PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
         eventSwallowed = mainFrameImpl()->frame()->eventHandler()->handleGestureEvent(platformEvent);
         break;
@@ -811 +810 @@
         if (m_webSettings->doubleTapToZoomEnabled() && m_minimumPageScaleFactor != m_maximumPageScaleFactor) {
             m_client->cancelScheduledContentIntents();
-            animateZoomAroundPoint(WebPoint(event.x, event.y), DoubleTap);
+            animateZoomAroundPoint(platformEvent.position(), DoubleTap);
             eventSwallowed = true;
             break;
@@ -824 +823 @@
     case WebInputEvent::GesturePinchEnd:
     case WebInputEvent::GesturePinchUpdate: {
-        PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
         eventSwallowed = mainFrameImpl()->frame()->eventHandler()->handleGestureEvent(platformEvent);
         break;
@@ -1271 +1269 @@
 }
 
-Node* WebViewImpl::bestTouchLinkNode(const WebGestureEvent& touchEvent)
+Node* WebViewImpl::bestTapNode(const PlatformGestureEvent& tapEvent)
 {
     if (!m_page || !m_page->mainFrame())
@@ -1278 +1276 @@
     Node* bestTouchNode = 0;
 
-    IntSize touchEventSearchRegionSize(touchEvent.data.tapDown.width / 2, touchEvent.data.tapDown.height / 2);
-    IntPoint touchEventLocation(touchEvent.x, touchEvent.y);
+    IntPoint touchEventLocation(tapEvent.position());
 #if ENABLE(TOUCH_ADJUSTMENT)
-    m_page->mainFrame()->eventHandler()->adjustGesturePosition(PlatformGestureEventBuilder(mainFrameImpl()->frameView(), touchEvent), touchEventLocation);
+    m_page->mainFrame()->eventHandler()->adjustGesturePosition(tapEvent, touchEventLocation);
 #endif
 
@@ -1291 +1288 @@
     // Make sure our highlight candidate uses a hand cursor as a heuristic to
     // choose appropriate targets.
-    bool shiftKey = touchEvent.modifiers & WebGestureEvent::ShiftKey;
-    while (bestTouchNode && !invokesHandCursor(bestTouchNode, shiftKey, m_page->mainFrame()))
+    while (bestTouchNode && !invokesHandCursor(bestTouchNode, false, m_page->mainFrame()))
         bestTouchNode = bestTouchNode->parentNode();
 
     // We should pick the largest enclosing node with hand cursor set.
-    while (bestTouchNode && bestTouchNode->parentNode() && invokesHandCursor(bestTouchNode->parentNode(), shiftKey, m_page->mainFrame()))
+    while (bestTouchNode && bestTouchNode->parentNode() && invokesHandCursor(bestTouchNode->parentNode(), false, m_page->mainFrame()))
         bestTouchNode = bestTouchNode->parentNode();
 
@@ -1302 +1298 @@
 }
 
-void WebViewImpl::enableTouchHighlight(const WebGestureEvent& touchEvent)
+void WebViewImpl::enableTapHighlight(const PlatformGestureEvent& tapEvent)
 {
     // Always clear any existing highlight when this is invoked, even if we don't get a new target to highlight.
     m_linkHighlight.clear();
 
-    Node* touchNode = bestTouchLinkNode(touchEvent);
+    Node* touchNode = bestTapNode(tapEvent);
 
     if (!touchNode || !touchNode->renderer() || !touchNode->renderer()->enclosingLayer())

trunk/Source/WebKit/chromium/src/WebViewImpl.h

@@ -577 +577 @@
 #if ENABLE(GESTURE_EVENTS)
     void computeScaleAndScrollForHitRect(const WebRect& hitRect, AutoZoomType, float& scale, WebPoint& scroll, bool& isAnchor);
-    WebCore::Node* bestTouchLinkNode(const WebGestureEvent& touchEvent);
-    void enableTouchHighlight(const WebGestureEvent& touchEvent);
+    WebCore::Node* bestTapNode(const WebCore::PlatformGestureEvent& tapEvent);
+    void enableTapHighlight(const WebCore::PlatformGestureEvent& tapEvent);
     void computeScaleAndScrollForFocusedNode(WebCore::Node* focusedNode, float& scale, WebCore::IntPoint& scroll, bool& needAnimation);
 #endif

trunk/Source/WebKit/chromium/tests/LinkHighlightTest.cpp

@@ -28 +28 @@
 
 #include "FrameTestHelpers.h"
+#include "FrameView.h"
 #include "IntRect.h"
 #include "Node.h"
@@ -33 +34 @@
 #include "WebCompositorInitializer.h"
 #include "WebFrame.h"
+#include "WebFrameImpl.h"
 #include "WebInputEvent.h"
+#include "WebInputEventConversion.h"
 #include "WebViewImpl.h"
 #include <gtest/gtest.h>
@@ -67 +70 @@
     touchEvent.x = 20;
     touchEvent.y = 20;
-    Node* touchNode = webViewImpl->bestTouchLinkNode(touchEvent);
-    ASSERT_TRUE(touchNode);
+
+    {
+        PlatformGestureEventBuilder platformEvent(webViewImpl->mainFrameImpl()->frameView(), touchEvent);
+        Node* touchNode = webViewImpl->bestTapNode(platformEvent);
+        ASSERT_TRUE(touchNode);
+    }
 
     touchEvent.y = 40;
-    EXPECT_FALSE(webViewImpl->bestTouchLinkNode(touchEvent));
+    {
+        PlatformGestureEventBuilder platformEvent(webViewImpl->mainFrameImpl()->frameView(), touchEvent);
+        EXPECT_FALSE(webViewImpl->bestTapNode(platformEvent));
+    }
 
     touchEvent.y = 20;
     // Shouldn't crash.
 
-    webViewImpl->enableTouchHighlight(touchEvent);
+    {
+        PlatformGestureEventBuilder platformEvent(webViewImpl->mainFrameImpl()->frameView(), touchEvent);
+        webViewImpl->enableTapHighlight(platformEvent);
+    }
+
     EXPECT_TRUE(webViewImpl->linkHighlight());
     EXPECT_TRUE(webViewImpl->linkHighlight()->contentLayer());
@@ -84 +98 @@
 
     touchEvent.y = 100;
-    webViewImpl->enableTouchHighlight(touchEvent);
+    {
+        PlatformGestureEventBuilder platformEvent(webViewImpl->mainFrameImpl()->frameView(), touchEvent);
+        webViewImpl->enableTapHighlight(platformEvent);
+    }
+
     ASSERT_TRUE(webViewImpl->linkHighlight());
 
     // Don't highlight if no "hand cursor"
     touchEvent.y = 220; // An A-link with cross-hair cursor.
-    webViewImpl->enableTouchHighlight(touchEvent);
+    {
+        PlatformGestureEventBuilder platformEvent(webViewImpl->mainFrameImpl()->frameView(), touchEvent);
+        webViewImpl->enableTapHighlight(platformEvent);
+    }
     ASSERT_FALSE(webViewImpl->linkHighlight());
 
     touchEvent.y = 260; // A text input box.
-    webViewImpl->enableTouchHighlight(touchEvent);
+    {
+        PlatformGestureEventBuilder platformEvent(webViewImpl->mainFrameImpl()->frameView(), touchEvent);
+        webViewImpl->enableTapHighlight(platformEvent);
+    }
     ASSERT_FALSE(webViewImpl->linkHighlight());