Bug 34974: Leak of ScheduledAction during layout tests
authormrowe@apple.com <mrowe@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Feb 2010 22:53:46 +0000 (22:53 +0000)
committermrowe@apple.com <mrowe@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Feb 2010 22:53:46 +0000 (22:53 +0000)
<https://bugs.webkit.org/show_bug.cgi?id=34974>

Reviewed by Alexey Proskuryakov.

ScheduledAction::create was returning a raw pointer which was threaded down through to an OwnPtr in DOMTimer.
If any of the code paths in between hit an error case and returned early the raw pointer would be leaked.  We
can avoid this by passing it as a PassOwnPtr.  This will ensure that the ScheduledAction is cleaned up should
an error case be hit.

* bindings/js/JSDOMWindowCustom.cpp:
(WebCore::JSDOMWindow::setTimeout): Store the newly-created ScheduledAction in an OwnPtr and then hand it off
as the function argument.
(WebCore::JSDOMWindow::setInterval): Ditto.
* bindings/js/JSWorkerContextCustom.cpp:
(WebCore::JSWorkerContext::setTimeout): Ditto.
(WebCore::JSWorkerContext::setInterval): Ditto.
* bindings/js/ScheduledAction.cpp:
(WebCore::ScheduledAction::create): Return a PassOwnPtr.
* bindings/js/ScheduledAction.h:
* page/DOMTimer.cpp:
(WebCore::DOMTimer::DOMTimer): Update argument type.
(WebCore::DOMTimer::install): Ditto.
* page/DOMTimer.h:
* page/DOMWindow.cpp:
(WebCore::DOMWindow::setTimeout): Ditto.
(WebCore::DOMWindow::setInterval): Ditto.
* page/DOMWindow.h:

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

WebCore/ChangeLog
WebCore/bindings/js/JSDOMWindowCustom.cpp
WebCore/bindings/js/JSWorkerContextCustom.cpp
WebCore/bindings/js/ScheduledAction.cpp
WebCore/bindings/js/ScheduledAction.h
WebCore/page/DOMTimer.cpp
WebCore/page/DOMTimer.h
WebCore/page/DOMWindow.cpp
WebCore/page/DOMWindow.h

index 83d5374c985e8c75c0a96d453a0910554b5570f6..08fb4ad0851b952c20bc1b2fe0250e2c3e349c1b 100644 (file)
@@ -1,3 +1,34 @@
+2010-02-16  Mark Rowe  <mrowe@apple.com>
+
+        Reviewed by Alexey Proskuryakov.
+
+        Bug 34974: Leak of ScheduledAction during layout tests
+        <https://bugs.webkit.org/show_bug.cgi?id=34974>
+
+        ScheduledAction::create was returning a raw pointer which was threaded down through to an OwnPtr in DOMTimer.
+        If any of the code paths in between hit an error case and returned early the raw pointer would be leaked.  We
+        can avoid this by passing it as a PassOwnPtr.  This will ensure that the ScheduledAction is cleaned up should
+        an error case be hit.
+
+        * bindings/js/JSDOMWindowCustom.cpp:
+        (WebCore::JSDOMWindow::setTimeout): Store the newly-created ScheduledAction in an OwnPtr and then hand it off
+        as the function argument.
+        (WebCore::JSDOMWindow::setInterval): Ditto.
+        * bindings/js/JSWorkerContextCustom.cpp:
+        (WebCore::JSWorkerContext::setTimeout): Ditto.
+        (WebCore::JSWorkerContext::setInterval): Ditto.
+        * bindings/js/ScheduledAction.cpp:
+        (WebCore::ScheduledAction::create): Return a PassOwnPtr.
+        * bindings/js/ScheduledAction.h:
+        * page/DOMTimer.cpp:
+        (WebCore::DOMTimer::DOMTimer): Update argument type.
+        (WebCore::DOMTimer::install): Ditto.
+        * page/DOMTimer.h:
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::setTimeout): Ditto.
+        (WebCore::DOMWindow::setInterval): Ditto.
+        * page/DOMWindow.h:
+
 2010-02-16  Nikolas Zimmermann  <nzimmermann@rim.com>
 
         Reviewed by David Hyatt.
index 82fa22793e56a3885156c77a86fb4c3724e612d7..bbd4a51bdf827961f188826e2cc614f0b747c0ea 100644 (file)
@@ -912,13 +912,13 @@ JSValue JSDOMWindow::postMessage(ExecState* exec, const ArgList& args)
 
 JSValue JSDOMWindow::setTimeout(ExecState* exec, const ArgList& args)
 {
-    ScheduledAction* action = ScheduledAction::create(exec, args, currentWorld(exec));
+    OwnPtr<ScheduledAction> action = ScheduledAction::create(exec, args, currentWorld(exec));
     if (exec->hadException())
         return jsUndefined();
     int delay = args.at(1).toInt32(exec);
 
     ExceptionCode ec = 0;
-    int result = impl()->setTimeout(action, delay, ec);
+    int result = impl()->setTimeout(action.release(), delay, ec);
     setDOMException(exec, ec);
 
     return jsNumber(exec, result);
@@ -926,13 +926,13 @@ JSValue JSDOMWindow::setTimeout(ExecState* exec, const ArgList& args)
 
 JSValue JSDOMWindow::setInterval(ExecState* exec, const ArgList& args)
 {
-    ScheduledAction* action = ScheduledAction::create(exec, args, currentWorld(exec));
+    OwnPtr<ScheduledAction> action = ScheduledAction::create(exec, args, currentWorld(exec));
     if (exec->hadException())
         return jsUndefined();
     int delay = args.at(1).toInt32(exec);
 
     ExceptionCode ec = 0;
-    int result = impl()->setInterval(action, delay, ec);
+    int result = impl()->setInterval(action.release(), delay, ec);
     setDOMException(exec, ec);
 
     return jsNumber(exec, result);
index d6c8dbdf84e288769a029ece7fab21fd34fc67b2..bf9409c83e32a35a1f7d61b3e91d23e7cf299e66 100644 (file)
@@ -143,20 +143,20 @@ JSValue JSWorkerContext::removeEventListener(ExecState* exec, const ArgList& arg
 
 JSValue JSWorkerContext::setTimeout(ExecState* exec, const ArgList& args)
 {
-    ScheduledAction* action = ScheduledAction::create(exec, args, currentWorld(exec));
+    OwnPtr<ScheduledAction> action = ScheduledAction::create(exec, args, currentWorld(exec));
     if (exec->hadException())
         return jsUndefined();
     int delay = args.at(1).toInt32(exec);
-    return jsNumber(exec, impl()->setTimeout(action, delay));
+    return jsNumber(exec, impl()->setTimeout(action.release(), delay));
 }
 
 JSValue JSWorkerContext::setInterval(ExecState* exec, const ArgList& args)
 {
-    ScheduledAction* action = ScheduledAction::create(exec, args, currentWorld(exec));
+    OwnPtr<ScheduledAction> action = ScheduledAction::create(exec, args, currentWorld(exec));
     if (exec->hadException())
         return jsUndefined();
     int delay = args.at(1).toInt32(exec);
-    return jsNumber(exec, impl()->setInterval(action, delay));
+    return jsNumber(exec, impl()->setInterval(action.release(), delay));
 }
 
 
index be9012588837a4bcd1a2f132702c5d4c652ae037..be62bb888b9d18f97517cefbb7a621b7b5e74c3a 100644 (file)
@@ -47,7 +47,7 @@ using namespace JSC;
 
 namespace WebCore {
 
-ScheduledAction* ScheduledAction::create(ExecState* exec, const ArgList& args, DOMWrapperWorld* isolatedWorld)
+PassOwnPtr<ScheduledAction> ScheduledAction::create(ExecState* exec, const ArgList& args, DOMWrapperWorld* isolatedWorld)
 {
     JSValue v = args.at(0);
     CallData callData;
index dd13ab165f5cd265214141fada49bcef6ffce401..3b7e001cd7b9f8d46e76f143c7c5600a38c8f404 100644 (file)
@@ -24,6 +24,7 @@
 #include <JSDOMBinding.h>
 #include <runtime/JSCell.h>
 #include <runtime/Protect.h>
+#include <wtf/PassOwnPtr.h>
 #include <wtf/Vector.h>
 
 namespace JSC {
@@ -42,7 +43,7 @@ namespace WebCore {
     */
     class ScheduledAction : public Noncopyable {
     public:
-        static ScheduledAction* create(JSC::ExecState*, const JSC::ArgList&, DOMWrapperWorld* isolatedWorld);
+        static PassOwnPtr<ScheduledAction> create(JSC::ExecState*, const JSC::ArgList&, DOMWrapperWorld* isolatedWorld);
 
         void execute(ScriptExecutionContext*);
 
@@ -56,7 +57,7 @@ namespace WebCore {
 
         void executeFunctionInContext(JSC::JSGlobalObject*, JSC::JSValue thisValue);
         void execute(Document*);
-#if ENABLE(WORKERS)        
+#if ENABLE(WORKERS)
         void execute(WorkerContext*);
 #endif
 
index 8971bb7f8c6c4dbfd26c47389ee135fe83223fec..72dc9ac05373405bcb53cedd06c7e58f9dbdd303 100644 (file)
@@ -43,7 +43,7 @@ double DOMTimer::s_minTimerInterval = 0.010; // 10 milliseconds
 
 static int timerNestingLevel = 0;
 
-DOMTimer::DOMTimer(ScriptExecutionContext* context, ScheduledAction* action, int timeout, bool singleShot)
+DOMTimer::DOMTimer(ScriptExecutionContext* context, PassOwnPtr<ScheduledAction> action, int timeout, bool singleShot)
     : ActiveDOMObject(context, this)
     , m_action(action)
     , m_nextFireInterval(0)
@@ -82,7 +82,7 @@ DOMTimer::~DOMTimer()
         scriptExecutionContext()->removeTimeout(m_timeoutId);
 }
 
-int DOMTimer::install(ScriptExecutionContext* context, ScheduledAction* action, int timeout, bool singleShot)
+int DOMTimer::install(ScriptExecutionContext* context, PassOwnPtr<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(),
index 460430f4d28c18b9a753de078880a39a5a47a516..da38178cdd96fb8417c0a5eb076198e0c422b081 100644 (file)
 #define DOMTimer_h
 
 #include "ActiveDOMObject.h"
+#include "ScheduledAction.h"
 #include "Timer.h"
 #include <wtf/OwnPtr.h>
+#include <wtf/PassOwnPtr.h>
 
 namespace WebCore {
 
     class InspectorTimelineAgent;
-    class ScheduledAction;
 
     class DOMTimer : public TimerBase, public ActiveDOMObject {
     public:
         virtual ~DOMTimer();
         // Creates a new timer owned by specified ScriptExecutionContext, starts it
         // and returns its Id.
-        static int install(ScriptExecutionContext*, ScheduledAction*, int timeout, bool singleShot);
+        static int install(ScriptExecutionContext*, PassOwnPtr<ScheduledAction>, int timeout, bool singleShot);
         static void removeById(ScriptExecutionContext*, int timeoutId);
 
         // ActiveDOMObject
@@ -59,7 +60,7 @@ namespace WebCore {
         static void setMinTimerInterval(double value) { s_minTimerInterval = value; }
 
     private:
-        DOMTimer(ScriptExecutionContext*, ScheduledAction*, int timeout, bool singleShot);
+        DOMTimer(ScriptExecutionContext*, PassOwnPtr<ScheduledAction>, int timeout, bool singleShot);
         virtual void fired();
 
         int m_timeoutId;
index 2f0f84fdb070c119e6adfacf5041ec61b7bd9b05..6af22c37c87c991eba670696e171448227113917 100644 (file)
@@ -1252,7 +1252,7 @@ void DOMWindow::resizeTo(float width, float height) const
     page->chrome()->setWindowRect(fr);
 }
 
-int DOMWindow::setTimeout(ScheduledAction* action, int timeout, ExceptionCode& ec)
+int DOMWindow::setTimeout(PassOwnPtr<ScheduledAction> action, int timeout, ExceptionCode& ec)
 {
     ScriptExecutionContext* context = scriptExecutionContext();
     if (!context) {
@@ -1270,7 +1270,7 @@ void DOMWindow::clearTimeout(int timeoutId)
     DOMTimer::removeById(context, timeoutId);
 }
 
-int DOMWindow::setInterval(ScheduledAction* action, int timeout, ExceptionCode& ec)
+int DOMWindow::setInterval(PassOwnPtr<ScheduledAction> action, int timeout, ExceptionCode& ec)
 {
     ScriptExecutionContext* context = scriptExecutionContext();
     if (!context) {
index dc1e68c689eab487d62838a7e7d882a3a4011fdd..4452dbbaae93c3c7def0397783c449aa1152c290 100644 (file)
@@ -237,9 +237,9 @@ namespace WebCore {
         void resizeTo(float width, float height) const;
 
         // Timers
-        int setTimeout(ScheduledAction*, int timeout, ExceptionCode&);
+        int setTimeout(PassOwnPtr<ScheduledAction>, int timeout, ExceptionCode&);
         void clearTimeout(int timeoutId);
-        int setInterval(ScheduledAction*, int timeout, ExceptionCode&);
+        int setInterval(PassOwnPtr<ScheduledAction>, int timeout, ExceptionCode&);
         void clearInterval(int timeoutId);
 
         // Events