Bug 211465 – [GTK] Cleanup KeyBindingTranslator on WebView disposal

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Lauro Moura]

During the GTK4 adjustments, r260869/bug211128 made the KeyBindingTranslator hold a plain pointer to a GtkWidget, cleaned up in the `destroyed()` method.

But in some cases, the KeyBindingTranslator can be C++-destroyed before this method being called, leading to an assertion failure.

For example, running the compositing/overflow makes 14 of the 53 tests run to crash. Together with other occurrences in other tests, this is causing the Debug test bot to exit early.

Sample trace:

04:38:20.812 1398   ASSERTION FAILED: !m_nativeWidget
04:38:20.812 1398   ../../Source/WebKit/UIProcess/gtk/KeyBindingTranslator.cpp(195) : WebKit::KeyBindingTranslator::~KeyBindingTranslator()
04:38:20.812 1398   1   0x7f4903e8446d WTFCrash
04:38:20.812 1398   2   0x55a0e73743bb /app/webkit/WebKitBuild/Debug/bin/WebKitTestRunner(+0x503bb) [0x55a0e73743bb]
04:38:20.812 1398   3   0x7f4912a87d51 WebKit::KeyBindingTranslator::~KeyBindingTranslator()
04:38:20.812 1398   4   0x7f49128d3f9e _WebKitWebViewBasePrivate::~_WebKitWebViewBasePrivate()
04:38:20.812 1398   5   0x7f49128cc73e /app/webkit/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37(+0xd36173e) [0x7f49128cc73e]
04:38:20.812 1398   6   0x7f48fd092d6e g_object_unref
04:38:20.812 1398   7   0x7f48fca65479 /usr/lib/x86_64-linux-gnu/libgtk-3.so.0(+0x394479) [0x7f48fca65479]
04:38:20.812 1398   8   0x7f48fc8071bb /usr/lib/x86_64-linux-gnu/libgtk-3.so.0(+0x1361bb) [0x7f48fc8071bb]
04:38:20.812 1398   9   0x7f48fd08d862 g_closure_invoke
04:38:20.812 1398   10  0x7f48fd0a1b65 /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0(+0x28b65) [0x7f48fd0a1b65]
04:38:20.812 1398   11  0x7f48fd0acbde g_signal_emit_valist
04:38:20.812 1398   12  0x7f48fd0ad113 g_signal_emit
04:38:20.813 1398   13  0x7f48fca5362f /usr/lib/x86_64-linux-gnu/libgtk-3.so.0(+0x38262f) [0x7f48fca5362f]
04:38:20.813 1398   14  0x7f48fca68790 /usr/lib/x86_64-linux-gnu/libgtk-3.so.0(+0x397790) [0x7f48fca68790]
04:38:20.813 1398   15  0x7f48fd094531 g_object_run_dispose
04:38:20.813 1398   16  0x55a0e73e0337 /app/webkit/WebKitBuild/Debug/bin/WebKitTestRunner(+0xbc337) [0x55a0e73e0337]
04:38:20.813 1398   17  0x55a0e73a1cea /app/webkit/WebKitBuild/Debug/bin/WebKitTestRunner(+0x7dcea) [0x55a0e73a1cea]
04:38:20.813 1398   18  0x55a0e73a35f7 /app/webkit/WebKitBuild/Debug/bin/WebKitTestRunner(+0x7f5f7) [0x55a0e73a35f7]
04:38:20.813 1398   19  0x55a0e739e87f /app/webkit/WebKitBuild/Debug/bin/WebKitTestRunner(+0x7a87f) [0x55a0e739e87f]
04:38:20.813 1398   20  0x55a0e73871ee /app/webkit/WebKitBuild/Debug/bin/WebKitTestRunner(+0x631ee) [0x55a0e73871ee]
04:38:20.813 1398   21  0x55a0e738b015 /app/webkit/WebKitBuild/Debug/bin/WebKitTestRunner(+0x67015) [0x55a0e738b015]
04:38:20.813 1398   22  0x55a0e73b76a5 /app/webkit/WebKitBuild/Debug/bin/WebKitTestRunner(+0x936a5) [0x55a0e73b76a5]
04:38:20.813 1398   23  0x55a0e738c3a8 /app/webkit/WebKitBuild/Debug/bin/WebKitTestRunner(+0x683a8) [0x55a0e738c3a8]
04:38:20.813 1398   24  0x55a0e738c5d3 /app/webkit/WebKitBuild/Debug/bin/WebKitTestRunner(+0x685d3) [0x55a0e738c5d3]
04:38:20.813 1398   25  0x55a0e738c628 /app/webkit/WebKitBuild/Debug/bin/WebKitTestRunner(+0x68628) [0x55a0e738c628]
04:38:20.813 1398   26  0x55a0e7384c10 /app/webkit/WebKitBuild/Debug/bin/WebKitTestRunner(+0x60c10) [0x55a0e7384c10]
04:38:20.813 1398   27  0x55a0e73e400f /app/webkit/WebKitBuild/Debug/bin/WebKitTestRunner(+0xc000f) [0x55a0e73e400f]
04:38:20.813 1398   28  0x7f48fc01b183 __libc_start_main
04:38:20.813 1398   29  0x55a0e737403e /app/webkit/WebKitBuild/Debug/bin/WebKitTestRunner(+0x5003e) [0x55a0e737403e]

[Lauro Moura]

Created attachment 398545 [details]
Tentative fix

[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 http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Adrian Perez]

Comment on attachment 398545 [details]
Tentative fix

Change LGTM, but I will wait for Carlos García to give the r+
because it won't hurt to have another pair of eyes look at this :]

[EWS]

Committed r261230: <https://trac.webkit.org/changeset/261230>

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