[Webkit-unassigned] [Bug 95923] [EFL] Implement GCActivityCallback

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=95923





--- Comment #28 from Hojong Han <hojong.han at samsung.com>  2013-05-21 23:56:28 PST ---
(From update of attachment 202500)
View in context: https://bugs.webkit.org/attachment.cgi?id=202500&action=review

>>>> Source/JavaScriptCore/heap/HeapTimer.h:42
>>>> +#include <Ecore.h>
>>> 
>>> A forward declaration of Ecore_Timer would suffice here. And this include can be moved to the cpp.
>> 
>> Do you mean that it'd better move "#include <Ecore.h>" to the cpp?
> 
> Yes, and have a forward declaration here:
> typedef struct _Ecore_Timer Ecore_Timer;

HeapTimer.h is included in GCActivityCallback.h and there's Ecore_Task_Cb in GCActivityCallback.cpp.
What do you think keeping it in HeapTimer.h?

>>>> Source/JavaScriptCore/runtime/GCActivityCallback.cpp:127
>>>> +    m_timer = ecore_timer_add(newDelay, reinterpret_cast<Ecore_Task_Cb>(HeapTimer::timerEvent), this);
>>> 
>>> I would add a ASSERT(!m_timer); right before this line to make sure we never leak.
>> 
>> I'll fix to set m_timer as 0 in stop().
> 
> You already set m_timer to 0 in stop. All I ask for in an extra assert to make sure someone does not remove the stop() later on :)

yes, it's good to apply for the case as you mentioned.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.