173968 2017-06-29 06:34:48 -0700 REGRESSION(r218896): ASSERT in WebPageProxy::dataCallback 2017-06-29 12:19:22 -0700 1 1 1 Unclassified WebKit Page Loading WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED Gtk, Regression P2 Normal --- 1 cgarcia webkit-unassigned achristensen beidson bugs-noreply buildbot cdumez commit-queue dbates japhet mcatanzaro rniwa oldest_to_newest 1323927 0 cgarcia 2017-06-29 06:34:48 -0700 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] 1323928 1 314131 cgarcia 2017-06-29 06:39:46 -0700 Created attachment 314131 Patch 1323936 2 314131 mcatanzaro 2017-06-29 07:45:22 -0700 Comment on attachment 314131 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. 1323945 3 cgarcia 2017-06-29 08:21:10 -0700 (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. 1323980 4 314131 beidson 2017-06-29 10:30:57 -0700 Comment on attachment 314131 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. 1324037 5 314131 commit-queue 2017-06-29 12:19:20 -0700 Comment on attachment 314131 Patch Clearing flags on attachment: 314131 Committed r218954: <http://trac.webkit.org/changeset/218954> 1324038 6 commit-queue 2017-06-29 12:19:22 -0700 All reviewed patches have been landed. Closing bug. 314131 2017-06-29 06:39:46 -0700 2017-06-29 12:19:20 -0700 Patch wcore-load-icon-crash.diff text/plain 3438 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA3ZWI0NWNlMDY3Zi4uYzM0ODViMDdlYTggMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyMyBAQAorMjAxNy0wNi0yOSAgQ2FybG9zIEdhcmNpYSBDYW1wb3MgIDxjZ2FyY2lhQGln YWxpYS5jb20+CisKKyAgICAgICAgUkVHUkVTU0lPTihyMjE4ODk2KTogQVNTRVJUIGluIFdlYlBh Z2VQcm94eTo6ZGF0YUNhbGxiYWNrCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0xNzM5NjgKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBUaGUgcHJvYmxlbSBpcyB0aGF0IFdlYlBhZ2VQcm94eTo6Z2V0TG9hZERl Y2lzaW9uRm9ySWNvbigpIHNlbmRzIDAgYXMgY2FsbGJhY2sgSUQgd2hlbiB0aGUgZGVjaXNpb24g aXMgdG8gbm90CisgICAgICAgIGxvYWQgdGhlIGljb24uIFNpbmNlIHIyMTg4OTYgd2UgYWx3YXlz IG5vdGlmeSB0aGUgY2xpZW50IGV2ZW4gd2hlbiB0aGUgZGVjaXNpb24gaXMgdG8gbm90IGxvYWQg dGhlIGljb24sIGluCisgICAgICAgIHdoaWNoIGNhc2UgdGhlIFVJIGRvZXNuJ3QgcmVhbGx5IGV4 cGVjdCBhIGNhbGxiYWNrLiBXaGVuIFdlYlBhZ2VQcm94eTo6ZGF0YUNhbGxiYWNrIGlzIGNhbGxl ZCB3aXRoIGEgMCBjYWxsYmFjayBJRCwKKyAgICAgICAgQ2FsbGJhY2tNYXA6OnRha2UoKSBjcmFz aGVzIGluIFJFTEVBU0VfQVNTRVJUKGNhbGxiYWNrSUQpLgorCisgICAgICAgIEZpeGVzIHNldmVy YWwgR1RLKyB1bml0IHRlc3RzIHRoYXQgYXJlIGNyYXNoaW5nLgorCisgICAgICAgICogbG9hZGVy L0RvY3VtZW50TG9hZGVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkRvY3VtZW50TG9hZGVyOjpk aWRHZXRMb2FkRGVjaXNpb25Gb3JJY29uKTogUmV0dXJuIGVhcmxpZXIgaWYgZGVjaXNpb24gaXMg ZmFsc2Ugb3IgZnJhbWUgaXMgbnVsbHB0ci4KKyAgICAgICAgKFdlYkNvcmU6OkRvY3VtZW50TG9h ZGVyOjpmaW5pc2hlZExvYWRpbmdJY29uKTogTW92ZSBSRUxFQVNFX0FTU0VSVCB0byBub3RpZnlG aW5pc2hlZExvYWRpbmdJY29uKCkuCisgICAgICAgIChXZWJDb3JlOjpEb2N1bWVudExvYWRlcjo6 bm90aWZ5RmluaXNoZWRMb2FkaW5nSWNvbik6IEFzc2VydCBpZiBjYWxsYmFja0lkZW50aWZpZXIg aXMgMCBvciBtX2ZyYW1lIGlzIG51bGxwdHIsCisgICAgICAgIHNpbmNlIGl0J3Mgbm8gbG9uZ2Vy IGV4cGVjdGVkIHRvIGhhcHBlbi4KKwogMjAxNy0wNi0yOCAgRnJlZGVyaWMgV2FuZyAgPGZ3YW5n QGlnYWxpYS5jb20+CiAKICAgICAgICAgQWxpZ24gRG9jdW1lbnQ6OmNhbk5hdmlnYXRlIG9uIHRo ZSBIVE01IHNwZWNpZmljYXRpb24KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2xvYWRlci9E b2N1bWVudExvYWRlci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvRG9jdW1lbnRMb2FkZXIu Y3BwCmluZGV4IDk4OWY5OWQ4N2QxLi5jOGZjYTYwMjhhYyAxMDA2NDQKLS0tIGEvU291cmNlL1dl YkNvcmUvbG9hZGVyL0RvY3VtZW50TG9hZGVyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9sb2Fk ZXIvRG9jdW1lbnRMb2FkZXIuY3BwCkBAIC0xNjgzLDYgKzE2ODMsMTAgQEAgdm9pZCBEb2N1bWVu dExvYWRlcjo6ZGlkR2V0TG9hZERlY2lzaW9uRm9ySWNvbihib29sIGRlY2lzaW9uLCB1aW50NjRf dCBsb2FkSWRlbnQKIHsKICAgICBhdXRvIGljb24gPSBtX2ljb25zUGVuZGluZ0xvYWREZWNpc2lv bi50YWtlKGxvYWRJZGVudGlmaWVyKTsKIAorICAgIC8vIElmIHRoZSBkZWNpc2lvbiB3YXMgbm90 IHRvIGxvYWQgb3IgdGhpcyBEb2N1bWVudExvYWRlciBpcyBhbHJlYWR5IGRldGFjaGVkLCB0aGVy ZSBpcyBubyBsb2FkIHRvIHBlcmZvcm0uCisgICAgaWYgKCFkZWNpc2lvbiB8fCAhbV9mcmFtZSkK KyAgICAgICAgcmV0dXJuOworCiAgICAgLy8gSWYgdGhlIExpbmtJY29uIHdlIGp1c3QgdG9vayBp cyBlbXB0eSwgdGhlbiB0aGUgRG9jdW1lbnRMb2FkZXIgaGFkIGFsbCBvZiBpdHMgbG9hZGVycyBz dG9wcGVkCiAgICAgLy8gd2hpbGUgdGhpcyBpY29uIGxvYWQgZGVjaXNpb24gd2FzIHBlbmRpbmcu CiAgICAgLy8gSW4gdGhpcyBjYXNlIHdlIG5lZWQgdG8gbm90aWZ5IHRoZSBjbGllbnQgdGhhdCB0 aGUgaWNvbiBmaW5pc2hlZCBsb2FkaW5nIHdpdGggZW1wdHkgZGF0YS4KQEAgLTE2OTEsMTAgKzE2 OTUsNiBAQCB2b2lkIERvY3VtZW50TG9hZGVyOjpkaWRHZXRMb2FkRGVjaXNpb25Gb3JJY29uKGJv b2wgZGVjaXNpb24sIHVpbnQ2NF90IGxvYWRJZGVudAogICAgICAgICByZXR1cm47CiAgICAgfQog Ci0gICAgLy8gSWYgdGhlIGRlY2lzaW9uIHdhcyBub3QgdG8gbG9hZCBvciB0aGlzIERvY3VtZW50 TG9hZGVyIGlzIGFscmVhZHkgZGV0YWNoZWQsIHRoZXJlIGlzIG5vIGxvYWQgdG8gcGVyZm9ybS4K LSAgICBpZiAoIWRlY2lzaW9uIHx8ICFtX2ZyYW1lKQotICAgICAgICByZXR1cm47Ci0KICAgICBh dXRvIGljb25Mb2FkZXIgPSBzdGQ6Om1ha2VfdW5pcXVlPEljb25Mb2FkZXI+KCp0aGlzLCBpY29u LnVybCk7CiAgICAgYXV0byogcmF3SWNvbkxvYWRlciA9IGljb25Mb2FkZXIuZ2V0KCk7CiAgICAg bV9pY29uTG9hZGVycy5zZXQoV1RGTW92ZShpY29uTG9hZGVyKSwgbmV3Q2FsbGJhY2tJRCk7CkBA IC0xNzA4LDE1ICsxNzA4LDE0IEBAIHZvaWQgRG9jdW1lbnRMb2FkZXI6OmZpbmlzaGVkTG9hZGlu Z0ljb24oSWNvbkxvYWRlciYgbG9hZGVyLCBTaGFyZWRCdWZmZXIqIGJ1ZmZlCiAgICAgQVNTRVJU KG1fZnJhbWUpOwogCiAgICAgYXV0byBjYWxsYmFja0lkZW50aWZpZXIgPSBtX2ljb25Mb2FkZXJz LnRha2UoJmxvYWRlcik7Ci0gICAgUkVMRUFTRV9BU1NFUlQoY2FsbGJhY2tJZGVudGlmaWVyKTsK LQogICAgIG5vdGlmeUZpbmlzaGVkTG9hZGluZ0ljb24oY2FsbGJhY2tJZGVudGlmaWVyLCBidWZm ZXIpOwogfQogCiB2b2lkIERvY3VtZW50TG9hZGVyOjpub3RpZnlGaW5pc2hlZExvYWRpbmdJY29u KHVpbnQ2NF90IGNhbGxiYWNrSWRlbnRpZmllciwgU2hhcmVkQnVmZmVyKiBidWZmZXIpCiB7Ci0g ICAgaWYgKG1fZnJhbWUpCi0gICAgICAgIG1fZnJhbWUtPmxvYWRlcigpLmNsaWVudCgpLmZpbmlz aGVkTG9hZGluZ0ljb24oY2FsbGJhY2tJZGVudGlmaWVyLCBidWZmZXIpOworICAgIFJFTEVBU0Vf QVNTRVJUKGNhbGxiYWNrSWRlbnRpZmllcik7CisgICAgUkVMRUFTRV9BU1NFUlQobV9mcmFtZSk7 CisgICAgbV9mcmFtZS0+bG9hZGVyKCkuY2xpZW50KCkuZmluaXNoZWRMb2FkaW5nSWNvbihjYWxs YmFja0lkZW50aWZpZXIsIGJ1ZmZlcik7CiB9CiAKIHZvaWQgRG9jdW1lbnRMb2FkZXI6OmRpc3Bh dGNoT25sb2FkRXZlbnRzKCkK