Bug 137735

Summary: [GTK] Move touch events handling from Platform to WebKit2
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, commit-queue, gustavo, gyuyoung.kim, mrobinson, pnormand, rakuco, ryuan.choi, sergio, svillar
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 137812    
Attachments:
Description Flags
Patch svillar: review+, svillar: commit-queue-

Description Carlos Garcia Campos 2014-10-15 01:10:20 PDT
GtkTouchContextHelper is only used by WebKitWebViewBase, since it's the only one that can create touch events. The code can be simplified if the events are processed in the view, and the native touch events are created with the native event and touch points.
Comment 1 Carlos Garcia Campos 2014-10-15 01:16:56 PDT
Created attachment 239857 [details]
Patch
Comment 2 WebKit Commit Bot 2014-10-15 01:18:34 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Sergio Villar Senin 2014-10-17 02:21:26 PDT
Comment on attachment 239857 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=239857&action=review

Another file nuked yay!

Everything seems to be right, it's a r+ for me, but consider my suggestions before landing.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:806
> +static void webkitWebViewBaseGetTouchPointForEvent(WebKitWebViewBase* webViewBase, GdkEvent* event, Vector<WebPlatformTouchPoint>& touchPoints)

Nit: GetTouchPoints

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:811
> +    for (auto it = priv->touchEvents.begin(); it != priv->touchEvents.end(); ++it) {

You can use a modern for loop here.

for (auto& touchEvent : priv->touchEvents)

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:828
> +    }

I am not sure a lambda is appropriate here, for me it seems far more confusing than calling a function.
Comment 4 Carlos Garcia Campos 2014-10-17 03:04:43 PDT
Committed r174817: <http://trac.webkit.org/changeset/174817>