Changeset 142454 in webkit

Timestamp:

Feb 11, 2013 4:44:34 AM (11 years ago)

Author:

Christophe Dumez

Message:

[EFL] Stop using smart pointers for Ecore_Timer
​https://bugs.webkit.org/show_bug.cgi?id=109409

Reviewed by Kenneth Rohde Christiansen.

Source/WebCore:

Stop using a smart pointer for Ecore_Timer in RunLoop::TimerBase. This
is a bad idea because the timer handle becomes invalid as soon as the
timer callback returns ECORE_CALLBACK_CANCEL. This may lead to crashes
on destruction because OwnPtr calls ecore_timer_del() on an invalid
handle.

No new tests, already covered by exiting tests.

• platform/RunLoop.h:

(TimerBase):

• platform/efl/RunLoopEfl.cpp:

(WebCore::RunLoop::TimerBase::timerFired):
(WebCore::RunLoop::TimerBase::start):
(WebCore::RunLoop::TimerBase::stop):

Source/WTF:

Remove support in OwnPtr for EFL's Ecore_Timer. It is a bad idea to use
OwnPtr for Ecore_Timer because the timer handle may become invalid.

• wtf/OwnPtrCommon.h:

(WTF):

• wtf/efl/OwnPtrEfl.cpp:

Location:

trunk/Source

Files:

• WTF/ChangeLog (modified) (1 diff)
• WTF/wtf/OwnPtrCommon.h (modified) (2 diffs)
• WTF/wtf/efl/OwnPtrEfl.cpp (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/platform/RunLoop.h (modified) (1 diff)
• WebCore/platform/efl/RunLoopEfl.cpp (modified) (3 diffs)

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2013-02-11  Christophe Dumez  <ch.dumez@sisa.samsung.com>
+
+        [EFL] Stop using smart pointers for Ecore_Timer
+        https://bugs.webkit.org/show_bug.cgi?id=109409
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        Remove support in OwnPtr for EFL's Ecore_Timer. It is a bad idea to use
+        OwnPtr for Ecore_Timer because the timer handle may become invalid.
+
+        * wtf/OwnPtrCommon.h:
+        (WTF):
+        * wtf/efl/OwnPtrEfl.cpp:
+
 2013-02-11  Abhishek Arya  <inferno@chromium.org>
 

trunk/Source/WTF/wtf/OwnPtrCommon.h

@@ -46 +46 @@
 typedef struct _Eina_Module Eina_Module;
 #if USE(EO)
-typedef struct _Eo Ecore_Timer;
 typedef struct _Eo Evas_Object;
 #else
-typedef struct _Ecore_Timer Ecore_Timer;
 typedef struct _Evas_Object Evas_Object;
 #endif
@@ -80 +78 @@
     WTF_EXPORT_PRIVATE void deleteOwnedPtr(Ecore_IMF_Context*);
     WTF_EXPORT_PRIVATE void deleteOwnedPtr(Ecore_Pipe*);
-    WTF_EXPORT_PRIVATE void deleteOwnedPtr(Ecore_Timer*);
     WTF_EXPORT_PRIVATE void deleteOwnedPtr(Eina_Hash*);
     WTF_EXPORT_PRIVATE void deleteOwnedPtr(Eina_Module*);

trunk/Source/WTF/wtf/efl/OwnPtrEfl.cpp

@@ -64 +64 @@
 }
 
-void deleteOwnedPtr(Ecore_Timer* ptr)
-{
-    if (ptr)
-        ecore_timer_del(ptr);
-}
-
 void deleteOwnedPtr(Ecore_IMF_Context* ptr)
 {

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-02-11  Christophe Dumez  <ch.dumez@sisa.samsung.com>
+
+        [EFL] Stop using smart pointers for Ecore_Timer
+        https://bugs.webkit.org/show_bug.cgi?id=109409
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        Stop using a smart pointer for Ecore_Timer in RunLoop::TimerBase. This
+        is a bad idea because the timer handle becomes invalid as soon as the
+        timer callback returns ECORE_CALLBACK_CANCEL. This may lead to crashes
+        on destruction because OwnPtr calls ecore_timer_del() on an invalid
+        handle.
+
+        No new tests, already covered by exiting tests.
+
+        * platform/RunLoop.h:
+        (TimerBase):
+        * platform/efl/RunLoopEfl.cpp:
+        (WebCore::RunLoop::TimerBase::timerFired):
+        (WebCore::RunLoop::TimerBase::start):
+        (WebCore::RunLoop::TimerBase::stop):
+
 2013-02-11  Vladislav Kaznacheev  <kaznacheev@chromium.org>
 

trunk/Source/WebCore/platform/RunLoop.h

@@ -106 +106 @@
 #elif PLATFORM(EFL)
         static bool timerFired(void* data);
-        OwnPtr<Ecore_Timer> m_timer;
+        Ecore_Timer* m_timer;
         bool m_isRepeating;
 #endif

trunk/Source/WebCore/platform/efl/RunLoopEfl.cpp

@@ -87 +87 @@
 
 RunLoop::TimerBase::TimerBase(RunLoop*)
-    : m_isRepeating(false)
+    : m_timer(0)
+    , m_isRepeating(false)
 {
 }
@@ -103 +104 @@
 
     if (!timer->m_isRepeating) {
-        timer->m_timer = nullptr;
+        timer->m_timer = 0;
         return ECORE_CALLBACK_CANCEL;
     }
@@ -112 +113 @@
 void RunLoop::TimerBase::start(double nextFireInterval, bool repeat)
 {
+    if (isActive())
+        stop();
+
     m_isRepeating = repeat;
-    m_timer = adoptPtr(ecore_timer_add(nextFireInterval, reinterpret_cast<Ecore_Task_Cb>(timerFired), this));
+    ASSERT(!m_timer);
+    m_timer = ecore_timer_add(nextFireInterval, reinterpret_cast<Ecore_Task_Cb>(timerFired), this);
 }
 
 void RunLoop::TimerBase::stop()
 {
-    m_timer = nullptr;
+    if (m_timer) {
+        ecore_timer_del(m_timer);
+        m_timer = 0;
+    }
 }