104113 2012-12-05 04:14:52 -0800 [EFL][WK2] Context clients should unregister themselves when destroyed 2012-12-06 05:12:24 -0800 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 103692 1 cdumez cdumez gyuyoung.kim jinwoo7.song kenneth lucas.de.marchi rakuco webkit.review.bot oldest_to_newest 783637 0 cdumez 2012-12-05 04:14:52 -0800 We get crashes after a Ewk_Context is destroyed because the corresponding client classes get destroyed but they don't unregister their callbacks. As a consequence, the client callbacks sometimes get called after the client objects have been destroyed and it causes crashes. We should make sure all our client classes unregister their callbacks in their class destructor. 783645 1 177726 cdumez 2012-12-05 04:30:41 -0800 Created attachment 177726 Patch 783649 2 177726 kenneth 2012-12-05 04:33:17 -0800 Comment on attachment 177726 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177726&action=review > Source/WebKit2/UIProcess/efl/ContextHistoryClientEfl.cpp:137 > + m_context->initializeHistoryClient(0); hmm maybe we should rename to not be called initialize then, and use setHistoryClient 783666 3 cdumez 2012-12-05 05:00:24 -0800 (In reply to comment #2) > (From update of attachment 177726 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=177726&action=review > > > Source/WebKit2/UIProcess/efl/ContextHistoryClientEfl.cpp:137 > > + m_context->initializeHistoryClient(0); > > hmm maybe we should rename to not be called initialize then, and use setHistoryClient Well, the corresponding C API is WKContextSetHistoryClient() so I tend to agree with you. However, I believe this should be done in a separate patch and we will need to CC Andersca. For now, what matters is that we get the crashes fixed. 784858 4 cdumez 2012-12-06 04:57:44 -0800 (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 177726 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=177726&action=review > > > > > Source/WebKit2/UIProcess/efl/ContextHistoryClientEfl.cpp:137 > > > + m_context->initializeHistoryClient(0); > > > > hmm maybe we should rename to not be called initialize then, and use setHistoryClient > > Well, the corresponding C API is WKContextSetHistoryClient() so I tend to agree with you. > > However, I believe this should be done in a separate patch and we will need to CC Andersca. > > For now, what matters is that we get the crashes fixed. Kenneth, do you mind if we land this patch like it is for now and propose the renaming afterwards? Or do you want to do the renaming beforehand? 784862 5 177726 kenneth 2012-12-06 05:04:59 -0800 Comment on attachment 177726 Patch no 784865 6 177726 webkit.review.bot 2012-12-06 05:12:19 -0800 Comment on attachment 177726 Patch Clearing flags on attachment: 177726 Committed r136833: <http://trac.webkit.org/changeset/136833> 784866 7 webkit.review.bot 2012-12-06 05:12:24 -0800 All reviewed patches have been landed. Closing bug. 177726 2012-12-05 04:30:41 -0800 2012-12-06 05:12:19 -0800 Patch 104113_context_clients_unregister.patch text/plain 5293 cdumez ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCBiMWQxMGVjLi4wZjM1MzUzIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMzMg QEAKIDIwMTItMTItMDUgIENocmlzdG9waGUgRHVtZXogIDxjaHJpc3RvcGhlLmR1bWV6QGludGVs LmNvbT4KIAorICAgICAgICBbRUZMXVtXSzJdIENvbnRleHQgY2xpZW50cyBzaG91bGQgdW5yZWdp c3RlciB0aGVtc2VsdmVzIHdoZW4gZGVzdHJveWVkCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDQxMTMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICBNYWtlIHN1cmUgdGhlIGNvbnRleHQgY2xpZW50cyAoSGlz dG9yeSBhbmQgRG93bmxvYWQgY2xpZW50cykKKyAgICAgICAgdW5yZWdpc3RlciB0aGVtc2VsdmVz IHdoZW4gZGVzdHJveWVkIHRvIG1ha2Ugc3VyZSB0aGVpcgorICAgICAgICBjYWxsYmFjayBmdW5j dGlvbnMgYXJlIG5ldmVyIGNhbGxlZCBhZnRlciB0aGUgY2xpZW50IG9iamVjdHMKKyAgICAgICAg aGF2ZSBiZWVuIGRlc3Ryb3llZCAoaS5lLiB3aGVuIHRoZSBwYXJlbnQgRXdrX0NvbnRleHQgaGFz CisgICAgICAgIGJlZW4gZGVzdHJveWVkKS4KKworICAgICAgICBUaGlzIGFkZHJlc3NlcyBjcmFz aGluZyBpc3N1ZXMgYWZ0ZXIgYSBFd2tfQ29udGV4dCBvYmplY3QKKyAgICAgICAgZ2V0cyB1bnJl ZidkIGFuZCBkZXN0cm95ZWQuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvZWZsL0NvbnRleHRIaXN0 b3J5Q2xpZW50RWZsLmNwcDoKKyAgICAgICAgKFdlYktpdDo6Q29udGV4dEhpc3RvcnlDbGllbnRF Zmw6OkNvbnRleHRIaXN0b3J5Q2xpZW50RWZsKToKKyAgICAgICAgKFdlYktpdCk6CisgICAgICAg IChXZWJLaXQ6OkNvbnRleHRIaXN0b3J5Q2xpZW50RWZsOjp+Q29udGV4dEhpc3RvcnlDbGllbnRF ZmwpOgorICAgICAgICAqIFVJUHJvY2Vzcy9lZmwvQ29udGV4dEhpc3RvcnlDbGllbnRFZmwuaDoK KyAgICAgICAgKENvbnRleHRIaXN0b3J5Q2xpZW50RWZsKToKKyAgICAgICAgKiBVSVByb2Nlc3Mv ZWZsL0Rvd25sb2FkTWFuYWdlckVmbC5jcHA6CisgICAgICAgIChXZWJLaXQ6OkRvd25sb2FkTWFu YWdlckVmbDo6fkRvd25sb2FkTWFuYWdlckVmbCk6CisgICAgICAgIChXZWJLaXQpOgorICAgICAg ICAqIFVJUHJvY2Vzcy9lZmwvRG93bmxvYWRNYW5hZ2VyRWZsLmg6CisgICAgICAgIChEb3dubG9h ZE1hbmFnZXJFZmwpOgorCisyMDEyLTEyLTA1ICBDaHJpc3RvcGhlIER1bWV6ICA8Y2hyaXN0b3Bo ZS5kdW1lekBpbnRlbC5jb20+CisKICAgICAgICAgW0VGTF1bV0syXSBFV0syVW5pdFRlc3RCYXNl LmV3a19mYXZpY29uX2RhdGFiYXNlX2FzeW5jX2ljb25fZ2V0IGlzIGNyYXNoaW5nIHdpdGggbmV3 IEV3a19Db250ZXh0CiAgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0xMDQxMTAKIApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL2VmbC9D b250ZXh0SGlzdG9yeUNsaWVudEVmbC5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvZWZs L0NvbnRleHRIaXN0b3J5Q2xpZW50RWZsLmNwcAppbmRleCA1ZjIwYzE3Li4wZTgwMjY4IDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvZWZsL0NvbnRleHRIaXN0b3J5Q2xpZW50 RWZsLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvZWZsL0NvbnRleHRIaXN0b3J5 Q2xpZW50RWZsLmNwcApAQCAtMTA3LDE0ICsxMDcsMTUgQEAgdm9pZCBDb250ZXh0SGlzdG9yeUNs aWVudEVmbDo6cG9wdWxhdGVWaXNpdGVkTGlua3MoV0tDb250ZXh0UmVmLCBjb25zdCB2b2lkKiBj bGkKIH0KIAogQ29udGV4dEhpc3RvcnlDbGllbnRFZmw6OkNvbnRleHRIaXN0b3J5Q2xpZW50RWZs KFBhc3NSZWZQdHI8V2ViQ29udGV4dD4gY29udGV4dCkKLSAgICA6IG1fdXNlckRhdGEoMCkKKyAg ICA6IG1fY29udGV4dChjb250ZXh0KQorICAgICwgbV91c2VyRGF0YSgwKQogICAgICwgbV9uYXZp Z2F0ZSgwKQogICAgICwgbV9jbGllbnRSZWRpcmVjdCgwKQogICAgICwgbV9zZXJ2ZXJSZWRpcmVj dCgwKQogICAgICwgbV90aXRsZVVwZGF0ZWQoMCkKICAgICAsIG1fcG9wdWxhdGVWaXNpdGVkTGlu a3MoMCkKIHsKLSAgICBBU1NFUlQoY29udGV4dCk7CisgICAgQVNTRVJUKG1fY29udGV4dCk7CiAK ICAgICBXS0NvbnRleHRIaXN0b3J5Q2xpZW50IHdrSGlzdG9yeUNsaWVudDsKICAgICBtZW1zZXQo JndrSGlzdG9yeUNsaWVudCwgMCwgc2l6ZW9mKFdLQ29udGV4dEhpc3RvcnlDbGllbnQpKTsKQEAg LTEyOCw3ICsxMjksMTIgQEAgQ29udGV4dEhpc3RvcnlDbGllbnRFZmw6OkNvbnRleHRIaXN0b3J5 Q2xpZW50RWZsKFBhc3NSZWZQdHI8V2ViQ29udGV4dD4gY29udGV4dCkKICAgICB3a0hpc3RvcnlD bGllbnQuZGlkVXBkYXRlSGlzdG9yeVRpdGxlID0gZGlkVXBkYXRlSGlzdG9yeVRpdGxlOwogICAg IHdrSGlzdG9yeUNsaWVudC5wb3B1bGF0ZVZpc2l0ZWRMaW5rcyA9IHBvcHVsYXRlVmlzaXRlZExp bmtzOwogCi0gICAgY29udGV4dC0+aW5pdGlhbGl6ZUhpc3RvcnlDbGllbnQoJndrSGlzdG9yeUNs aWVudCk7CisgICAgbV9jb250ZXh0LT5pbml0aWFsaXplSGlzdG9yeUNsaWVudCgmd2tIaXN0b3J5 Q2xpZW50KTsKK30KKworQ29udGV4dEhpc3RvcnlDbGllbnRFZmw6On5Db250ZXh0SGlzdG9yeUNs aWVudEVmbCgpCit7CisgICAgbV9jb250ZXh0LT5pbml0aWFsaXplSGlzdG9yeUNsaWVudCgwKTsK IH0KIAogdm9pZCBDb250ZXh0SGlzdG9yeUNsaWVudEVmbDo6c2V0Q2FsbGJhY2tzKEV3a19IaXN0 b3J5X05hdmlnYXRpb25fQ2IgbmF2aWdhdGUsIEV3a19IaXN0b3J5X0NsaWVudF9SZWRpcmVjdGlv bl9DYiBjbGllbnRSZWRpcmVjdCwgRXdrX0hpc3RvcnlfU2VydmVyX1JlZGlyZWN0aW9uX0NiIHNl cnZlclJlZGlyZWN0LCBFd2tfSGlzdG9yeV9UaXRsZV9VcGRhdGVfQ2IgdGl0bGVVcGRhdGUsIEV3 a19IaXN0b3J5X1BvcHVsYXRlX1Zpc2l0ZWRfTGlua3NfQ2IgcG9wdWxhdGVWaXNpdGVkTGlua3Ms IHZvaWQqIGRhdGEpCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvZWZsL0Nv bnRleHRIaXN0b3J5Q2xpZW50RWZsLmggYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvZWZsL0Nv bnRleHRIaXN0b3J5Q2xpZW50RWZsLmgKaW5kZXggYjY3MDI3MS4uYzdiM2EyMCAxMDA2NDQKLS0t IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL2VmbC9Db250ZXh0SGlzdG9yeUNsaWVudEVmbC5o CisrKyBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9lZmwvQ29udGV4dEhpc3RvcnlDbGllbnRF ZmwuaApAQCAtNDAsNiArNDAsOCBAQCBwdWJsaWM6CiAgICAgICAgIHJldHVybiBhZG9wdFB0cihu ZXcgQ29udGV4dEhpc3RvcnlDbGllbnRFZmwoY29udGV4dCkpOwogICAgIH0KIAorICAgIH5Db250 ZXh0SGlzdG9yeUNsaWVudEVmbCgpOworCiAgICAgdm9pZCBzZXRDYWxsYmFja3MoRXdrX0hpc3Rv cnlfTmF2aWdhdGlvbl9DYiwgRXdrX0hpc3RvcnlfQ2xpZW50X1JlZGlyZWN0aW9uX0NiLCBFd2tf SGlzdG9yeV9TZXJ2ZXJfUmVkaXJlY3Rpb25fQ2IsIEV3a19IaXN0b3J5X1RpdGxlX1VwZGF0ZV9D YiwgRXdrX0hpc3RvcnlfUG9wdWxhdGVfVmlzaXRlZF9MaW5rc19DYiwgdm9pZCopOwogCiBwcml2 YXRlOgpAQCAtNTEsNiArNTMsNyBAQCBwcml2YXRlOgogICAgIHN0YXRpYyB2b2lkIGRpZFVwZGF0 ZUhpc3RvcnlUaXRsZShXS0NvbnRleHRSZWYsIFdLUGFnZVJlZiwgV0tTdHJpbmdSZWYsIFdLVVJM UmVmLCBXS0ZyYW1lUmVmLCBjb25zdCB2b2lkKik7CiAgICAgc3RhdGljIHZvaWQgcG9wdWxhdGVW aXNpdGVkTGlua3MoV0tDb250ZXh0UmVmLCBjb25zdCB2b2lkKik7CiAKKyAgICBSZWZQdHI8V2Vi Q29udGV4dD4gbV9jb250ZXh0OwogICAgIHZvaWQqIG1fdXNlckRhdGE7CiAgICAgRXdrX0hpc3Rv cnlfTmF2aWdhdGlvbl9DYiBtX25hdmlnYXRlOwogICAgIEV3a19IaXN0b3J5X0NsaWVudF9SZWRp cmVjdGlvbl9DYiBtX2NsaWVudFJlZGlyZWN0OwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIv VUlQcm9jZXNzL2VmbC9Eb3dubG9hZE1hbmFnZXJFZmwuY3BwIGIvU291cmNlL1dlYktpdDIvVUlQ cm9jZXNzL2VmbC9Eb3dubG9hZE1hbmFnZXJFZmwuY3BwCmluZGV4IGRmZjY2YTYuLjkzYTdkYjAg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9lZmwvRG93bmxvYWRNYW5hZ2Vy RWZsLmNwcAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvZWZsL0Rvd25sb2FkTWFuYWdl ckVmbC5jcHAKQEAgLTEzOSw2ICsxMzksMTEgQEAgRG93bmxvYWRNYW5hZ2VyRWZsOjpEb3dubG9h ZE1hbmFnZXJFZmwoRXdrQ29udGV4dCogY29udGV4dCkKICAgICBXS0NvbnRleHRTZXREb3dubG9h ZENsaWVudCh0b0FQSShjb250ZXh0LT53ZWJDb250ZXh0KCkuZ2V0KCkpLCAmd2tEb3dubG9hZENs aWVudCk7CiB9CiAKK0Rvd25sb2FkTWFuYWdlckVmbDo6fkRvd25sb2FkTWFuYWdlckVmbCgpCit7 CisgICAgV0tDb250ZXh0U2V0RG93bmxvYWRDbGllbnQodG9BUEkobV9jb250ZXh0LT53ZWJDb250 ZXh0KCkuZ2V0KCkpLCAwKTsKK30KKwogdm9pZCBEb3dubG9hZE1hbmFnZXJFZmw6OnJlZ2lzdGVy RG93bmxvYWQoRG93bmxvYWRQcm94eSogZG93bmxvYWQsIEV3a1ZpZXdJbXBsKiB2aWV3SW1wbCkK IHsKICAgICB1aW50NjRfdCBkb3dubG9hZElkID0gZG93bmxvYWQtPmRvd25sb2FkSUQoKTsKZGlm ZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9lZmwvRG93bmxvYWRNYW5hZ2VyRWZs LmggYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvZWZsL0Rvd25sb2FkTWFuYWdlckVmbC5oCmlu ZGV4IDc3NTE2NDIuLmQzNzI4YzMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vz cy9lZmwvRG93bmxvYWRNYW5hZ2VyRWZsLmgKKysrIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNz L2VmbC9Eb3dubG9hZE1hbmFnZXJFZmwuaApAQCAtNDMsNiArNDMsOCBAQCBwdWJsaWM6CiAgICAg ICAgIHJldHVybiBhZG9wdFB0cihuZXcgRG93bmxvYWRNYW5hZ2VyRWZsKGNvbnRleHQpKTsKICAg ICB9CiAKKyAgICB+RG93bmxvYWRNYW5hZ2VyRWZsKCk7CisKICAgICB2b2lkIHJlZ2lzdGVyRG93 bmxvYWQoRG93bmxvYWRQcm94eSosIEV3a1ZpZXdJbXBsKik7CiAKIHByaXZhdGU6Cg==