19813 2008-06-29 07:31:11 -0700 [GTK] WebKit crashes on invalid settings notify callback 2008-07-17 04:53:58 -0700 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) PC Linux RESOLVED FIXED Gtk P2 Major --- 1 christian christian alp jmalonzo oldest_to_newest 84708 0 christian 2008-06-29 07:31:11 -0700 A bug reported for Midori [1] is in fact also a bug in WebKit. The WebView connects a notify callback for its WebSettings. But this callback isn't disconnected when the WebView is finalized which means that the callback is still emitted but the, by then destroyed, web view is invalid. [1] http://software.twotoasts.de/bugs/index.php?do=details&task_id=15&project=0 84709 1 21996 christian 2008-06-29 07:37:23 -0700 Created attachment 21996 Disconnect notify callback on finalize 84821 2 alp 2008-06-30 23:12:28 -0700 (In reply to comment #1) > Created an attachment (id=21996) [edit] > Disconnect notify callback on finalize > Would it be cleaner to re-use webkit_web_view_set_settings() for this purpose? If we change webkit_web_view_set_settings() to accept a NULL WebKitWebSettings*, it already has the code to disconnect the signal, unref etc. This would allow developers using the API to disassociate the WebView from WebSettings too using the same code path. 85131 3 christian 2008-07-03 15:48:07 -0700 (In reply to comment #2) > (In reply to comment #1) > > Created an attachment (id=21996) [edit] > > Disconnect notify callback on finalize > > > > Would it be cleaner to re-use webkit_web_view_set_settings() for this purpose? > > If we change webkit_web_view_set_settings() to accept a NULL > WebKitWebSettings*, it already has the code to disconnect the signal, unref > etc. This would allow developers using the API to disassociate the WebView from > WebSettings too using the same code path. I thought about this for a bit. I tend to think that's not really useful. Basically the view will always need some kind of WebSettings instance. That means if you could unset it via _set_settings (view, NULL), the view would have two options to cope with this: a) Unref the old settings and disconnect notify. Then essentially enter an undefined state. b) Unref the old settings, disconnect notify and immeditately create a new settings instance with default values. The state is 'back to defaults' I don't find either option too attractive. This is my estimation. If you, or anyone for that matter, have a use case I will probably not mind to add something in this direction. 86073 4 21996 zecke 2008-07-16 11:02:14 -0700 Comment on attachment 21996 Disconnect notify callback on finalize Looks sensible. I only wonder about the (gpointer) G_CALLBACK() mix in regard to handling this callback in the various methods but I have not checked if I we get compile warnings when always using G_CALLBACK... 86201 5 21996 christian 2008-07-17 04:53:37 -0700 Comment on attachment 21996 Disconnect notify callback on finalize (In reply to comment #4) > (From update of attachment 21996 [edit]) > Looks sensible. > > I only wonder about the (gpointer) G_CALLBACK() mix in regard to handling this > callback in the various methods but I have not checked if I we get compile > warnings when always using G_CALLBACK... We can't use G_CALLBACK() when disconnecting the handler, since that unfortunately breaks with the compiler saying that it can't cast »void (*)()« to »void*«. Committed in revision 35219. 21996 2008-06-29 07:37:23 -0700 2008-07-17 04:53:37 -0700 Disconnect notify callback on finalize webviewnotify.diff text/plain 1064 christian ZGlmZiAtLWdpdCBhL1dlYktpdC9ndGsvd2Via2l0L3dlYmtpdHdlYnZpZXcuY3BwIGIvV2ViS2l0 L2d0ay93ZWJraXQvd2Via2l0d2Vidmlldy5jcHAKaW5kZXggNmE3ZWExZi4uYjllYzYxNyAxMDA2 NDQKLS0tIGEvV2ViS2l0L2d0ay93ZWJraXQvd2Via2l0d2Vidmlldy5jcHAKKysrIGIvV2ViS2l0 L2d0ay93ZWJraXQvd2Via2l0d2Vidmlldy5jcHAKQEAgLTEwOCw2ICsxMDgsOCBAQCBzdGF0aWMg Z3VpbnQgd2Via2l0X3dlYl92aWV3X3NpZ25hbHNbTEFTVF9TSUdOQUxdID0geyAwLCB9OwogCiBH X0RFRklORV9UWVBFKFdlYktpdFdlYlZpZXcsIHdlYmtpdF93ZWJfdmlldywgR1RLX1RZUEVfQ09O VEFJTkVSKQogCitzdGF0aWMgdm9pZCB3ZWJraXRfd2ViX3ZpZXdfc2V0dGluZ3Nfbm90aWZ5KFdl YktpdFdlYlNldHRpbmdzKiB3ZWJTZXR0aW5ncywgR1BhcmFtU3BlYyogcHNwZWMsIFdlYktpdFdl YlZpZXcqIHdlYlZpZXcpOworCiBzdGF0aWMgdm9pZCB3ZWJraXRfd2ViX3ZpZXdfY29udGV4dF9t ZW51X3Bvc2l0aW9uX2Z1bmMoR3RrTWVudSosIGdpbnQqIHgsIGdpbnQqIHksIGdib29sZWFuKiBw dXNoSW4sIFdlYktpdFdlYlZpZXdQcml2YXRlKiBkYXRhKQogewogICAgICpwdXNoSW4gPSBGQUxT RTsKQEAgLTcyMCw2ICs3MjIsNyBAQCBzdGF0aWMgdm9pZCB3ZWJraXRfd2ViX3ZpZXdfZmluYWxp emUoR09iamVjdCogb2JqZWN0KQogICAgIGlmIChwcml2LT52ZXJ0aWNhbEFkanVzdG1lbnQpCiAg ICAgICAgIGdfb2JqZWN0X3VucmVmKHByaXYtPnZlcnRpY2FsQWRqdXN0bWVudCk7CiAgICAgZ19v YmplY3RfdW5yZWYocHJpdi0+YmFja0ZvcndhcmRMaXN0KTsKKyAgICBnX3NpZ25hbF9oYW5kbGVy c19kaXNjb25uZWN0X2J5X2Z1bmMocHJpdi0+d2ViU2V0dGluZ3MsIChncG9pbnRlcil3ZWJraXRf d2ViX3ZpZXdfc2V0dGluZ3Nfbm90aWZ5LCB3ZWJWaWV3KTsKICAgICBnX29iamVjdF91bnJlZihw cml2LT53ZWJTZXR0aW5ncyk7CiAgICAgZ19vYmplY3RfdW5yZWYocHJpdi0+bWFpbkZyYW1lKTsK ICAgICBnX29iamVjdF91bnJlZihwcml2LT5pbUNvbnRleHQpOwo=