Bug 134236

Summary: [EFL] Replace RefPtr<Evas_Object> with UniquePtrEfl
Product: WebKit Reporter: Ryuan Choi <ryuan.choi>
Component: WebKit EFLAssignee: Ryuan Choi <ryuan.choi>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, bunhere, cdumez, cmarcelo, commit-queue, gyuyoung.kim, jinwoo7.song, lucas.de.marchi, rakuco, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Ryuan Choi 2014-06-23 19:43:12 PDT
There are two custom smart pointers for Evas_Object, RefPtr<Evas_Object> and UniquePtr<Evas_Object>.

Although evas_object had _ref()/_unref(), these APIs are bit odd.
Indeed, we don't use ref-counting for Evas Object internally.

So, I suggest to replace RefPtr<Evas_Object> into UniquePtrEfl for internal Evas_Object.
Comment 1 Ryuan Choi 2014-06-23 21:15:21 PDT
Created attachment 233677 [details]
Patch
Comment 2 Jinwoo Song 2014-06-23 21:34:07 PDT
Looks good to me.
Comment 3 Gyuyoung Kim 2014-06-23 22:02:50 PDT
Comment on attachment 233677 [details]
Patch

Isn't *EflUniquePtr* for OwnPtr/PassOwnPtr ? Can EflUniquePtr support RefPtr as well ?
Comment 4 Ryuan Choi 2014-06-23 22:46:03 PDT
(In reply to comment #3)
> (From update of attachment 233677 [details])
> Isn't *EflUniquePtr* for OwnPtr/PassOwnPtr ? Can EflUniquePtr support RefPtr as well ?

No. this patch is not to drop RefPtr style(Reference counting) of Evas_Object.

Unfortunately, We are using both EflUniquePtr<Evas_Object> and RefPtr<Evas_Object>.
So, we should drop one of them and my suggestion is to drop RefPtr<Evas_Object>.
Although Evas_Object has ref/unref logic, it is little bit different from common RefPtr logic.

Evas_Object created with ref count of 0, unref does not destory it.
And evas_object_del also has some problems with _ref.
Since we increase ref count using _ref, evas_object_del just mark the deletion and it will be removed when ref count became 0 (by _unref).

In order to solve this issue, RefPtr<Evas_Object> had some extra conditions.
But, we don't use reference counting of Evas_Object widely (Almost use case is just for local variable.)

Instead, I think that it looks clear that Evas_Object will be owned by one instance like the unique_ptr (or OwnPtr).
Comment 5 Ryuan Choi 2014-06-23 22:46:49 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 233677 [details] [details])
> > Isn't *EflUniquePtr* for OwnPtr/PassOwnPtr ? Can EflUniquePtr support RefPtr as well ?
> 
> No. this patch is not to drop RefPtr style(Reference counting) of Evas_Object.

s/not to drop/to drop

> 
> Unfortunately, We are using both EflUniquePtr<Evas_Object> and RefPtr<Evas_Object>.
> So, we should drop one of them and my suggestion is to drop RefPtr<Evas_Object>.
> Although Evas_Object has ref/unref logic, it is little bit different from common RefPtr logic.
> 
> Evas_Object created with ref count of 0, unref does not destory it.
> And evas_object_del also has some problems with _ref.
> Since we increase ref count using _ref, evas_object_del just mark the deletion and it will be removed when ref count became 0 (by _unref).
> 
> In order to solve this issue, RefPtr<Evas_Object> had some extra conditions.
> But, we don't use reference counting of Evas_Object widely (Almost use case is just for local variable.)
> 
> Instead, I think that it looks clear that Evas_Object will be owned by one instance like the unique_ptr (or OwnPtr).
Comment 6 Gyuyoung Kim 2014-06-23 23:04:34 PDT
Comment on attachment 233677 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=233677&action=review

Now I understand what this patch want to do. LGTM.

> Source/WTF/ChangeLog:3
> +        [EFL] Replace RefPtr<Evas_Object> to UniquePtrEfl

to -> with ?
Comment 7 Ryuan Choi 2014-06-23 23:12:44 PDT
Committed r170347: <http://trac.webkit.org/changeset/170347>
Comment 8 Ryuan Choi 2014-06-23 23:13:07 PDT
Comment on attachment 233677 [details]
Patch

clearing flags after manually landed.