Bug 153829

Summary: [GTK] Touch slider test fails due to assertion in webkitWebViewBaseTouchEvent()
Product: WebKit Reporter: Adrien Plazas <aplazas>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, bugs-noreply, carlosg, cgarcia, commit-queue, gustavo, mcatanzaro, mrobinson
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Remove gesture sequence from touch sequence set
none
Remove gesture sequence from touch sequence set
cgarcia: review+, cgarcia: commit-queue-
Archive of layout-test-results from ews117 for mac-yosemite
none
Remove gesture sequence from touch sequence set
cgarcia: review+, cgarcia: commit-queue-
Remove gesture sequence from touch sequence set
none
Remove gesture sequence from touch sequence set none

Description Adrien Plazas 2016-02-03 04:22:49 PST
The fast/events/touch/touch-slider.html test is crashing due to an assert in the GTK+ debug bot.

In the function handling touch events webkitWebViewBaseTouchEvent():
- when a touch event begins a touch event sequence, we add the sequence's ID to the set of the current touch sequences,
- when a touch event ends a touch event sequence, we remove the sequence's ID from the set of the current touch sequences,
- when a new touch event starts a new touch event sequence, as the previous suquence have ended, its ID is free to be used again and hence is used again (in the test, all sequences have the id 1).

Except this is true is we don't have gestures, when we do a gesture controller takes over the handling of the events, but as explained by some comment in webkitWebViewBaseTouchEvent(): "If we are already processing gestures is because the WebProcess didn't handle the BEGIN touch event, so pass subsequent events to the GestureController.".
Hence this happen:
- a touch event begins the touch event sequence of ID 1 and we add the sequence's ID to the set of the current touch sequences,
- the next events of the sequence are handled by the gesture controller,
- the touch event sequence of ID 1 ends and its ID can be used by another sequence,  but its ID have not been removed from the set of current sequences,
- a new touch event begins a new touch event sequence of ID 1 and when we try to add a sequence of ID 1 again, the assertion ASSERT(!priv->touchEvents.contains(sequence)) fails.

STDERR: ASSERTION FAILED: !priv->touchEvents.contains(sequence)
STDERR: ../../Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp(903) : gboolean webkitWebViewBaseTouchEvent(GtkWidget*, GdkEventTouch*)
STDERR: 1   0x7fe9bb420d61 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(WTFCrash+0x1e) [0x7fe9bb420d61]
STDERR: 2   0x7fe9b3d8ef7f /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(+0x4fdcf7f) [0x7fe9b3d8ef7f]
STDERR: 3   0x7fe9ae851c1e /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/DependenciesGTK/Root/lib/libgtk-3.so.0(+0x29cc1e) [0x7fe9ae851c1e]
STDERR: 4   0x7fe9ae271942 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/DependenciesGTK/Root/lib/libgobject-2.0.so.0(+0x11942) [0x7fe9ae271942]
STDERR: 5   0x7fe9ae271505 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/DependenciesGTK/Root/lib/libgobject-2.0.so.0(+0x11505) [0x7fe9ae271505]
STDERR: 6   0x7fe9ae28b4a6 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/DependenciesGTK/Root/lib/libgobject-2.0.so.0(g_signal_emit_valist+0x53a) [0x7fe9ae28b4a6]
STDERR: 7   0x7fe9ae28c610 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/DependenciesGTK/Root/lib/libgobject-2.0.so.0(g_signal_emit+0xa6) [0x7fe9ae28c610]
STDERR: 8   0x7fe9ae9fe238 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/DependenciesGTK/Root/lib/libgtk-3.so.0(+0x449238) [0x7fe9ae9fe238]
STDERR: 9   0x7fe9ae9fd6f1 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/DependenciesGTK/Root/lib/libgtk-3.so.0(gtk_widget_event+0x112) [0x7fe9ae9fd6f1]
STDERR: 10  0x7fe9ae850377 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/DependenciesGTK/Root/lib/libgtk-3.so.0(+0x29b377) [0x7fe9ae850377]
STDERR: 11  0x7fe9ae850669 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/DependenciesGTK/Root/lib/libgtk-3.so.0(+0x29b669) [0x7fe9ae850669]
STDERR: 12  0x7fe9ae850738 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/DependenciesGTK/Root/lib/libgtk-3.so.0(gtk_propagate_event+0xcc) [0x7fe9ae850738]
STDERR: 13  0x7fe9ae84f3b5 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/DependenciesGTK/Root/lib/libgtk-3.so.0(gtk_main_do_event+0x6f4) [0x7fe9ae84f3b5]
STDERR: 14  0x494e0e /home/slave/webkitgtk/gtk-linux-64-debug-tests/build/WebKitBuild/Debug/bin/WebKitTestRunner(WTR::EventSenderProxy::dispatchEvent(_GdkEvent*)+0x7e) [0x494e0e]
STDERR: 15  0x494eea /home/slave/webkitgtk/gtk-linux-64-debug-tests/build/WebKitBuild/Debug/bin/WebKitTestRunner(WTR::EventSenderProxy::sendOrQueueEvent(_GdkEvent*)+0x60) [0x494eea]
STDERR: 16  0x4961e2 /home/slave/webkitgtk/gtk-linux-64-debug-tests/build/WebKitBuild/Debug/bin/WebKitTestRunner(WTR::EventSenderProxy::sendUpdatedTouchEvents()+0xa0) [0x4961e2]
STDERR: 17  0x496282 /home/slave/webkitgtk/gtk-linux-64-debug-tests/build/WebKitBuild/Debug/bin/WebKitTestRunner(WTR::EventSenderProxy::touchStart()+0x18) [0x496282]
STDERR: 18  0x471893 /home/slave/webkitgtk/gtk-linux-64-debug-tests/build/WebKitBuild/Debug/bin/WebKitTestRunner(WTR::TestController::didReceiveSynchronousMessageFromInjectedBundle(OpaqueWKString const*, void const*)+0xb1b) [0x471893]
STDERR: 19  0x4702dc /home/slave/webkitgtk/gtk-linux-64-debug-tests/build/WebKitBuild/Debug/bin/WebKitTestRunner(WTR::TestController::didReceiveSynchronousPageMessageFromInjectedBundle(OpaqueWKPage const*, OpaqueWKString const*, void const*, void const**, void const*)+0x34) [0x4702dc]
STDERR: 20  0x7fe9b39f6542 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebKit::WebPageInjectedBundleClient::didReceiveSynchronousMessageFromInjectedBundle(WebKit::WebPageProxy*, WTF::String const&, API::Object*, WTF::RefPtr<API::Object>&)+0xaa) [0x7fe9b39f6542]
STDERR: 21  0x7fe9b39f8d96 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebKit::WebPageProxy::handleSynchronousMessage(IPC::Connection&, WTF::String const&, WebKit::UserData const&, WebKit::UserData&)+0xd6) [0x7fe9b39f8d96]
STDERR: 22  0x7fe9b3e5ebb4 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(void IPC::callMemberFunctionImpl<WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(IPC::Connection&, WTF::String const&, WebKit::UserData const&, WebKit::UserData&), std::tuple<WTF::String, WebKit::UserData>, 0ul, 1ul, std::tuple<WebKit::UserData>, 0ul>(WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(IPC::Connection&, WTF::String const&, WebKit::UserData const&, WebKit::UserData&), IPC::Connection&, std::tuple<WTF::String, WebKit::UserData>&&, std::tuple<WebKit::UserData>&, std::index_sequence<0ul, 1ul>, std::index_sequence<0ul>)+0xba) [0x7fe9b3e5ebb4]
STDERR: 23  0x7fe9b3e524fe /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(void IPC::callMemberFunction<WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(IPC::Connection&, WTF::String const&, WebKit::UserData const&, WebKit::UserData&), std::tuple<WTF::String, WebKit::UserData>, std::make_index_sequence<2ul>, std::tuple<WebKit::UserData>, std::make_index_sequence<1ul> >(IPC::Connection&, std::tuple<WTF::String, WebKit::UserData>&&, std::tuple<WebKit::UserData>&, WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(IPC::Connection&, WTF::String const&, WebKit::UserData const&, WebKit::UserData&))+0x54) [0x7fe9b3e524fe]
STDERR: 24  0x7fe9b3e4ed1f /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(void IPC::handleMessage<Messages::WebPageProxy::HandleSynchronousMessage, WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(IPC::Connection&, WTF::String const&, WebKit::UserData const&, WebKit::UserData&)>(IPC::Connection&, IPC::MessageDecoder&, IPC::MessageEncoder&, WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(IPC::Connection&, WTF::String const&, WebKit::UserData const&, WebKit::UserData&))+0xc5) [0x7fe9b3e4ed1f]
STDERR: 25  0x7fe9b3e3f00e /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebKit::WebPageProxy::didReceiveSyncMessage(IPC::Connection&, IPC::MessageDecoder&, std::unique_ptr<IPC::MessageEncoder, std::default_delete<IPC::MessageEncoder> >&)+0x1976) [0x7fe9b3e3f00e]
STDERR: 26  0x7fe9b3906330 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(IPC::MessageReceiverMap::dispatchSyncMessage(IPC::Connection&, IPC::MessageDecoder&, std::unique_ptr<IPC::MessageEncoder, std::default_delete<IPC::MessageEncoder> >&)+0x126) [0x7fe9b3906330]
STDERR: 27  0x7fe9b39b2fec /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebKit::ChildProcessProxy::dispatchSyncMessage(IPC::Connection&, IPC::MessageDecoder&, std::unique_ptr<IPC::MessageEncoder, std::default_delete<IPC::MessageEncoder> >&)+0x34) [0x7fe9b39b2fec]
STDERR: 28  0x7fe9b3a74cba /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(WebKit::WebProcessProxy::didReceiveSyncMessage(IPC::Connection&, IPC::MessageDecoder&, std::unique_ptr<IPC::MessageEncoder, std::default_delete<IPC::MessageEncoder> >&)+0x30) [0x7fe9b3a74cba]
STDERR: 29  0x7fe9b38f3417 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(IPC::Connection::dispatchSyncMessage(IPC::MessageDecoder&)+0x2c9) [0x7fe9b38f3417]
STDERR: 30  0x7fe9b38f3973 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(IPC::Connection::dispatchMessage(std::unique_ptr<IPC::MessageDecoder, std::default_delete<IPC::MessageDecoder> >)+0x141) [0x7fe9b38f3973]
STDERR: 31  0x7fe9b38f3b7a /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(IPC::Connection::dispatchOneMessage()+0xc8) [0x7fe9b38f3b7a]
STDERR: LEAK: 33 RenderObject
STDERR: LEAK: 1 Page
STDERR: LEAK: 1 Frame
STDERR: LEAK: 5 CachedResource
STDERR: LEAK: 1 SubresourceLoader
STDERR: LEAK: 118 WebCoreNode
STDERR: LEAK: 1 WebPage
STDERR: LEAK: 1 WebFrame
Comment 1 Adrien Plazas 2016-02-29 02:34:29 PST
Created attachment 272480 [details]
Remove gesture sequence from touch sequence set

This allows several non-passing tests to pass.

Some of the passing tests where expected not to pass on my machine, I don't know if that's good or bad:
- fast/events/touch/frame-hover-update.html
- fast/events/touch/touch-stale-node-crash.html
Comment 2 WebKit Commit Bot 2016-02-29 02:36:07 PST
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 WebKit Commit Bot 2016-02-29 02:36:15 PST
Attachment 272480 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:961:  Missing space before ( in if(  [whitespace/parens] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Adrien Plazas 2016-02-29 02:40:23 PST
Created attachment 272481 [details]
Remove gesture sequence from touch sequence set

This allows several failing tests to pass.

Some of the passing tests where expected not to pass on my machine, I don't know if that's good or bad:
- fast/events/touch/frame-hover-update.html
- fast/events/touch/touch-stale-node-crash.html
Comment 5 Carlos Garcia Campos 2016-02-29 03:05:05 PST
Comment on attachment 272481 [details]
Remove gesture sequence from touch sequence set

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

You should remove the tests the pass now from the TestExpectations file.

> Source/WebKit2/ChangeLog:9
> +        Once a touch event sequence is identified as a gesture, remove it
> +        from the list of handled touch events.
> +
> +        Reviewed by NOBODY (OOPS!).

We normally add the description after the Reviewed by line.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:962
> +        if (priv->touchEvents.contains(sequence))
> +            priv->touchEvents.remove(sequence);

No need to check the sequence is in the map, remove() does nothing if not present.
Comment 6 Build Bot 2016-02-29 03:36:56 PST
Comment on attachment 272481 [details]
Remove gesture sequence from touch sequence set

Attachment 272481 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/900062

New failing tests:
storage/indexeddb/odd-strings-private.html
Comment 7 Build Bot 2016-02-29 03:37:00 PST
Created attachment 272483 [details]
Archive of layout-test-results from ews117 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 8 Adrien Plazas 2016-02-29 04:03:09 PST
Created attachment 272484 [details]
Remove gesture sequence from touch sequence set
Comment 9 Carlos Garcia Campos 2016-02-29 04:07:33 PST
Comment on attachment 272484 [details]
Remove gesture sequence from touch sequence set

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

No need to ask for review again once you got an r+. Upload the new patch for landing with the review comments addressed, the Reviewed by line already filled and only ask for commit-queue

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:962
> +        if (priv->touchEvents.contains(sequence))
> +            priv->touchEvents.remove(sequence);

No need to check the sequence is in the map, remove() does nothing if not present.
Comment 10 Adrien Plazas 2016-02-29 04:12:42 PST
Created attachment 272485 [details]
Remove gesture sequence from touch sequence set
Comment 11 Adrien Plazas 2016-02-29 04:53:24 PST
Created attachment 272487 [details]
Remove gesture sequence from touch sequence set
Comment 12 Carlos Garcia Campos 2016-02-29 05:10:39 PST
Carlos Garnacho is going to take a look at the patch too, I'll cq+ once he confirms it's ok.
Comment 13 Carlos Garnacho 2016-02-29 06:08:35 PST
The patch indeed makes sense. In X11 this is not usually a problem since touch sequences are monotonically increasing, so there's little chances of collision. I guess this could be promptly seen in wayland since touch IDs are quickly reused there.
Comment 14 WebKit Commit Bot 2016-02-29 07:01:47 PST
Comment on attachment 272487 [details]
Remove gesture sequence from touch sequence set

Clearing flags on attachment: 272487

Committed r197351: <http://trac.webkit.org/changeset/197351>
Comment 15 WebKit Commit Bot 2016-02-29 07:01:53 PST
All reviewed patches have been landed.  Closing bug.