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

Tue Dec 7 10:09:43 PST 2010

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

--- Comment #5 from Adam Barth <abarth at webkit.org>  2010-12-07 10:09:43 PST ---
(From update of attachment 75402)
View in context: https://bugs.webkit.org/attachment.cgi?id=75402&action=review

> LayoutTests/fast/events/touch/touch-gesture-scroll.html:46
> +<body style="margin:0" onload="setTimeout('runTest();', 100)">

Why wait 100ms?

> LayoutTests/fast/events/touch/touch-gesture-scroll.html:84
> +        successfullyParsed = true;
> +        isSuccessfullyParsed();

Seems like we should do these even if there's no layoutTestController.

> LayoutTests/fast/events/touch/touch-gesture-scroll.html:113
> +    // Wait for layout.
> +    setTimeout(checkScrollOffset, 50);

Boo.  setTimeout makes tests slow and flaky.

> LayoutTests/fast/events/touch/touch-gesture-scroll.html:139
> +    layoutTestController.notifyDone();

This should be protected by if (window.layoutTestController)

> LayoutTests/fast/events/touch/touch-gesture-scroll.html:155
> +            setTimeout(exitIfNecessary, 200);

boo

> WebCore/page/EventHandler.cpp:3001
> -}
> +} // namespace WebCore

This change isn't really necessary.  We're inconsistent about whether to include these comments.

> WebCore/page/GestureManager.cpp:34
> +// #include <wtf/Assertions.h>

No commented out code, please.  :)

> WebCore/page/GestureManager.cpp:53
> +static const double maxClickDownTime = .8;

0.8

> WebCore/page/GestureManager.cpp:56
> +static const double minClickDownTime = .01;

ditto

> WebCore/page/GestureManager.cpp:78
> +void GestureManager::addEdgeFunction(State state,
> +                                     unsigned finger,
> +                                     PlatformTouchPoint::State touchType,
> +                                     bool handled,
> +                                     GestureTransitionFunction f)

All on one line please.

> WebCore/page/GestureManager.cpp:108
> +    PlatformMouseEvent fakeMouseDown(p.pos(),
> +                                     p.screenPos(),
> +                                     LeftButton,
> +                                     MouseEventPressed,
> +                                     1,
> +                                     false,
> +                                     false,
> +                                     false,
> +                                     false,
> +                                     WTF::currentTime());

WebKit doesn't have an 80 col limit.  Please feel free to use more horizontal space.  :)

> WebCore/page/GestureManager.cpp:153
> +        GestureTransitionFunction ef = m_edgeFunctions.get(hash(m_state, p.id(), p.state(), handled));
> +        if (ef)

We usually combine these lines.

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