2009-01-02 Dmitry Titov <dimich@chromium.org>
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 3 Jan 2009 02:27:33 +0000 (02:27 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 3 Jan 2009 02:27:33 +0000 (02:27 +0000)
        Reviewed and landed by Darin.

        https://bugs.webkit.org/show_bug.cgi?id=23025
        DOMTimer lifetime cleanup: timeoutMap methods on Document now do not delete the timer.
        Instead, all 3 places that delete timers do it directly calling 'delete' and then timer's dtor removes the ID from the timeoutMap.
        Note that in case the context is destroyed and timers are deleted at once, the check in ~DOMTimer() prevents
        unnecessary HashMap remove in case the Document is being destroyed.

        * bindings/js/DOMTimer.cpp:
        (WebCore::DOMTimer::~DOMTimer): removes the id from the timeoutMap.
        (WebCore::DOMTimer::install):
        (WebCore::DOMTimer::removeById): simply uses 'delete this'.
        (WebCore::DOMTimer::fired): same.
        (WebCore::DOMTimer::contextDestroyed): same.
        * dom/Document.cpp:
        (WebCore::Document::removeTimeout): now it only removes the id from the map, does not delete the timer.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@39567 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/bindings/js/DOMTimer.cpp
WebCore/dom/Document.cpp

index 8e716b4..b47bf54 100644 (file)
@@ -1,3 +1,22 @@
+2009-01-02  Dmitry Titov  <dimich@chromium.org>
+
+        Reviewed and landed by Darin.
+
+        https://bugs.webkit.org/show_bug.cgi?id=23025
+        DOMTimer lifetime cleanup: timeoutMap methods on Document now do not delete the timer.
+        Instead, all 3 places that delete timers do it directly calling 'delete' and then timer's dtor removes the ID from the timeoutMap.
+        Note that in case the context is destroyed and timers are deleted at once, the check in ~DOMTimer() prevents 
+        unnecessary HashMap remove in case the Document is being destroyed.
+
+        * bindings/js/DOMTimer.cpp:
+        (WebCore::DOMTimer::~DOMTimer): removes the id from the timeoutMap.
+        (WebCore::DOMTimer::install):
+        (WebCore::DOMTimer::removeById): simply uses 'delete this'.
+        (WebCore::DOMTimer::fired): same.
+        (WebCore::DOMTimer::contextDestroyed): same.
+        * dom/Document.cpp:
+        (WebCore::Document::removeTimeout): now it only removes the id from the map, does not delete the timer.
+
 2009-01-02  Darin Adler  <darin@apple.com>
 
         Reviewed by Sam Weinig.
index 7c8f68e..911da71 100644 (file)
@@ -78,12 +78,17 @@ DOMTimer::DOMTimer(ScriptExecutionContext* context, ScheduledAction* action, int
 
 DOMTimer::~DOMTimer()
 {
+    if (scriptExecutionContext()) {
+        ASSERT(scriptExecutionContext()->isDocument());
+        static_cast<Document*>(scriptExecutionContext())->removeTimeout(m_timeoutId);
+    }
 }
     
 int DOMTimer::install(ScriptExecutionContext* context, ScheduledAction* action, int timeout, bool singleShot)
 {
     // DOMTimer constructor links the new timer into a list of ActiveDOMObjects held by the 'context'.
-    // The timer is deleted when context is deleted (DOMTimer::contextDestroyed) or explicitly via DOMTimer::removeById().
+    // The timer is deleted when context is deleted (DOMTimer::contextDestroyed) or explicitly via DOMTimer::removeById(),
+    // or if it is a one-time timer and it has fired (DOMTimer::fired).
     DOMTimer* timer = new DOMTimer(context, action, timeout, singleShot);
     return timer->m_timeoutId;
 }
@@ -96,7 +101,7 @@ void DOMTimer::removeById(ScriptExecutionContext* context, int timeoutId)
     if (timeoutId <= 0)
         return;
     ASSERT(context && context->isDocument());
-    static_cast<Document*>(context)->removeTimeout(timeoutId);
+    delete static_cast<Document*>(context)->findTimeout(timeoutId);
 }
 
 void DOMTimer::fired()
@@ -121,9 +126,8 @@ void DOMTimer::fired()
     ScheduledAction* action = m_action.release();
 
     // No access to member variables after this point.
-    ASSERT(context->isDocument());
-    static_cast<Document*>(context)->removeTimeout(m_timeoutId);
-
+    delete this;
+    
     action->execute(context);
     delete action;
     timerNestingLevel = 0;
index 83928ed..ab4d5e5 100644 (file)
@@ -4286,8 +4286,7 @@ void Document::addTimeout(int timeoutId, DOMTimer* timer)
 
 void Document::removeTimeout(int timeoutId)
 {
-    DOMTimer* timer = m_timeouts.take(timeoutId);
-    delete timer;
+    m_timeouts.remove(timeoutId);
 }
 
 DOMTimer* Document::findTimeout(int timeoutId)