Changeset 142913 in webkit


Ignore:
Timestamp:
Feb 14, 2013, 1:56:03 PM (12 years ago)
Author:
aelias@chromium.org
Message:

[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):

Location:
trunk/Source/WebKit/chromium
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/chromium/ChangeLog

    r142911 r142913  
     12013-02-14  Alexandre Elias  <aelias@chromium.org>
     2
     3        [chromium] Fix scaling in WebViewImpl::handleGestureEvent, second try
     4        https://bugs.webkit.org/show_bug.cgi?id=109671
     5
     6        Reviewed by James Robinson.
     7
     8        My patch 142571 broke a bunch of things in handleGestureEvent that
     9        assumed the event came in scaled, most notably tap highlight and
     10        double-tap zoom. Switch those to PlatformGestureEvent.
     11
     12        142808 was an earlier version of this patch that was reverted
     13        due to fling events asserting they can't be converted to
     14        PlatformGestureEvent. This version moves fling earlier in the
     15        function to avoid that.
     16
     17        * src/WebViewImpl.cpp:
     18        (WebKit::WebViewImpl::handleGestureEvent):
     19        (WebKit::WebViewImpl::bestTapNode):
     20        (WebKit::WebViewImpl::enableTapHighlight):
     21        * src/WebViewImpl.h:
     22        (WebViewImpl):
     23        * tests/LinkHighlightTest.cpp:
     24        (WebCore::TEST):
     25
    1262013-02-14  Dirk Pranke  <dpranke@chromium.org>
    227

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

    r142879 r142913  
    697697    bool eventCancelled = false; // for disambiguation
    698698
     699    // Special handling for slow-path fling gestures, which have no PlatformGestureEvent equivalent.
     700    switch (event.type) {
     701    case WebInputEvent::GestureFlingStart: {
     702        if (mainFrameImpl()->frame()->eventHandler()->isScrollbarHandlingGestures()) {
     703            m_client->didHandleGestureEvent(event, eventCancelled);
     704            return eventSwallowed;
     705        }
     706        m_client->cancelScheduledContentIntents();
     707        m_positionOnFlingStart = WebPoint(event.x / pageScaleFactor(), event.y / pageScaleFactor());
     708        m_globalPositionOnFlingStart = WebPoint(event.globalX, event.globalY);
     709        m_flingModifier = event.modifiers;
     710        m_flingSourceDevice = event.sourceDevice;
     711        OwnPtr<WebGestureCurve> flingCurve = adoptPtr(Platform::current()->createFlingAnimationCurve(event.sourceDevice, WebFloatPoint(event.data.flingStart.velocityX, event.data.flingStart.velocityY), WebSize()));
     712        m_gestureAnimation = WebActiveGestureAnimation::createAtAnimationStart(flingCurve.release(), this);
     713        scheduleAnimation();
     714        eventSwallowed = true;
     715
     716        m_client->didHandleGestureEvent(event, eventCancelled);
     717        return eventSwallowed;
     718    }
     719    case WebInputEvent::GestureFlingCancel:
     720        if (m_gestureAnimation) {
     721            m_gestureAnimation.clear();
     722            if (m_layerTreeView)
     723                m_layerTreeView->didStopFlinging();
     724            eventSwallowed = true;
     725        }
     726
     727        m_client->didHandleGestureEvent(event, eventCancelled);
     728        return eventSwallowed;
     729    default:
     730        break;
     731    }
     732
     733    PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
     734
    699735    // Handle link highlighting outside the main switch to avoid getting lost in the
    700736    // complicated set of cases handled below.
    … …  
    704740#if OS(LINUX)
    705741        if (settingsImpl()->gestureTapHighlightEnabled())
    706             enableTouchHighlight(event);
     742            enableTapHighlight(platformEvent);
    707743#endif
    708744        break;
    … …  
    718754
    719755    switch (event.type) {
    720     case WebInputEvent::GestureFlingStart: {
    721         if (mainFrameImpl()->frame()->eventHandler()->isScrollbarHandlingGestures())
    722             break;
    723         m_client->cancelScheduledContentIntents();
    724         m_positionOnFlingStart = WebPoint(event.x, event.y);
    725         m_globalPositionOnFlingStart = WebPoint(event.globalX, event.globalY);
    726         m_flingModifier = event.modifiers;
    727         m_flingSourceDevice = event.sourceDevice;
    728         OwnPtr<WebGestureCurve> flingCurve = adoptPtr(Platform::current()->createFlingAnimationCurve(event.sourceDevice, WebFloatPoint(event.data.flingStart.velocityX, event.data.flingStart.velocityY), WebSize()));
    729         m_gestureAnimation = WebActiveGestureAnimation::createAtAnimationStart(flingCurve.release(), this);
    730         scheduleAnimation();
    731         eventSwallowed = true;
    732         break;
    733     }
    734     case WebInputEvent::GestureFlingCancel:
    735         if (m_gestureAnimation) {
    736             m_gestureAnimation.clear();
    737             if (m_layerTreeView)
    738                 m_layerTreeView->didStopFlinging();
    739             eventSwallowed = true;
    740         }
    741         break;
    742756    case WebInputEvent::GestureTap: {
    743757        m_client->cancelScheduledContentIntents();
    744         if (detectContentOnTouch(WebPoint(event.x, event.y))) {
     758        if (detectContentOnTouch(platformEvent.position())) {
    745759            eventSwallowed = true;
    746760            break;
    … …  
    757771            // FIXME: didTapMultipleTargets should just take a rect instead of
    758772            // an event.
    759             WebGestureEvent scaledEvent;
     773            WebGestureEvent scaledEvent = event;
    760774            scaledEvent.x = event.x / pageScaleFactor();
    761775            scaledEvent.y = event.y / pageScaleFactor();
    … …  
    774788        }
    775789
    776         PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
    777790        eventSwallowed = mainFrameImpl()->frame()->eventHandler()->handleGestureEvent(platformEvent);
    778791
    … …  
    796809        m_page->contextMenuController()->clearContextMenu();
    797810        m_contextMenuAllowed = true;
    798         PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
    799811        eventSwallowed = mainFrameImpl()->frame()->eventHandler()->handleGestureEvent(platformEvent);
    800812        m_contextMenuAllowed = false;
    … …  
    804816    case WebInputEvent::GestureTapDown: {
    805817        m_client->cancelScheduledContentIntents();
    806         PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
    807818        eventSwallowed = mainFrameImpl()->frame()->eventHandler()->handleGestureEvent(platformEvent);
    808819        break;
    … …  
    811822        if (m_webSettings->doubleTapToZoomEnabled() && m_minimumPageScaleFactor != m_maximumPageScaleFactor) {
    812823            m_client->cancelScheduledContentIntents();
    813             animateZoomAroundPoint(WebPoint(event.x, event.y), DoubleTap);
     824            animateZoomAroundPoint(platformEvent.position(), DoubleTap);
    814825            eventSwallowed = true;
    815826            break;
    … …  
    824835    case WebInputEvent::GesturePinchEnd:
    825836    case WebInputEvent::GesturePinchUpdate: {
    826         PlatformGestureEventBuilder platformEvent(mainFrameImpl()->frameView(), event);
    827837        eventSwallowed = mainFrameImpl()->frame()->eventHandler()->handleGestureEvent(platformEvent);
    828838        break;
    … …  
    12711281}
    12721282
    1273 Node* WebViewImpl::bestTouchLinkNode(const WebGestureEvent& touchEvent)
     1283Node* WebViewImpl::bestTapNode(const PlatformGestureEvent& tapEvent)
    12741284{
    12751285    if (!m_page || !m_page->mainFrame())
    … …  
    12781288    Node* bestTouchNode = 0;
    12791289
    1280     IntSize touchEventSearchRegionSize(touchEvent.data.tapDown.width / 2, touchEvent.data.tapDown.height / 2);
    1281     IntPoint touchEventLocation(touchEvent.x, touchEvent.y);
     1290    IntPoint touchEventLocation(tapEvent.position());
    12821291#if ENABLE(TOUCH_ADJUSTMENT)
    1283     m_page->mainFrame()->eventHandler()->adjustGesturePosition(PlatformGestureEventBuilder(mainFrameImpl()->frameView(), touchEvent), touchEventLocation);
     1292    m_page->mainFrame()->eventHandler()->adjustGesturePosition(tapEvent, touchEventLocation);
    12841293#endif
    12851294
    … …  
    12911300    // Make sure our highlight candidate uses a hand cursor as a heuristic to
    12921301    // choose appropriate targets.
    1293     bool shiftKey = touchEvent.modifiers & WebGestureEvent::ShiftKey;
    1294     while (bestTouchNode && !invokesHandCursor(bestTouchNode, shiftKey, m_page->mainFrame()))
     1302    while (bestTouchNode && !invokesHandCursor(bestTouchNode, false, m_page->mainFrame()))
    12951303        bestTouchNode = bestTouchNode->parentNode();
    12961304
    12971305    // We should pick the largest enclosing node with hand cursor set.
    1298     while (bestTouchNode && bestTouchNode->parentNode() && invokesHandCursor(bestTouchNode->parentNode(), shiftKey, m_page->mainFrame()))
     1306    while (bestTouchNode && bestTouchNode->parentNode() && invokesHandCursor(bestTouchNode->parentNode(), false, m_page->mainFrame()))
    12991307        bestTouchNode = bestTouchNode->parentNode();
    13001308
    … …  
    13021310}
    13031311
    1304 void WebViewImpl::enableTouchHighlight(const WebGestureEvent& touchEvent)
     1312void WebViewImpl::enableTapHighlight(const PlatformGestureEvent& tapEvent)
    13051313{
    13061314    // Always clear any existing highlight when this is invoked, even if we don't get a new target to highlight.
    13071315    m_linkHighlight.clear();
    13081316
    1309     Node* touchNode = bestTouchLinkNode(touchEvent);
     1317    Node* touchNode = bestTapNode(tapEvent);
    13101318
    13111319    if (!touchNode || !touchNode->renderer() || !touchNode->renderer()->enclosingLayer())

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

    r142872 r142913  
    577577#if ENABLE(GESTURE_EVENTS)
    578578    void computeScaleAndScrollForHitRect(const WebRect& hitRect, AutoZoomType, float& scale, WebPoint& scroll, bool& isAnchor);
    579     WebCore::Node* bestTouchLinkNode(const WebGestureEvent& touchEvent);
    580     void enableTouchHighlight(const WebGestureEvent& touchEvent);
     579    WebCore::Node* bestTapNode(const WebCore::PlatformGestureEvent& tapEvent);
     580    void enableTapHighlight(const WebCore::PlatformGestureEvent& tapEvent);
    581581    void computeScaleAndScrollForFocusedNode(WebCore::Node* focusedNode, float& scale, WebCore::IntPoint& scroll, bool& needAnimation);
    582582#endif

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

    r142872 r142913  
    2828
    2929#include "FrameTestHelpers.h"
     30#include "FrameView.h"
    3031#include "IntRect.h"
    3132#include "Node.h"
    … …  
    3334#include "WebCompositorInitializer.h"
    3435#include "WebFrame.h"
     36#include "WebFrameImpl.h"
    3537#include "WebInputEvent.h"
     38#include "WebInputEventConversion.h"
    3639#include "WebViewImpl.h"
    3740#include <gtest/gtest.h>
    … …  
    6770    touchEvent.x = 20;
    6871    touchEvent.y = 20;
    69     Node* touchNode = webViewImpl->bestTouchLinkNode(touchEvent);
    70     ASSERT_TRUE(touchNode);
     72
     73    {
     74        PlatformGestureEventBuilder platformEvent(webViewImpl->mainFrameImpl()->frameView(), touchEvent);
     75        Node* touchNode = webViewImpl->bestTapNode(platformEvent);
     76        ASSERT_TRUE(touchNode);
     77    }
    7178
    7279    touchEvent.y = 40;
    73     EXPECT_FALSE(webViewImpl->bestTouchLinkNode(touchEvent));
     80    {
     81        PlatformGestureEventBuilder platformEvent(webViewImpl->mainFrameImpl()->frameView(), touchEvent);
     82        EXPECT_FALSE(webViewImpl->bestTapNode(platformEvent));
     83    }
    7484
    7585    touchEvent.y = 20;
    7686    // Shouldn't crash.
    7787
    78     webViewImpl->enableTouchHighlight(touchEvent);
     88    {
     89        PlatformGestureEventBuilder platformEvent(webViewImpl->mainFrameImpl()->frameView(), touchEvent);
     90        webViewImpl->enableTapHighlight(platformEvent);
     91    }
     92
    7993    EXPECT_TRUE(webViewImpl->linkHighlight());
    8094    EXPECT_TRUE(webViewImpl->linkHighlight()->contentLayer());
    … …  
    8498
    8599    touchEvent.y = 100;
    86     webViewImpl->enableTouchHighlight(touchEvent);
     100    {
     101        PlatformGestureEventBuilder platformEvent(webViewImpl->mainFrameImpl()->frameView(), touchEvent);
     102        webViewImpl->enableTapHighlight(platformEvent);
     103    }
     104
    87105    ASSERT_TRUE(webViewImpl->linkHighlight());
    88106
    89107    // Don't highlight if no "hand cursor"
    90108    touchEvent.y = 220; // An A-link with cross-hair cursor.
    91     webViewImpl->enableTouchHighlight(touchEvent);
     109    {
     110        PlatformGestureEventBuilder platformEvent(webViewImpl->mainFrameImpl()->frameView(), touchEvent);
     111        webViewImpl->enableTapHighlight(platformEvent);
     112    }
    92113    ASSERT_FALSE(webViewImpl->linkHighlight());
    93114
    94115    touchEvent.y = 260; // A text input box.
    95     webViewImpl->enableTouchHighlight(touchEvent);
     116    {
     117        PlatformGestureEventBuilder platformEvent(webViewImpl->mainFrameImpl()->frameView(), touchEvent);
     118        webViewImpl->enableTapHighlight(platformEvent);
     119    }
    96120    ASSERT_FALSE(webViewImpl->linkHighlight());
    97121
Note: See TracChangeset for help on using the changeset viewer.