Bug 137735 - [GTK] Move touch events handling from Platform to WebKit2
Summary: [GTK] Move touch events handling from Platform to WebKit2
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Keywords: Gtk
Depends on:
Blocks: 137812
  Show dependency treegraph
Reported: 2014-10-15 01:10 PDT by Carlos Garcia Campos
Modified: 2014-10-17 03:04 PDT (History)
10 users (show)

See Also:

Patch (18.92 KB, patch)
2014-10-15 01:16 PDT, Carlos Garcia Campos
svillar: review+
svillar: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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]
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]

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>