Bug 59922

Summary: [EFL] Cleanup EFL initialization/shutdown in the EWebLauncher
Product: WebKit Reporter: Tomasz Morawski <t.morawski>
Component: WebKit EFLAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Normal CC: commit-queue, gyuyoung.kim, kenneth, leandro, lucas.de.marchi, rakuco, tonikitoo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch none

Description Tomasz Morawski 2011-05-02 02:46:15 PDT
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.
Comment 1 Tomasz Morawski 2011-05-02 02:50:13 PDT
Created attachment 91901 [details]
Patch
Comment 2 Gyuyoung Kim 2011-05-02 03:24:02 PDT
Comment on attachment 91901 [details]
Patch

LGTM.
Comment 3 WebKit Commit Bot 2011-05-02 04:02:29 PDT
The commit-queue encountered the following flaky tests while processing attachment 91901 [details]:

http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch.html bug 52016 (author: jchaffraix@webkit.org)
The commit-queue is continuing to process your patch.
Comment 4 WebKit Commit Bot 2011-05-02 04:03:37 PDT
Comment on attachment 91901 [details]
Patch

Clearing flags on attachment: 91901

Committed r85471: <http://trac.webkit.org/changeset/85471>
Comment 5 WebKit Commit Bot 2011-05-02 04:03:42 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Lucas De Marchi 2011-05-02 06:37:29 PDT
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.
Comment 7 Tomasz Morawski 2011-05-02 07:33:14 PDT
(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
Comment 8 Lucas De Marchi 2011-05-02 08:40:38 PDT
(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.
Comment 9 Tomasz Morawski 2011-05-03 04:06:59 PDT
Thank you for your explanation. I know what I have done wrong. Could you revert my commit?
Comment 10 Lucas De Marchi 2011-05-03 11:16:07 PDT
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>
Comment 11 Raphael Kubo da Costa (:rakuco) 2011-05-11 09:03:25 PDT
Hey there, wouldn't it make sense to close this bug now?
Comment 12 Lucas De Marchi 2011-05-11 10:42:46 PDT
(In reply to comment #11)
> Hey there, wouldn't it make sense to close this bug now?

Yes, closing it now.