Bug 80119 – [EFL] Add OwnPtr specialization for Ecore_Timer

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

[YoungTaeck Song]

Add an overload for deleteOwnedPtr(Ecore_Timer*) on EFL port.

[YoungTaeck Song]

Created attachment 129833 [details]
patch

[Ryuan Choi]

How do you think about applying this in SharedTimerEfl.cpp and somewhere Ecore_Timer is used ?

[YoungTaeck Song]

(In reply to comment #2)
> How do you think about applying this in SharedTimerEfl.cpp and somewhere Ecore_Timer is used ?

This patch for RunLoopEfl.cpp. Please see https://bugs.webkit.org/attachment.cgi?id=129848&action=review.
So I think it's not suitable to using SharedTimerEfl.cpp

[YoungTaeck Song]

Created attachment 130057 [details]
Patch

[YoungTaeck Song]

(In reply to comment #2)
> How do you think about applying this in SharedTimerEfl.cpp and somewhere Ecore_Timer is used ?

Sorry, Ryuan. I misunderstood your intentions.
I changed SharedTimerEfl.cpp to use OwnPtr<Ecore_timer> at next patch.

[Ryuan Choi]

(In reply to comment #5)
> (In reply to comment #2)
> > How do you think about applying this in SharedTimerEfl.cpp and somewhere Ecore_Timer is used ?
> 
> Sorry, Ryuan. I misunderstood your intentions.
> I changed SharedTimerEfl.cpp to use OwnPtr<Ecore_timer> at next patch.

You are missing WebCore's ChangeLog.

[YoungTaeck Song]

Created attachment 130066 [details]
Patch

[Ryuan Choi]

(In reply to comment #7)
> Created an attachment (id=130066) [details]
> Patch

Looks good to me.

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 130066 [details]
Patch

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

> Source/WebCore/platform/efl/SharedTimerEfl.cpp:41
> -static Ecore_Timer *_sharedTimer = 0;
> +static OwnPtr<Ecore_Timer> sharedTimer;

Declaring a static object like this makes the code prone to all sorts of problems, since you can't guarantee when this object will be initialized. Either keep the old version, or use the DEFINE_STATIC_LOCAL macro to make sure the object is created on first use.

[YoungTaeck Song]

Created attachment 130247 [details]
Patch

[YoungTaeck Song]

(In reply to comment #9)
> (From update of attachment 130066 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=130066&action=review
> 
> > Source/WebCore/platform/efl/SharedTimerEfl.cpp:41
> > -static Ecore_Timer *_sharedTimer = 0;
> > +static OwnPtr<Ecore_Timer> sharedTimer;
> 
> Declaring a static object like this makes the code prone to all sorts of problems, since you can't guarantee when this object will be initialized. Either keep the old version, or use the DEFINE_STATIC_LOCAL macro to make sure the object is created on first use.

Thanks for your review.
We decided to keep the old version.

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 130247 [details]
Patch

Looks OK to me.

[Gyuyoung Kim]

Comment on attachment 130247 [details]
Patch

LGTM.

[WebKit Review Bot]

Comment on attachment 130247 [details]
Patch

Clearing flags on attachment: 130247

Committed r110876: <http://trac.webkit.org/changeset/110876>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.