Bug 235694 – [GTK] Crash when starting DND on touchscreen

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Alice Mikhaylenko]

(MiniBrowser:20): Gdk-CRITICAL **: 23:56:59.312: gdk_wayland_window_get_wl_surface: assertion 'GDK_IS_WAYLAND_WINDOW (window)' failed
Gdk-Message: 23:56:59.314: Error 22 (Invalid argument) dispatching to Wayland display.



Thread 1 "MiniBrowser" hit Breakpoint 1, gdk_wayland_window_get_wl_surface (window=0x555556242250 [GdkWaylandWindow]) at ../gdk/wayland/gdkwindow-wayland.c:4882
4882	{
(gdb) bt
#0  gdk_wayland_window_get_wl_surface (window=0x555556242250 [GdkWaylandWindow]) at ../gdk/wayland/gdkwindow-wayland.c:4882
#1  0x00007ffff2336c26 in _gdk_wayland_window_drag_begin
    (window=0x5555562420b0 [GdkWaylandWindow], device=<optimized out>, targets=0x555555787620 = {...}, x_root=<optimized out>, y_root=<optimized out>)
    at ../gdk/wayland/gdkdnd-wayland.c:539
#2  0x00007ffff27a58fe in gtk_drag_begin_internal
    (widget=widget@entry=0x55555598d730 [WebKitWebView], out_needs_icon=out_needs_icon@entry=0x0, target_list=target_list@entry=0x5555559f6b30, actions=actions@entry=(GDK_ACTION_COPY | GDK_ACTION_MOVE | GDK_ACTION_LINK), button=button@entry=1, event=event@entry=0x0, x=<optimized out>, y=<optimized out>) at ../gtk/gtkdnd.c:1828
#3  0x00007ffff27a5e60 in gtk_drag_begin_with_coordinates
    (widget=0x55555598d730 [WebKitWebView], targets=0x5555559f6b30, actions=(GDK_ACTION_COPY | GDK_ACTION_MOVE | GDK_ACTION_LINK), button=1, event=0x0, x=-1, y=-1)
    at ../gtk/gtkdnd.c:1995
#4  0x00007ffff3abcb3d in WebKit::DragSource::begin(WebCore::SelectionData&&, WTF::OptionSet<WebCore::DragOperation>, WTF::RefPtr<WebKit::ShareableBitmap, WTF::RawPtrTraits<WebKit::ShareableBitmap>, WTF::DefaultRefDerefTraits<WebKit::ShareableBitmap> >&&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#5  0x00007ffff3acd9d6 in webkitWebViewBaseStartDrag(_WebKitWebViewBase*, WebCore::SelectionData&&, WTF::OptionSet<WebCore::DragOperation>, WTF::RefPtr<WebKit::ShareableBitmap, WTF::RawPtrTraits<WebKit::ShareableBitmap>, WTF::DefaultRefDerefTraits<WebKit::ShareableBitmap> >&&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#6  0x00007ffff39a4501 in WebKit::WebPageProxy::startDrag(WebCore::SelectionData&&, WTF::OptionSet<WebCore::DragOperation>, WebKit::ShareableBitmap::Handle const&) ()
    at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#7  0x00007ffff3605f5a in void IPC::handleMessage<Messages::WebPageProxy::StartDrag, WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(WebCore::SelectionData&&, WTF::OptionSet<WebCore::DragOperation>, WebKit::ShareableBitmap::Handle const&)>(IPC::Connection&, IPC::Decoder&, WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(WebCore::SelectionData&&, WTF::OptionSet<WebCore::DragOperation>, WebKit::ShareableBitmap::Handle const&)) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#8  0x00007ffff35e8e2b in WebKit::WebPageProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#9  0x00007ffff38a920b in IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#10 0x00007ffff3996b4f in non-virtual thunk to WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&) ()
    at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#11 0x00007ffff38a1775 in IPC::Connection::dispatchMessage(std::unique_ptr<IPC::Decoder, std::default_delete<IPC::Decoder> >) ()
    at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#12 0x00007ffff38a3f7b in IPC::Connection::dispatchIncomingMessages() () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#13 0x00007ffff1ba3454 in WTF::RunLoop::performWork() () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.1.so.0
#14 0x00007ffff1c15c79 in WTF::RunLoop::RunLoop()::{lambda(void*)#1}::_FUN(void*) () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.1.so.0
#15 0x00007ffff1c1673f in WTF::RunLoop::{lambda(_GSource*, int (*)(void*), void*)#1}::_FUN(_GSource*, int (*)(void*), void*) ()
    at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.1.so.0
#16 0x00007ffff7ecf294 in g_main_dispatch (context=0x5555556086b0) at ../glib/gmain.c:3381
#17 g_main_context_dispatch (context=0x5555556086b0) at ../glib/gmain.c:4099
#18 0x00007ffff7ecf638 in g_main_context_iterate (context=context@entry=0x5555556086b0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:4175
#19 0x00007ffff7ecf703 in g_main_context_iteration (context=context@entry=0x5555556086b0, may_block=may_block@entry=1) at ../glib/gmain.c:4240
#20 0x00007fffece47765 in g_application_run (application=0x555555663210 [GtkApplication], argc=-7484, argv=<optimized out>) at ../gio/gapplication.c:2569
#21 0x000055555556487c in main ()
(gdb) 


It doesn't crash in XWayland, though DND gets stuck instead. Doesn't seem to happen at all on GTK4.

[Alice Mikhaylenko]

Created attachment 450516 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Carlos Garcia Campos]

Do we have a way to detect if a PlatformMouseEvent is a touchscreen event? Maybe we can prevent the drag from starting already in the web process, in EventHandler::handleMousePressEvent() setting m_mouseDownMayStartDrag = false. Is it right to assume all synthesized events are touchscreen events?

[Alice Mikhaylenko]

Hm no, it's not right, I once again forgot to check other places where it's used, outside WebKitWebView.

I guess then we can do a similar thing as for https://bugs.webkit.org/show_bug.cgi?id=235436 and have an extra parameter for webkitWebViewBaseSynthesizeMouseEvent(), and a GTK-specific PlatformWebMouseEvent field to store this.

[Alice Mikhaylenko]

Created attachment 450630 [details]
Patch

[Alice Mikhaylenko]

Random EWS failures? macOS fixed itself, now iOS fails instead...

[Adrian Perez]

Patch looks good to me, but I would prefer if someone else who has
better touch-input-fu could also take a look and review this :)

[Carlos Garcia Campos]

Comment on attachment 450630 [details]
Patch

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

LGTM

> Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:1936
> +    webkitWebViewBaseSynthesizeMouseEvent(webViewBase, MouseEventType::Motion, 0, 0, x, y, modifiers, nPress, mousePointerEventType(), true);
> +    webkitWebViewBaseSynthesizeMouseEvent(webViewBase, MouseEventType::Press, button, 0, x, y, modifiers, nPress, mousePointerEventType(), true);
> +    webkitWebViewBaseSynthesizeMouseEvent(webViewBase, MouseEventType::Release, button, buttons, x, y, modifiers, nPress, mousePointerEventType(), true);

This would be easier to read using an enum class instead of a boolean, PlatformMouseEvent::IsTouch::Yes, for example.

[Alice Mikhaylenko]

Created attachment 450898 [details]
Patch

[Alice Mikhaylenko]

Created attachment 450899 [details]
Patch

[EWS]

Committed r289129 (246826@main): <https://commits.webkit.org/246826@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 450899 [details].