59922 2011-05-02 02:46:15 -0700 [EFL] Cleanup EFL initialization/shutdown in the EWebLauncher 2011-05-11 10:42:46 -0700 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) PC Linux RESOLVED INVALID P2 Normal --- 0 t.morawski webkit-unassigned commit-queue gyuyoung.kim kenneth leandro lucas.de.marchi rakuco tonikitoo oldest_to_newest 396304 0 t.morawski 2011-05-02 02:46:15 -0700 Removed ecore_evas_init and edje_init form main function due to they are also called in the ewk_init. Moved ewk_shutdown to quit function. 396305 1 91901 t.morawski 2011-05-02 02:50:13 -0700 Created attachment 91901 Patch 396311 2 91901 gyuyoung.kim 2011-05-02 03:24:02 -0700 Comment on attachment 91901 Patch LGTM. 396327 3 commit-queue 2011-05-02 04:02:29 -0700 The commit-queue encountered the following flaky tests while processing attachment 91901: http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch.html bug 52016 (author: jchaffraix@webkit.org) The commit-queue is continuing to process your patch. 396328 4 91901 commit-queue 2011-05-02 04:03:37 -0700 Comment on attachment 91901 Patch Clearing flags on attachment: 91901 Committed r85471: <http://trac.webkit.org/changeset/85471> 396329 5 commit-queue 2011-05-02 04:03:42 -0700 All reviewed patches have been landed. Closing bug. 396358 6 lucas.de.marchi 2011-05-02 06:37:29 -0700 The rule of thumb in EFL is that you init in all the places you use it. So, if in your application you call ecore_evas_* functions, you have to call ecore_evas_init() first. All the EFL init functions will just increment a ref if they are already initialized. This is the easiest way to keep things safe and is the way adopted in all the rest of EFL. So, ewk calls ecore_evas_init() and edje_init() because it uses functions of these libraries. In your application, you shouldn't trust what the library will do with external libraries, so you should call the _init() function too. As I said, this will be almost a nop and you'll be safe regarding future updates. 396382 7 t.morawski 2011-05-02 07:33:14 -0700 (In reply to comment #6) > The rule of thumb in EFL is that you init in all the places you use it. So, if in your application you call ecore_evas_* functions, you have to call ecore_evas_init() first. > > All the EFL init functions will just increment a ref if they are already initialized. This is the easiest way to keep things safe and is the way adopted in all the rest of EFL. > > > So, ewk calls ecore_evas_init() and edje_init() because it uses functions of these libraries. In your application, you shouldn't trust what the library will do with external libraries, so you should call the _init() function too. As I said, this will be almost a nop and you'll be safe regarding future updates. Thank you for your comment. Generally I agre with you. This is a good way. But in this specific case where we use our own _WebKit engine library_ this redundant initialization makes only code confusion. We exactly know what initialization function will do and it is hard to belive that the EFL initialization code will disappears from there. :) Moreover, when I use an engine I expect that a lot of think like initializations will be done on the engine site without my control. On the other hand why there are functions like ewk_init and ewk_shutdown in the public api and the browser programmer have to call another initalization for objects that are belogns to him only for increase ref counter. Should we also move glib initialization to EWebLauncher to be consistent? :) Best Regards Tomasz Morawski 396393 8 lucas.de.marchi 2011-05-02 08:40:38 -0700 (In reply to comment #7) > Thank you for your comment. Generally I agre with you. This is a good way. > But in this specific case where we use our own _WebKit engine library_ this What do you mean by "our own webkit engine library"? Besides being a browser for testing the EFL port, EWebLauncher serves as a demo how a browser would be done with this port. > redundant initialization makes only code confusion. We exactly know what Why? This is a common agreement among EFL developers: you init what you use. If your application uses evas, you init evas; if it uses ecore_evas, you init ecore_evas... even if evas is actually initialized by ecore_evas_init(). > initialization function will do and it is hard to belive that the EFL > initialization code will disappears from there. :) Moreover, when I use an > engine I expect that a lot of think like initializations will be done on the > engine site without my control. Exactly because it's without your control, you shouldn't trust it. > > Should we also move glib initialization to EWebLauncher to be consistent? :) If you use a function in glib's namespace, I'd say yes. 397033 9 t.morawski 2011-05-03 04:06:59 -0700 Thank you for your explanation. I know what I have done wrong. Could you revert my commit? 397205 10 lucas.de.marchi 2011-05-03 11:16:07 -0700 Reverted r85471 for reason: The rule of thumb in EFL is that you init in all the places you use it. So, if in your application you call ecore_evas_* functions, you have to call ecore_evas_init() first. Committed r85625: <http://trac.webkit.org/changeset/85625> 402006 11 rakuco 2011-05-11 09:03:25 -0700 Hey there, wouldn't it make sense to close this bug now? 402100 12 lucas.de.marchi 2011-05-11 10:42:46 -0700 (In reply to comment #11) > Hey there, wouldn't it make sense to close this bug now? Yes, closing it now. 91901 2011-05-02 02:50:13 -0700 2011-05-02 04:03:37 -0700 Patch EFLInitialization.patch text/plain 1896 t.morawski ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCBkNTQw YWFkLi42MzQyOGExIDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTEtMDUtMDIgIFRvbWFzeiBNb3Jhd3NraSAgPHQu bW9yYXdza2lAc2Ftc3VuZy5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgQ2xlYW51cCBFRkwgaW5pdGlhbGl6YXRpb24vc2h1dGRvd24gaW4gdGhl IEVXZWJMYXVuY2hlcgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9NTk5MjIKKworICAgICAgICBSZW1vdmVkIGVjb3JlX2V2YXNfaW5pdCBhbmQgZWRqZV9p bml0IGZvcm0gbWFpbiBmdW5jdGlvbiBkdWUgdG8gdGhleQorICAgICAgICBhcmUgYWxzbyBjYWxs ZWQgaW4gdGhlIGV3a19pbml0LiBNb3ZlZCBld2tfc2h1dGRvd24gdG8gcXVpdCBmdW5jdGlvbi4K KworICAgICAgICAqIEVXZWJMYXVuY2hlci9tYWluLmM6CisgICAgICAgIChxdWl0KToKKyAgICAg ICAgKG1haW4pOgorCiAyMDExLTA1LTAyICBBZGFtIEJhcnRoICA8YWJhcnRoQHdlYmtpdC5vcmc+ CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRXJpYyBTZWlkZWwuCmRpZmYgLS1naXQgYS9Ub29scy9F V2ViTGF1bmNoZXIvbWFpbi5jIGIvVG9vbHMvRVdlYkxhdW5jaGVyL21haW4uYwppbmRleCAzYmIy N2EyLi5iMmQ3NTBlIDEwMDY0NAotLS0gYS9Ub29scy9FV2ViTGF1bmNoZXIvbWFpbi5jCisrKyBi L1Rvb2xzL0VXZWJMYXVuY2hlci9tYWluLmMKQEAgLTYzNCw4ICs2MzQsNyBAQCBvbl9jbG9zZVdp bmRvdyhFY29yZV9FdmFzICplZSkKIHN0YXRpYyBpbnQKIHF1aXQoRWluYV9Cb29sIHN1Y2Nlc3Ms IGNvbnN0IGNoYXIgKm1zZykKIHsKLSAgICBlZGplX3NodXRkb3duKCk7Ci0gICAgZWNvcmVfZXZh c19zaHV0ZG93bigpOworICAgIGV3a19zaHV0ZG93bigpOwogCiAgICAgaWYgKG1zZykKICAgICAg ICAgZnB1dHMobXNnLCAoc3VjY2VzcykgPyBzdGRvdXQgOiBzdGRlcnIpOwpAQCAtODQ2LDEzICs4 NDUsOCBAQCBtYWluKGludCBhcmdjLCBjaGFyICphcmd2W10pCiAgICAgICAgIEVDT1JFX0dFVE9Q VF9WQUxVRV9OT05FCiAgICAgfTsKIAotICAgIGlmICghZWNvcmVfZXZhc19pbml0KCkpCi0gICAg ICAgIHJldHVybiBFWElUX0ZBSUxVUkU7Ci0KLSAgICBpZiAoIWVkamVfaW5pdCgpKSB7Ci0gICAg ICAgIGVjb3JlX2V2YXNfc2h1dGRvd24oKTsKLSAgICAgICAgcmV0dXJuIEVYSVRfRkFJTFVSRTsK LSAgICB9CisgICAgaWYgKCFld2tfaW5pdCgpKQorICAgICAgICByZXR1cm4gcXVpdChFSU5BX0ZB TFNFLCAiRVJST1I6IGNvdWxkIG5vdCBpbml0aWFsaXplIGV3ay5cbiIpOwogCiAgICAgZWNvcmVf YXBwX2FyZ3Nfc2V0KGFyZ2MsIChjb25zdCBjaGFyKiopIGFyZ3YpOwogICAgIGFyZ3MgPSBlY29y ZV9nZXRvcHRfcGFyc2UoJm9wdGlvbnMsIHZhbHVlcywgYXJnYywgYXJndik7CkBAIC04NzIsNyAr ODY2LDYgQEAgbWFpbihpbnQgYXJnYywgY2hhciAqYXJndltdKQogICAgIGlmICghdGhlbWVQYXRo KQogICAgICAgICByZXR1cm4gcXVpdChFSU5BX0ZBTFNFLCAiRVJST1I6IGNvdWxkIG5vdCBmaW5k IHRoZW1lLlxuIik7CiAKLSAgICBld2tfaW5pdCgpOwogICAgIHRtcCA9IGdldGVudigiVE1QRElS Iik7CiAgICAgaWYgKCF0bXApCiAgICAgICAgIHRtcCA9ICIvdG1wIjsKQEAgLTg5MCw3ICs4ODMs NSBAQCBtYWluKGludCBhcmdjLCBjaGFyICphcmd2W10pCiAKICAgICBlY29yZV9tYWluX2xvb3Bf YmVnaW4oKTsKIAotICAgIGV3a19zaHV0ZG93bigpOwotCiAgICAgcmV0dXJuIHF1aXQoRUlOQV9U UlVFLCBOVUxMKTsKIH0K