Bug 216727

Summary: [GTK] REGRESSION(r267250) API test /webkit/WebKitWebView/usermedia-enumeratedevices-permission-check is crashing
Product: WebKit Reporter: Diego Pino <dpino>
Component: WebKitGTKAssignee: Lauro Moura <lmoura>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, bugs-noreply, cgarcia, ews-watchlist, gustavo, lmoura, pnormand, sihui_liu, vjaquez
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Updated using dispose
none
Patch for landing none

Description Diego Pino 2020-09-18 23:25:40 PDT 
According to post-commit bot log, the test started crashing in the interval [r267249-r267251]. Within the range, the most likely cause for the regression seems to be r267250. 

Unexpected crashes (1)
    /WebKit2Gtk/TestUIClient
        /webkit/WebKitWebView/usermedia-enumeratedevices-permission-check

ERROR:../../Tools/TestWebKitAPI/Tests/WebKitGLib/TestUIClient.cpp:817:void testWebViewMouseTarget(UIClientTest *, gconstpointer): 'webkit_hit_test_result_context_is_media(hitTestResult)' should be TRUE
  /webkit/WebKitWebView/geolocation-permission-requests:              PASS
GLib-GIO-DEBUG: _g_io_module_get_default: Found default implementation local (GLocalVfs) for ‘gio-vfs’
  /webkit/WebKitWebView/usermedia-enumeratedevices-permission-check:  CRASH

See: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Tests%29/builds/15910/steps/API%20tests/logs/stdio
Comment 1 Diego Pino 2020-09-18 23:46:24 PDT 
The interval is actually [r267250-r267251]. Considering r267251 is a WPT re-sync the cause has to be r267250.
Comment 2 Philippe Normand 2020-09-21 04:34:07 PDT 
Can you share the backtrace?
Comment 3 Philippe Normand 2020-10-07 06:37:40 PDT 
Seems related with the geolocation test running just before:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  g_type_check_instance (type_instance=type_instance@entry=0x216ee20) at ../gobject/gtype.c:4134
4134		  TypeNode *node = lookup_type_node_I (type_instance->g_class->g_type);
[Current thread is 1 (Thread 0x7fdebd5999c0 (LWP 852))]
(gdb) bt
#0  g_type_check_instance (type_instance=type_instance@entry=0x216ee20) at ../gobject/gtype.c:4134
#1  0x00007fdebeec7bff in g_signal_emit_valist (instance=0x216ee20, signal_id=182, detail=0, var_args=var_args@entry=0x7ffc4d560210) at ../gobject/gsignal.c:3273
#2  0x00007fdebeec8d43 in g_signal_emit (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>) at ../gobject/gsignal.c:3550
#3  0x00007fdec65448b9 in GeolocationProvider::stopUpdating(WebKit::WebGeolocationManagerProxy&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#4  0x00007fdec64722bb in WebKit::WebGeolocationManagerProxy::stopUpdating(IPC::Connection&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#5  0x00007fdec6199269 in WebKit::WebGeolocationManagerProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#6  0x00007fdec63d8dff in IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#7  0x00007fdec64d2a91 in non-virtual thunk to WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#8  0x00007fdec63d2de9 in IPC::Connection::dispatchMessage(std::unique_ptr<IPC::Decoder, std::default_delete<IPC::Decoder> >) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#9  0x00007fdec63d230f in IPC::Connection::dispatchIncomingMessages() () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37
#10 0x00007fdec3c13856 in WTF::RunLoop::performWork() () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#11 0x00007fdec3c6e7c6 in WTF::RunLoop::RunLoop()::$_1::__invoke(void*) () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#12 0x00007fdec3c6dcba in WTF::RunLoop::$_0::__invoke(_GSource*, int (*)(void*), void*) () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18
#13 0x00007fdec9bc504f in g_main_dispatch (context=0x200bb30) at ../glib/gmain.c:3325
#14 g_main_context_dispatch (context=0x200bb30) at ../glib/gmain.c:4016
#15 0x00007fdec9bc53f8 in g_main_context_iterate (context=context@entry=0x200bb30, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:4092
#16 0x00007fdec9bc54c3 in g_main_context_iteration (context=0x200bb30, may_block=1) at ../glib/gmain.c:4157
#17 0x00000000004106bc in  () at elf-init.c:87
#18 0x0000000000000002 in  ()
#19 0x000000000247e3f0 in  ()
#20 0x00000000024812a0 in  ()
#21 0x00000000020d4f80 in  ()
#22 0x00000000023bce80 in  ()
#23 0x0000000000000000 in  ()
Comment 4 Philippe Normand 2020-10-07 06:53:30 PDT 
#0  g_type_check_instance (type_instance=type_instance@entry=0x1535620) at ../gobject/gtype.c:4134
4134		  TypeNode *node = lookup_type_node_I (type_instance->g_class->g_type);
[Current thread is 1 (Thread 0x7f7b6093a9c0 (LWP 853))]
(gdb) bt
#0  g_type_check_instance (type_instance=type_instance@entry=0x1535620) at ../gobject/gtype.c:4134
#1  0x00007f7b62268bff in g_signal_emit_valist (instance=0x1535620, signal_id=182, detail=0, var_args=var_args@entry=0x7ffd50133d50) at ../gobject/gsignal.c:3273
#2  0x00007f7b62269d43 in g_signal_emit (instance=<optimized out>, signal_id=<optimized out>, detail=<optimized out>) at ../gobject/gsignal.c:3550
#3  0x00007f7b76f6662a in webkitGeolocationManagerStop(_WebKitGeolocationManager*) (manager=0x1535620) at ../../Source/WebKit/UIProcess/API/glib/WebKitGeolocationManager.cpp:269
#4  0x00007f7b76f66fdd in GeolocationProvider::stopUpdating(WebKit::WebGeolocationManagerProxy&) (this=0x7f7b600fa1a0) at ../../Source/WebKit/UIProcess/API/glib/WebKitGeolocationManager.cpp:301
#5  0x00007f7b76d0229e in WebKit::WebGeolocationManagerProxy::removeRequester(IPC::Connection::Client const*) (this=0x7f7b600e7000, client=0x7f7afc2fc488)
    at ../../Source/WebKit/UIProcess/WebGeolocationManagerProxy.cpp:149
#6  0x00007f7b76d02820 in WebKit::WebGeolocationManagerProxy::stopUpdating(IPC::Connection&) (this=0x7f7b600e7000, connection=...) at ../../Source/WebKit/UIProcess/WebGeolocationManagerProxy.cpp:137
#7  0x00007f7b76340f5c in IPC::callMemberFunctionImpl<WebKit::WebGeolocationManagerProxy, void (WebKit::WebGeolocationManagerProxy::*)(IPC::Connection&), std::tuple<>>(WebKit::WebGeolocationManagerProxy*, void (WebKit::WebGeolocationManagerProxy::*)(IPC::Connection&), IPC::Connection&, std::tuple<>&&, std::integer_sequence<unsigned long>) (object=0x7f7b600e7000, function=
    (void (WebKit::WebGeolocationManagerProxy::*)(class WebKit::WebGeolocationManagerProxy * const, class IPC::Connection &)) 0x7f7b76d027f0 <WebKit::WebGeolocationManagerProxy::stopUpdating(IPC::Connection&)>, connection=..., args=...) at ../../Source/WebKit/Platform/IPC/HandleMessage.h:83
#8  0x00007f7b76340ec0 in IPC::callMemberFunction<WebKit::WebGeolocationManagerProxy, void (WebKit::WebGeolocationManagerProxy::*)(IPC::Connection&), std::tuple<>, std::integer_sequence<unsigned long> >(IPC::Connection&, std::tuple<>&&, WebKit::WebGeolocationManagerProxy*, void (WebKit::WebGeolocationManagerProxy::*)(IPC::Connection&)) (connection=..., args=..., object=0x7f7b600e7000, function=
    (void (WebKit::WebGeolocationManagerProxy::*)(class WebKit::WebGeolocationManagerProxy * const, class IPC::Connection &)) 0x7f7b76d027f0 <WebKit::WebGeolocationManagerProxy::stopUpdating(IPC::Connection&)>)
    at ../../Source/WebKit/Platform/IPC/HandleMessage.h:89
#9  0x00007f7b7633ffb9 in IPC::handleMessage<Messages::WebGeolocationManagerProxy::StopUpdating, WebKit::WebGeolocationManagerProxy, void (WebKit::WebGeolocationManagerProxy::*)(IPC::Connection&)>(IPC::Connection&, IPC::Decoder&, WebKit::WebGeolocationManagerProxy*, void (WebKit::WebGeolocationManagerProxy::*)(IPC::Connection&)) (connection=..., decoder=..., object=0x7f7b600e7000, function=
    (void (WebKit::WebGeolocationManagerProxy::*)(class WebKit::WebGeolocationManagerProxy * const, class IPC::Connection &)) 0x7f7b76d027f0 <WebKit::WebGeolocationManagerProxy::stopUpdating(IPC::Connection&)>)
    at ../../Source/WebKit/Platform/IPC/HandleMessage.h:132
#10 0x00007f7b7633fd0a in WebKit::WebGeolocationManagerProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&) (this=0x7f7b600e7000, connection=..., decoder=...)
    at DerivedSources/WebKit/WebGeolocationManagerProxyMessageReceiver.cpp:46
#11 0x00007f7b76b47cfc in IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) (this=0x7f7b600f4038, connection=..., decoder=...)
    at ../../Source/WebKit/Platform/IPC/MessageReceiverMap.cpp:118
#12 0x00007f7b76d6636e in WebKit::WebProcessPool::dispatchMessage(IPC::Connection&, IPC::Decoder&) (this=0x7f7b600f4000, connection=..., decoder=...) at ../../Source/WebKit/UIProcess/WebProcessPool.cpp:1377
#13 0x00007f7b76d6bbd3 in WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&) (this=0x7f7afc2fc480, connection=..., decoder=...) at ../../Source/WebKit/UIProcess/WebProcessProxy.cpp:796
#14 0x00007f7b76b23a3f in IPC::Connection::dispatchMessage(IPC::Decoder&) (this=0x7f7b600ce1c0, decoder=...) at ../../Source/WebKit/Platform/IPC/Connection.cpp:1001
#15 0x00007f7b76b23fd0 in IPC::Connection::dispatchMessage(std::unique_ptr<IPC::Decoder, std::default_delete<IPC::Decoder> >) (this=0x7f7b600ce1c0, message=std::unique_ptr<class IPC::Decoder> = {...})
    at ../../Source/WebKit/Platform/IPC/Connection.cpp:1070
#16 0x00007f7b76b22ee1 in IPC::Connection::dispatchIncomingMessages() (this=0x7f7b600ce1c0) at ../../Source/WebKit/Platform/IPC/Connection.cpp:1174
#17 0x00007f7b76b2986f in IPC::Connection::MessagesThrottler::scheduleMessagesDispatch()::$_10::operator()() (this=0x7f7b6007fc20) at ../../Source/WebKit/Platform/IPC/Connection.cpp:1106
#18 0x00007f7b76b297de in WTF::Detail::CallableWrapper<IPC::Connection::MessagesThrottler::scheduleMessagesDispatch()::$_10, void>::call() (this=0x7f7b6007fc18)
    at DerivedSources/ForwardingHeaders/wtf/Function.h:52
#19 0x00007f7b75fd9a82 in WTF::Function<void ()>::operator()() const (this=0x7ffd50134408) at DerivedSources/ForwardingHeaders/wtf/Function.h:83
#20 0x00007f7b698d2995 in WTF::RunLoop::performWork() (this=0x7f7b600f9000) at ../../Source/WTF/wtf/RunLoop.cpp:123
#21 0x00007f7b6996ba5c in WTF::RunLoop::RunLoop()::$_1::operator()(void*) const (this=0x7f7b600f9000, userData=0x7f7b600f9000) at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:80
#22 0x00007f7b6996ba35 in WTF::RunLoop::RunLoop()::$_1::__invoke(void*) (userData=0x7f7b600f9000) at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:79
#23 0x00007f7b6996b9e9 in WTF::RunLoop::$_0::operator()(_GSource*, int (*)(void*), void*) const
    (this=0x14beaa0, source=0x14beaa0, callback=0x7f7b6996ba20 <WTF::RunLoop::RunLoop()::$_1::__invoke(void*)>, userData=0x7f7b600f9000) at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:53
#24 0x00007f7b6996aa65 in WTF::RunLoop::$_0::__invoke(_GSource*, int (*)(void*), void*) (source=0x14beaa0, callback=0x7f7b6996ba20 <WTF::RunLoop::RunLoop()::$_1::__invoke(void*)>, userData=0x7f7b600f9000)
    at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:45
#25 0x00007f7b81eb804f in g_main_dispatch (context=0x13d2ca0) at ../glib/gmain.c:3325
#26 g_main_context_dispatch (context=0x13d2ca0) at ../glib/gmain.c:4016
#27 0x00007f7b81eb83f8 in g_main_context_iterate (context=context@entry=0x13d2ca0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:4092
#28 0x00007f7b81eb84c3 in g_main_context_iteration (context=0x13d2ca0, may_block=1) at ../glib/gmain.c:4157
#29 0x000000000042ad2b in WebViewTest::showInWindow(int, int) (this=0x13eba40, width=0, height=0) at ../../Tools/TestWebKitAPI/glib/WebKitGLib/gtk/WebViewTestGtk.cpp:88
#30 0x00000000004148a6 in testWebViewUserMediaEnumerateDevicesPermissionCheck(UIClientTest*, void const*) (test=0x13eba40) at ../../Tools/TestWebKitAPI/Tests/WebKitGLib/TestUIClient.cpp:908
#31 0x00007f7b81ee092e in test_case_run (tc=0x1539960) at ../glib/gtestutils.c:2652
#32 g_test_run_suite_internal (suite=suite@entry=0x153b0e0, path=path@entry=0x0) at ../glib/gtestutils.c:2740
#33 0x00007f7b81ee072b in g_test_run_suite_internal (suite=suite@entry=0x153b100, path=path@entry=0x0) at ../glib/gtestutils.c:2752
#34 0x00007f7b81ee072b in g_test_run_suite_internal (suite=suite@entry=0x153b1a0, path=path@entry=0x0) at ../glib/gtestutils.c:2752
--Type <RET> for more, q to quit, c to continue without paging--
#35 0x00007f7b81ee0e1a in g_test_run_suite (suite=0x153b1a0) at ../glib/gtestutils.c:2827
#36 0x00007f7b81ee0e35 in g_test_run () at ../glib/gtestutils.c:2061
#37 0x0000000000421141 in main(int, char**) (argc=1, argv=0x7ffd50134b68) at ../../Tools/TestWebKitAPI/glib/WebKitGLib/TestMain.cpp:138
(gdb) f 3
#3  0x00007f7b76f6662a in webkitGeolocationManagerStop (manager=0x1535620) at ../../Source/WebKit/UIProcess/API/glib/WebKitGeolocationManager.cpp:269
269	    g_signal_emit(manager, signals[STOP], 0, nullptr);
(gdb) p manager
$1 = 0x1535620
(gdb) p *manager
$2 = {parent = {g_type_instance = {g_class = <error reading variable: Cannot access memory at address 0x1>}, ref_count = 25341984, qdata = 0x1823150}, priv = 0x1535600}
Comment 5 Lauro Moura 2020-11-05 19:28:53 PST 
Created attachment 413385 [details]
Patch
Comment 6 EWS Watchlist 2020-11-05 19:29:42 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 https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 7 Carlos Garcia Campos 2020-11-05 21:48:56 PST 
Comment on attachment 413385 [details]
Patch

I don't think this is the right fix. The provider is set on construction, and it doesn't make sense to keep it alive after the manager is deleted, so I think we should just call setProvider(nullptr) on WebKitGeolocationManager dispose
Comment 8 Lauro Moura 2020-11-06 07:59:40 PST 
Created attachment 413431 [details]
Updated using dispose
Comment 9 Carlos Garcia Campos 2020-11-07 00:13:46 PST 
Comment on attachment 413431 [details]
Updated using dispose

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

> Source/WebKit/ChangeLog:16
> +        (webkit_geolocation_manager_class_init): Register dispose CB.

Don't use abbreviations, I guess CB means callback in this case?

> Source/WebKit/UIProcess/API/glib/WebKitGeolocationManager.cpp:336
> +    if (manager->priv->manager)
> +        manager->priv->manager->setProvider(nullptr);

How can be manager->priv->manager nullptr at this point? we take a reference on construction.
Comment 10 Lauro Moura 2020-11-08 19:55:29 PST 
Created attachment 413554 [details]
Patch for landing
Comment 11 EWS 2020-11-08 20:26:28 PST 
Committed r269573: <https://trac.webkit.org/changeset/269573>

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