Modified: trunk/Source/WebKit/chromium/ChangeLog (142912 => 142913)
--- trunk/Source/WebKit/chromium/ChangeLog 2013-02-14 21:54:32 UTC (rev 142912)
+++ trunk/Source/WebKit/chromium/ChangeLog 2013-02-14 21:56:03 UTC (rev 142913)
@@ -1,3 +1,28 @@
+2013-02-14 Alexandre Elias <ael...@chromium.org>
+
+ [chromium] Fix scaling in WebViewImpl::handleGestureEvent, second try
+ https://bugs.webkit.org/show_bug.cgi?id=109671
+
+ Reviewed by James Robinson.
+
+ My patch 142571 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.
+
+ 142808 was an earlier version of this patch that was reverted
+ due to fling events asserting they can't be converted to
+ PlatformGestureEvent. This version moves fling earlier in the
+ function to avoid that.
+
+ * src/WebViewImpl.cpp:
+ (WebKit::WebViewImpl::handleGestureEvent):
+ (WebKit::WebViewImpl::bestTapNode):
+ (WebKit::WebViewImpl::enableTapHighlight):
+ * src/WebViewImpl.h:
+ (WebViewImpl):
+ * tests/LinkHighlightTest.cpp:
+ (WebCore::TEST):
+
2013-02-14 Dirk Pranke <dpra...@chromium.org>
Unreviewed, rolling out r142901.
Modified: trunk/Source/WebKit/chromium/src/WebViewImpl.cpp (142912 => 142913)
--- trunk/Source/WebKit/chromium/src/WebViewImpl.cpp 2013-02-14 21:54:32 UTC (rev 142912)
+++ trunk/Source/WebKit/chromium/src/WebViewImpl.cpp 2013-02-14 21:56:03 UTC (rev 142913)
@@ -696,6 +696,42 @@
bool eventSwallowed = false;
bool eventCancelled = false; // for disambiguation
+ // Special handling for slow-path fling gestures, which have no PlatformGestureEvent equivalent.
+ switch (event.type) {
+ case WebInputEvent::GestureFlingStart: {
+ if (mainFrameImpl()->frame()->eventHandler()->isScrollbarHandlingGestures()) {
+ m_client->didHandleGestureEvent(event, eventCancelled);
+ return eventSwallowed;
+ }
+ m_client->cancelScheduledContentIntents();
+ m_positionOnFlingStart = WebPoint(event.x / pageScaleFactor(), event.y / pageScaleFactor());
+ m_globalPositionOnFlingStart = WebPoint(event.globalX, event.globalY);
+ m_flingModifier = event.modifiers;
+ m_flingSourceDevice = event.sourceDevice;
+ OwnPtr<WebGestureCurve> flingCurve = adoptPtr(Platform::current()->createFlingAnimationCurve(event.sourceDevice, WebFloatPoint(event.data.flingStart.velocityX, event.data.flingStart.velocityY), WebSize()));
+ m_gestureAnimation = WebActiveGestureAnimation::createAtAnimationStart(flingCurve.release(), this);
+ scheduleAnimation();
+ eventSwallowed = true;
+
+ m_client->didHandleGestureEvent(event, eventCancelled);
+ return eventSwallowed;
+ }
+ case WebInputEvent::GestureFlingCancel:
+ if (m_gestureAnimation) {
+ m_gestureAnimation.clear();
+ if (m_layerTreeView)
+ m_layerTreeView->didStopFlinging();
+ eventSwallowed = true;
+ }
+
+ m_client->didHandleGestureEvent(event, eventCancelled);
+ return eventSwallowed;
+ default:
+ break;
+ }
+
+ PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
+
// Handle link highlighting outside the main switch to avoid getting lost in the
// complicated set of cases handled below.
switch (event.type) {
@@ -703,7 +739,7 @@
// Queue a highlight animation, then hand off to regular handler.
#if OS(LINUX)
if (settingsImpl()->gestureTapHighlightEnabled())
- enableTouchHighlight(event);
+ enableTapHighlight(platformEvent);
#endif
break;
case WebInputEvent::GestureTapCancel:
@@ -717,31 +753,9 @@
}
switch (event.type) {
- case WebInputEvent::GestureFlingStart: {
- if (mainFrameImpl()->frame()->eventHandler()->isScrollbarHandlingGestures())
- break;
- m_client->cancelScheduledContentIntents();
- m_positionOnFlingStart = WebPoint(event.x, event.y);
- m_globalPositionOnFlingStart = WebPoint(event.globalX, event.globalY);
- m_flingModifier = event.modifiers;
- m_flingSourceDevice = event.sourceDevice;
- OwnPtr<WebGestureCurve> flingCurve = adoptPtr(Platform::current()->createFlingAnimationCurve(event.sourceDevice, WebFloatPoint(event.data.flingStart.velocityX, event.data.flingStart.velocityY), WebSize()));
- m_gestureAnimation = WebActiveGestureAnimation::createAtAnimationStart(flingCurve.release(), this);
- scheduleAnimation();
- eventSwallowed = true;
- break;
- }
- case WebInputEvent::GestureFlingCancel:
- if (m_gestureAnimation) {
- m_gestureAnimation.clear();
- if (m_layerTreeView)
- m_layerTreeView->didStopFlinging();
- eventSwallowed = true;
- }
- break;
case WebInputEvent::GestureTap: {
m_client->cancelScheduledContentIntents();
- if (detectContentOnTouch(WebPoint(event.x, event.y))) {
+ if (detectContentOnTouch(platformEvent.position())) {
eventSwallowed = true;
break;
}
@@ -756,7 +770,7 @@
if (event.data.tap.width > 0 && !shouldDisableDesktopWorkarounds()) {
// 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();
scaledEvent.data.tap.width = event.data.tap.width / pageScaleFactor();
@@ -773,7 +787,6 @@
}
}
- PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
eventSwallowed = mainFrameImpl()->frame()->eventHandler()->handleGestureEvent(platformEvent);
if (m_selectPopup && m_selectPopup == selectPopup) {
@@ -795,7 +808,6 @@
m_client->cancelScheduledContentIntents();
m_page->contextMenuController()->clearContextMenu();
m_contextMenuAllowed = true;
- PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
eventSwallowed = mainFrameImpl()->frame()->eventHandler()->handleGestureEvent(platformEvent);
m_contextMenuAllowed = false;
@@ -803,14 +815,13 @@
}
case WebInputEvent::GestureTapDown: {
m_client->cancelScheduledContentIntents();
- PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
eventSwallowed = mainFrameImpl()->frame()->eventHandler()->handleGestureEvent(platformEvent);
break;
}
case WebInputEvent::GestureDoubleTap:
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;
}
@@ -823,7 +834,6 @@
case WebInputEvent::GestureTapCancel:
case WebInputEvent::GesturePinchEnd:
case WebInputEvent::GesturePinchUpdate: {
- PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
eventSwallowed = mainFrameImpl()->frame()->eventHandler()->handleGestureEvent(platformEvent);
break;
}
@@ -1270,17 +1280,16 @@
|| (cursor == CURSOR_AUTO && frame->eventHandler()->useHandCursor(node, node->isLink(), shiftKey));
}
-Node* WebViewImpl::bestTouchLinkNode(const WebGestureEvent& touchEvent)
+Node* WebViewImpl::bestTapNode(const PlatformGestureEvent& tapEvent)
{
if (!m_page || !m_page->mainFrame())
return 0;
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
IntPoint hitTestPoint = m_page->mainFrame()->view()->windowToContents(touchEventLocation);
@@ -1290,23 +1299,22 @@
// 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();
return bestTouchNode;
}
-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())
return;
Modified: trunk/Source/WebKit/chromium/src/WebViewImpl.h (142912 => 142913)
--- trunk/Source/WebKit/chromium/src/WebViewImpl.h 2013-02-14 21:54:32 UTC (rev 142912)
+++ trunk/Source/WebKit/chromium/src/WebViewImpl.h 2013-02-14 21:56:03 UTC (rev 142913)
@@ -576,8 +576,8 @@
#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
void animateZoomAroundPoint(const WebCore::IntPoint&, AutoZoomType);
Modified: trunk/Source/WebKit/chromium/tests/LinkHighlightTest.cpp (142912 => 142913)
--- trunk/Source/WebKit/chromium/tests/LinkHighlightTest.cpp 2013-02-14 21:54:32 UTC (rev 142912)
+++ trunk/Source/WebKit/chromium/tests/LinkHighlightTest.cpp 2013-02-14 21:56:03 UTC (rev 142913)
@@ -27,12 +27,15 @@
#include "LinkHighlight.h"
#include "FrameTestHelpers.h"
+#include "FrameView.h"
#include "IntRect.h"
#include "Node.h"
#include "URLTestHelpers.h"
#include "WebCompositorInitializer.h"
#include "WebFrame.h"
+#include "WebFrameImpl.h"
#include "WebInputEvent.h"
+#include "WebInputEventConversion.h"
#include "WebViewImpl.h"
#include <gtest/gtest.h>
#include <public/WebContentLayer.h>
@@ -66,16 +69,27 @@
// The coordinates below are linked to absolute positions in the referenced .html file.
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());
EXPECT_TRUE(webViewImpl->linkHighlight()->clipLayer());
@@ -83,16 +97,26 @@
// Find a target inside a scrollable div
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());
webViewImpl->close();
