Commit 0bc54557 authored by mrowe@apple.com's avatar mrowe@apple.com

Bug 34974: Leak of ScheduledAction during layout tests

<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: http://svn.webkit.org/repository/webkit/trunk@54835 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 1eb1b942
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.
......
......@@ -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);
......
......@@ -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));
}
......
......@@ -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;
......
......@@ -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
......
......@@ -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(),
......
......@@ -28,20 +28,21 @@
#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;
......
......@@ -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) {
......
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment