[webkit-reviews] review denied: [Bug 62380] [EFL] correct sharedTimer value in SharedTimerEfl.cpp : [Attachment 96646] remove _sharedTimer = 0 in timerEvent()

[bugzilla-daemon at webkit.org]

Lucas De Marchi <demarchi at webkit.org> has denied  review:
Bug 62380: [EFL] correct sharedTimer value in SharedTimerEfl.cpp
https://bugs.webkit.org/show_bug.cgi?id=62380

Attachment 96646: remove _sharedTimer = 0 in timerEvent()
https://bugs.webkit.org/attachment.cgi?id=96646&action=review

------- Additional Comments from Lucas De Marchi <demarchi at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=96646&action=review

> Source/WebCore/ChangeLog:6
> +	   SharedTimerEfl.cpp set _sharedTimer = 0 is incorrect in timerEvent,
stopSharedTimer can be call by both stopSharedTimer() or by addNewTimer()
withint timerFunction(). When addNewTimer(), the global _sharedTimer is
replaced by the new timer object. Setting it to 0 loses the new timer object
within ecore_timer without deleting it.
> +	   https://bugs.webkit.org/show_bug.cgi?id=62380

Sorry, I didn't understand what you mean by "stopSharedTimer can be call by
both stopSharedTimer() or by addNewTimer() withint timerFunction()".

Reading this file, what I understand is:

 * If WebCore added a new timer by calling WebCore::addNewTimer(), the old
timer will be deleted by calling WebCore::stopSharedTimer(), which in turn
deletes the timer inside ecore calling ecore_timer_del()

 * If the timer is triggered before WebCore calls WebCore::stopSharedTimer(),
then the registered function is called and the timer is deleted inside ecore by
returning ECORE_CALLBACK_CANCEL


The only reason this might be wrong (and then your patch partially right) is if
the timer is supposed to keep triggering until the WebCore::stopSharedTimer()
is called. In that case you have to fix the return value too, which should be
EINA_TRUE.