[Webkit-unassigned] [Bug 49345] [Chromium] Add a basic GestureManager to Chromium

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=49345


Antonio Gomes <tonikitoo at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #76281|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #17 from Antonio Gomes <tonikitoo at webkit.org>  2010-12-15 12:29:05 PST ---
(From update of attachment 76281)
View in context: https://bugs.webkit.org/attachment.cgi?id=76281&action=review

I like Benjamin's review. His comments should be addressed.

> WebCore/page/GestureManager.cpp:46
> +// Edge functions.
> +static bool click(GestureManager*, const PlatformTouchPoint&);
> +static bool clickOrScroll(GestureManager*, const PlatformTouchPoint&);
> +static bool inScroll(GestureManager*, const PlatformTouchPoint&);
> +static bool reset(GestureManager*, const PlatformTouchPoint&);
> +static bool touchDown(GestureManager*, const PlatformTouchPoint&);
> +

If you are passing the GM here anyways, why not putting these methods as class methods?

> WebCore/platform/PlatformWheelEvent.h:114
>  
> +        PlatformWheelEvent(IntPoint position,
> +                           IntPoint globalPosition,
> +                           float deltaX,
> +                           float deltaY,
> +                           float wheelTicksX,
> +                           float wheelTicksY,
> +                           PlatformWheelEventGranularity granularity,
> +                           bool isAccepted,
> +                           bool shiftKey,
> +                           bool ctrlKey,
> +                           bool altKey,
> +                           bool metaKey)
> +            : m_position(position)
> +            , m_globalPosition(globalPosition)
> +            , m_deltaX(deltaX)
> +            , m_deltaY(deltaY)
> +            , m_wheelTicksX(wheelTicksX)
> +            , m_wheelTicksY(wheelTicksY)
> +            , m_granularity(granularity)
> +            , m_isAccepted(isAccepted)
> +            , m_shiftKey(shiftKey)
> +            , m_ctrlKey(ctrlKey)
> +            , m_altKey(altKey)
> +            , m_metaKey(metaKey)
> +        {
> +        }

You should say in the changelog why you are adding this. (Or it could come on its own patch(?)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.