NEW 166059
[GTK] Crash on drag above web view of nautilus' tab 
https://bugs.webkit.org/show_bug.cgi?id=166059
Summary [GTK] Crash on drag above web view of nautilus' tab
Milan Crha
Reported 2016-12-20 06:52:47 PST
This had been reported downstream against evolution: https://bugzilla.gnome.org/show_bug.cgi?id=776224 When a user adds a tab into nautilus and then drops it above message preview of the evolution, then it causes a crash in the WebKit with this backtrace: Thread 1 "evolution" received signal SIGSEGV, Segmentation fault. 0x00007fffefb91d5d in WebKit::DragAndDropHandler::drop(_GdkDragContext*, WebCore::IntPoint const&, unsigned int) () from /lib64/libwebkit2gtk-4.0.so.37 (gdb) bt full #0 0x00007fffefb91d5d in WebKit::DragAndDropHandler::drop(_GdkDragContext*, WebCore::IntPoint const&, unsigned int) () at /lib64/libwebkit2gtk-4.0.so.37 #1 0x00007fffefb7703c in webkitWebViewBaseDragDrop(_GtkWidget*, _GdkDragContext*, int, int, unsigned int) () at /lib64/libwebkit2gtk-4.0.so.37 #2 0x00007fffed8f13d7 in _gtk_marshal_BOOLEAN__OBJECT_INT_INT_UINT () at /lib64/libgtk-3.so.0 #6 0x00007fffec0408eb in <emit signal 0x7fffedabe22c "drag-drop" on instance 0x5555569cdbb0 [EMailDisplay]> (instance=0x5555569cdbb0, detailed_signal=0x7fffedabe22c "drag-drop") at gsignal.c:3487 var_args = {{gp_offset = 48, fp_offset = 48, overflow_arg_area = 0x7fffffffe1a0, reg_save_area = 0x7fffffffe0b0}} detail = 0 itype = 93825002162336 __func__ = "g_signal_emit_by_name" #3 0x00007fffec0253e5 in g_closure_invoke (closure=closure@entry=0x5555557eaa10, return_value=return_value@entry=0x7fffffffdda0, n_param_values=5, param_values=param_values@entry=0x7fffffffde00, invocation_hint=invocation_hint@entry=0x7fffffffdd80) at gclosure.c:804 marshal = <optimized out> marshal_data = <optimized out> in_marshal = 0 real_closure = 0x5555557ea9f0 __func__ = "g_closure_invoke" #4 0x00007fffec03782d in signal_emit_unlocked_R (node=node@entry=0x5555557eaa40, detail=detail@entry=0, instance=instance@entry=0x5555569cdbb0, emission_return=emission_return@entry=0x7fffffffdf70, instance_and_params=instance_and_params@entry=0x7fffffffde00) at gsignal.c:3673 accumulator = 0x555555782440 emission = {next = 0x0, instance = 0x5555569cdbb0, ihint = {signal_id = 91, detail = 0, run_type = G_SIGNAL_RUN_LAST}, state = EMISSION_RUN, chain_type = 93825002162336} class_closure = 0x5555557eaa10 handler_list = <optimized out> return_accu = 0x7fffffffdda0 accu = {g_type = 20, data = {{v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}, {v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}}} signal_id = 91 max_sequential_handler_number = 62687 return_value_altered = 0 #5 0x00007fffec03fb8f in g_signal_emit_valist (instance=instance@entry=0x5555569cdbb0, signal_id=signal_id@entry=91, detail=detail@entry=0, var_args=var_args@entry=0x7fffffffe068) at gsignal.c:3401 return_value = {g_type = 20, data = {{v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}, {v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}}} error = 0x0 rtype = 20 static_scope = 0 instance_and_params = 0x7fffffffde00 signal_return_type = <optimized out> param_values = 0x7fffffffde18 node = <optimized out> i = <optimized out> n_params = <optimized out> __func__ = "g_signal_emit_valist" #7 0x00007fffeda69b57 in gtk_drag_dest_drop () at /lib64/libgtk-3.so.0 #8 0x00007fffeda6a6d0 in _gtk_drag_dest_handle_event () at /lib64/libgtk-3.so.0 #9 0x00007fffed8ef33a in gtk_main_do_event () at /lib64/libgtk-3.so.0 #10 0x00007fffed406485 in _gdk_event_emit () at /lib64/libgdk-3.so.0 #11 0x00007fffed437452 in gdk_event_source_dispatch () at /lib64/libgdk-3.so.0 #12 0x00007fffebd4ce42 in g_main_dispatch (context=0x5555557ddeb0) at gmain.c:3203 dispatch = 0x7fffed437430 <gdk_event_source_dispatch> prev_source = 0x0 was_in_call = 0 user_data = 0x0 callback = 0x0 cb_funcs = 0x0 cb_data = 0x0 need_destroy = <optimized out> source = 0x5555557bed60 current = 0x555555821eb0 i = 0 #13 0x00007fffebd4ce42 in g_main_context_dispatch (context=context@entry=0x5555557ddeb0) at gmain.c:3856 #14 0x00007fffebd4d1c0 in g_main_context_iterate (context=0x5555557ddeb0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3929 max_priority = 2147483647 timeout = 31 some_ready = 1 nfds = 4 allocated_nfds = 4 fds = <optimized out> #15 0x00007fffebd4d4e2 in g_main_loop_run (loop=0x555556c5cab0) at gmain.c:4125 __func__ = "g_main_loop_run" #16 0x00007fffed8ee655 in gtk_main () at /lib64/libgtk-3.so.0 #17 0x0000555555557e05 in main (argc=<optimized out>, argv=<optimized out>) at main.c:665 settings = <optimized out> success = <optimized out> error = 0x0 ------------------------------------------------------------------------------- I also used gdb with this output (note of a NULL 'this' pointer there, which seems that Fedora either has disabled asserts or the test checks that the structure is not NULL, but not its content): Thread 1 "evolution" hit Breakpoint 1, webkitWebViewBaseDragDrop (widget=0x36ae320 [EMailDisplay], context=0x32898a0, x=30, y=225, time=20018240) at /usr/src/debug/webkitgtk-2.14.2/Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:1054 1054 return priv->dragAndDropHandler->drop(context, IntPoint(x, y), time); (gdb) p priv $2 = (WebKitWebViewBasePrivate *) 0x36ae040 (gdb) p priv->dragAndDropHandler $3 = std::unique_ptr<WebKit::DragAndDropHandler> containing 0x0 (gdb) s WebCore::IntPoint::IntPoint (y=225, x=30, this=0x7fffffffd2e0) at /usr/src/debug/webkitgtk-2.14.2/Source/WebCore/platform/graphics/IntPoint.h:64 64 IntPoint(int x, int y) : m_x(x), m_y(y) { } (gdb) n webkitWebViewBaseDragDrop (widget=0x36ae320 [EMailDisplay], context=0x32898a0, x=30, y=225, time=20018240) at /usr/src/debug/webkitgtk-2.14.2/Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:1054 1054 return priv->dragAndDropHandler->drop(context, IntPoint(x, y), time); (gdb) s WebKit::DragAndDropHandler::drop (this=0x0, context=context@entry=0x32898a0, position=..., time=time@entry=20018240) at /usr/src/debug/webkitgtk-2.14.2/Source/WebKit2/UIProcess/gtk/DragAndDropHandler.cpp:285 285 { (gdb) n 286 DroppingContext* droppingContext = m_droppingContexts.get(context); (gdb) 285 { (gdb) 286 DroppingContext* droppingContext = m_droppingContexts.get(context); (gdb) Thread 1 "evolution" received signal SIGSEGV, Segmentation fault. WTF::HashTable<_GdkDragContext*, WTF::KeyValuePair<_GdkDragContext*, std::unique_ptr<WebKit::DragAndDropHandler::DroppingContext, std::default_delete<WebKit::DragAndDropHandler::DroppingContext> > >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<_GdkDragContext*, std::unique_ptr<WebKit::DragAndDropHandler::DroppingContext, std::default_delete<WebKit::DragAndDropHandler::DroppingContext> > > >, WTF::PtrHash<_GdkDragContext*>, WTF::HashMap<_GdkDragContext*, std::unique_ptr<WebKit::DragAndDropHandler::DroppingContext, std::default_delete<WebKit::DragAndDropHandler::DroppingContext> >, WTF::PtrHash<_GdkDragContext*>, WTF::HashTraits<_GdkDragContext*>, WTF::HashTraits<std::unique_ptr<WebKit::DragAndDropHandler::DroppingContext, std::default_delete<WebKit::DragAndDropHandler::DroppingContext> > > >::KeyValuePairTraits, WTF::HashTraits<_GdkDragContext*> >::inlineLookup<WTF::IdentityHashTranslator<WTF::HashMap<_GdkDragContext*, std::unique_ptr<WebKit::DragAndDropHandler::DroppingContext, std::default_delete<WebKit::DragAndDropHandler::DroppingContext> >, WTF::PtrHash<_GdkDragContext*>, WTF::HashTraits<_GdkDragContext*>, WTF::HashTraits<std::unique_ptr<WebKit::DragAndDropHandler::DroppingContext, std::default_delete<WebKit::DragAndDropHandler::DroppingContext> > > >::KeyValuePairTraits, WTF::PtrHash<_GdkDragContext*> >, _GdkDragContext*> (key=<optimized out>, this=<optimized out>) at /usr/src/debug/webkitgtk-2.14.2/Source/WTF/wtf/HashTable.h:611 611 unsigned sizeMask = m_tableSizeMask; ------------------------------------------------------------------------------- Note that the EMailDisplay is a descendant of EWebView, which is a descendant of WebKitWebView. The EWebView overrides GtkWidgetClass::drag_motion and simply returns FALSE, which is meant to indicate that the widget is not used as a drop zone, but for some reason (I suspect due to WebKit), the widget finally claims to be a drop zone (based on the mouse cursor). To workaround that I changed the body of the overridden function and made it return TRUE, while also calling gdk_drag_status (context, 0, time_);. That fixes the drop zone detection, but crashes in drag-leave, when I move away from the widget, again in the WebKit code, unless I also redefine GtkWidgetClass::drag_leave function. To have it complete, there's also redefined a GtkWidgetClass::drag_drop function to avoid the crash there (see the downstream bug report). I hope it is only a temporary change. Interestingly, neither Epiphany, nor MiniBrowser, suffer of this. Could it be because they both do something in those drag-related callbacks on their own, thus the default implementations in the WebKitWebViewBase are not called? I'm only guessing here and I definitely do not want to mislead your investigation of the cause with it.
Attachments
Add attachment proposed patch, testcase, etc.
Michael Catanzaro
Comment 1 2016-12-20 08:32:38 PST
(Note: you can't check if 'this' is NULL in C++, because it's illegal for that to ever happen; at that point you've got undefined behavior and have already lost. Seems that's happening here. Unless you're building with -O0, the compiler will just optimize away the check. Also note that yes, WebKit ASSERTs are all disabled in release builds as WebKit is full of bugs and the web process would be constantly crashing if they were enabled.)
Milan Crha
Comment 2 2016-12-21 01:29:58 PST
Yes, I wouldn't suggest to check for `this != NULL`, illegal or not, it looks and feels awkward and like a very bad hack. What I've on my mind is to change the asserts into a real runtime check, instead of "check only in debug build, or whatever". That will make the code more robust. And if you feel that it'll hide some issues, then keep there both the assert and the runtime check, and eventually print a runtime warning if the runtime check fails, if you think that the DragAndDropHandler cannot be NULL in all cases. In other words, the change should come, from my point of view, into webkitWebViewBaseDragDrop() and the related drag & drop functions, instead of into the DragAndDropHandler class.
Note You need to log in before you can comment on or make changes to this bug.