Bug 134236 – [EFL] Replace RefPtr<Evas_Object> with UniquePtrEfl

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Ryuan Choi]

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.

[Ryuan Choi]

Created attachment 233677 [details]
Patch

[Jinwoo Song]

Looks good to me.

[Gyuyoung Kim]

Comment on attachment 233677 [details]
Patch

Isn't *EflUniquePtr* for OwnPtr/PassOwnPtr ? Can EflUniquePtr support RefPtr as well ?

[Ryuan Choi]

(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).

[Ryuan Choi]

(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).

[Gyuyoung Kim]

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 ?

[Ryuan Choi]

Committed r170347: <http://trac.webkit.org/changeset/170347>

[Ryuan Choi]

Comment on attachment 233677 [details]
Patch

clearing flags after manually landed.