Several GTK+ unit tests are crashing because of this. The problem is that WebPageProxy::getLoadDecisionForIcon() sends 0 as callback ID when the decision is to not load the icon. Since r218896 we always notify the client even when the decision is not not load the icon, in which case the UI doesn't really expect a callback. When WebPageProxy::dataCallback is called with a 0 callback, CallbackMap::take crashes in RELEASE_ASSERT(callbackID). 1 0x7f3a21cf2c67 /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18(WTFCrash+0x17) [0x7f3a21cf2c67] 2 0x7f3a2366b09d /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37(_ZN6WebKit12WebPageProxy12dataCallbackERKN3IPC13DataReferenceEm+0x15d) [0x7f3a2366b09d] 3 0x7f3a2397d003 /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37(_ZN6WebKit12WebPageProxy17didReceiveMessageERN3IPC10ConnectionERNS1_7DecoderE+0x1d63) [0x7f3a2397d003] 4 0x7f3a235b0fc9 /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC18MessageReceiverMap15dispatchMessageERNS_10ConnectionERNS_7DecoderE+0x299) [0x7f3a235b0fc9] 5 0x7f3a23692002 /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37(_ZN6WebKit15WebProcessProxy17didReceiveMessageERN3IPC10ConnectionERNS1_7DecoderE+0x12) [0x7f3a23692002] 6 0x7f3a235ace8b /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC10Connection15dispatchMessageESt10unique_ptrINS_7DecoderESt14default_deleteIS2_EE+0x6b) [0x7f3a235ace8b] 7 0x7f3a235addbc /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37(_ZN3IPC10Connection18dispatchOneMessageEv+0xec) [0x7f3a235addbc] 8 0x7f3a21d066dd /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18(_ZN3WTF7RunLoop11performWorkEv+0x18d) [0x7f3a21d066dd] 9 0x7f3a21d3c039 /home/slave/webkitgtk/gtk-linux-64-release/build/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18(+0xe23039) [0x7f3a21d3c039] 10 0x7f3a26d0e5ca /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0(g_main_context_dispatch+0x13a) [0x7f3a26d0e5ca] 11 0x7f3a26d0e948 /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0(+0x49948) [0x7f3a26d0e948] 12 0x7f3a26d0ec62 /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0(g_main_loop_run+0xc2) [0x7f3a26d0ec62] 13 0x7f3a276c4623 ./Tools/gtk/../../WebKitBuild/Release/bin/TestWebKitAPI/WebKit2Gtk/TestResources(+0x8e5623) [0x7f3a276c4623] 14 0x7f3a26d34039 /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0(+0x6f039) [0x7f3a26d34039] 15 0x7f3a26d34207 /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0(+0x6f207) [0x7f3a26d34207] 16 0x7f3a26d34207 /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0(+0x6f207) [0x7f3a26d34207] 17 0x7f3a26d343fe /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0(g_test_run_suite+0xbe) [0x7f3a26d343fe] 18 0x7f3a26d34421 /home/slave/webkitgtk/gtk-linux-64-release-tests/build/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0(g_test_run+0x11) [0x7f3a26d34421] 19 0x7f3a276c2514 ./Tools/gtk/../../WebKitBuild/Release/bin/TestWebKitAPI/WebKit2Gtk/TestResources(main+0x114) [0x7f3a276c2514] 20 0x7f3a1b3222b1 /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1) [0x7f3a1b3222b1] 21 0x7f3a276c2a8a ./Tools/gtk/../../WebKitBuild/Release/bin/TestWebKitAPI/WebKit2Gtk/TestResources(_start+0x2a) [0x7f3a276c2a8a]
Created attachment 314131 [details] Patch
Comment on attachment 314131 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=314131&action=review > Source/WebCore/loader/DocumentLoader.cpp:1717 > + RELEASE_ASSERT(callbackIdentifier); > + RELEASE_ASSERT(m_frame); IMO there's no particular reason for these to be release asserts rather than normal asserts.
(In reply to Michael Catanzaro from comment #2) > Comment on attachment 314131 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=314131&action=review > > > Source/WebCore/loader/DocumentLoader.cpp:1717 > > + RELEASE_ASSERT(callbackIdentifier); > > + RELEASE_ASSERT(m_frame); > > IMO there's no particular reason for these to be release asserts rather than > normal asserts. No that I haven't added it, but moved and it was already a release assert. Then I added the frame one, and used release assert too for consistency.
Comment on attachment 314131 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=314131&action=review >>> Source/WebCore/loader/DocumentLoader.cpp:1717 >>> + RELEASE_ASSERT(m_frame); >> >> IMO there's no particular reason for these to be release asserts rather than normal asserts. > > No that I haven't added it, but moved and it was already a release assert. Then I added the frame one, and used release assert too for consistency. Yup, this is fine.
Comment on attachment 314131 [details] Patch Clearing flags on attachment: 314131 Committed r218954: <http://trac.webkit.org/changeset/218954>
All reviewed patches have been landed. Closing bug.