Commit 57d6cce7 authored by ggaren@apple.com's avatar ggaren@apple.com

2009-04-20 Geoffrey Garen <ggaren@apple.com>

        Reviewed by Darin Adler.

        Last patch for https://bugs.webkit.org/show_bug.cgi?id=21260
        Unbounded memory growth when churning elements with anonymous event handler functions
        
        Converted "lazy" event listeners to be unprotected, just like all the others.

        * bindings/js/JSEventListener.cpp:
        (WebCore::JSEventListener::JSEventListener):
        (WebCore::JSEventListener::~JSEventListener):
        (WebCore::JSEventListener::jsFunction):
        (WebCore::JSEventListener::markJSFunction):
        (WebCore::JSEventListener::handleEvent):
        (WebCore::JSEventListener::virtualIsInline):
        * bindings/js/JSEventListener.h:
        (WebCore::JSEventListener::isInline): Merged JSAbstractEventListener
        into JSEventListener. Now that the only difference between JSEventListener
        and JSLazyEventListener is that JSLazyEventListener compiles lazily,
        there's no need for an abstract base class.

        * bindings/js/JSLazyEventListener.cpp: Converted JSLazyEventListener to
        inherit from JSEventListener and match its un-GC-protected behavior.
        (WebCore::JSLazyEventListener::JSLazyEventListener): ditto
        (WebCore::JSLazyEventListener::parseCode): ditto
        (WebCore::createInlineEventListener): When creating a lazy event listener,
        ensure that the related node has a JS wrapper to mark the listener. Since
        the parser makes these listeners, it's possible that no JS reference has
        been made to the node yet.
        * bindings/js/JSLazyEventListener.h: ditto

        * dom/EventListener.h:
        (WebCore::EventListener::clearJSFunction): Removed an usused function.



git-svn-id: svn://svn.chromium.org/blink/trunk@42687 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent a1350a31
2009-04-20 Geoffrey Garen <ggaren@apple.com>
Reviewed by Darin Adler.
Last patch for https://bugs.webkit.org/show_bug.cgi?id=21260
Unbounded memory growth when churning elements with anonymous event handler functions
Converted "lazy" event listeners to be unprotected, just like all the others.
* bindings/js/JSEventListener.cpp:
(WebCore::JSEventListener::JSEventListener):
(WebCore::JSEventListener::~JSEventListener):
(WebCore::JSEventListener::jsFunction):
(WebCore::JSEventListener::markJSFunction):
(WebCore::JSEventListener::handleEvent):
(WebCore::JSEventListener::virtualIsInline):
* bindings/js/JSEventListener.h:
(WebCore::JSEventListener::isInline): Merged JSAbstractEventListener
into JSEventListener. Now that the only difference between JSEventListener
and JSLazyEventListener is that JSLazyEventListener compiles lazily,
there's no need for an abstract base class.
* bindings/js/JSLazyEventListener.cpp: Converted JSLazyEventListener to
inherit from JSEventListener and match its un-GC-protected behavior.
(WebCore::JSLazyEventListener::JSLazyEventListener): ditto
(WebCore::JSLazyEventListener::parseCode): ditto
(WebCore::createInlineEventListener): When creating a lazy event listener,
ensure that the related node has a JS wrapper to mark the listener. Since
the parser makes these listeners, it's possible that no JS reference has
been made to the node yet.
* bindings/js/JSLazyEventListener.h: ditto
* dom/EventListener.h:
(WebCore::EventListener::clearJSFunction): Removed an usused function.
2009-04-20 Justin Garcia <justin.garcia@apple.com> 2009-04-20 Justin Garcia <justin.garcia@apple.com>
Reviewed by Dan Bernstein. Reviewed by Dan Bernstein.
...@@ -31,7 +31,35 @@ using namespace JSC; ...@@ -31,7 +31,35 @@ using namespace JSC;
namespace WebCore { namespace WebCore {
void JSAbstractEventListener::handleEvent(Event* event, bool isWindowEvent) JSEventListener::JSEventListener(JSObject* function, JSDOMGlobalObject* globalObject, bool isInline)
: m_jsFunction(function)
, m_globalObject(globalObject)
, m_isInline(isInline)
{
if (!m_isInline && m_jsFunction)
globalObject->jsEventListeners().set(m_jsFunction, this);
}
JSEventListener::~JSEventListener()
{
if (!m_isInline && m_jsFunction && m_globalObject)
m_globalObject->jsEventListeners().remove(m_jsFunction);
}
JSObject* JSEventListener::jsFunction() const
{
return m_jsFunction;
}
void JSEventListener::markJSFunction()
{
if (m_jsFunction && !m_jsFunction->marked())
m_jsFunction->mark();
if (m_globalObject && !m_globalObject->marked())
m_globalObject->mark();
}
void JSEventListener::handleEvent(Event* event, bool isWindowEvent)
{ {
JSLock lock(false); JSLock lock(false);
...@@ -39,7 +67,7 @@ void JSAbstractEventListener::handleEvent(Event* event, bool isWindowEvent) ...@@ -39,7 +67,7 @@ void JSAbstractEventListener::handleEvent(Event* event, bool isWindowEvent)
if (!jsFunction) if (!jsFunction)
return; return;
JSDOMGlobalObject* globalObject = this->globalObject(); JSDOMGlobalObject* globalObject = m_globalObject;
// Null check as clearGlobalObject() can clear this and we still get called back by // Null check as clearGlobalObject() can clear this and we still get called back by
// xmlhttprequest objects. See http://bugs.webkit.org/show_bug.cgi?id=13275 // xmlhttprequest objects. See http://bugs.webkit.org/show_bug.cgi?id=13275
// FIXME: Is this check still necessary? Requests are supposed to be stopped before clearGlobalObject() is called. // FIXME: Is this check still necessary? Requests are supposed to be stopped before clearGlobalObject() is called.
...@@ -125,57 +153,9 @@ void JSAbstractEventListener::handleEvent(Event* event, bool isWindowEvent) ...@@ -125,57 +153,9 @@ void JSAbstractEventListener::handleEvent(Event* event, bool isWindowEvent)
} }
} }
bool JSAbstractEventListener::virtualIsInline() const bool JSEventListener::virtualIsInline() const
{ {
return m_isInline; return m_isInline;
} }
// -------------------------------------------------------------------------
JSEventListener::JSEventListener(JSObject* function, JSDOMGlobalObject* globalObject, bool isInline)
: JSAbstractEventListener(isInline)
, m_jsFunction(function)
, m_globalObject(globalObject)
{
if (!isInline && m_jsFunction)
globalObject->jsEventListeners().set(m_jsFunction, this);
}
inline void JSEventListener::clearJSFunctionInline()
{
if (!isInline() && m_jsFunction && m_globalObject)
m_globalObject->jsEventListeners().remove(m_jsFunction);
m_jsFunction = 0;
m_globalObject = 0;
}
JSEventListener::~JSEventListener()
{
clearJSFunctionInline();
}
JSObject* JSEventListener::jsFunction() const
{
return m_jsFunction;
}
void JSEventListener::clearJSFunction()
{
clearJSFunctionInline();
}
JSDOMGlobalObject* JSEventListener::globalObject() const
{
return m_globalObject;
}
void JSEventListener::markJSFunction()
{
if (m_jsFunction && !m_jsFunction->marked())
m_jsFunction->mark();
if (m_globalObject && !m_globalObject->marked())
m_globalObject->mark();
}
} // namespace WebCore } // namespace WebCore
...@@ -28,25 +28,7 @@ namespace WebCore { ...@@ -28,25 +28,7 @@ namespace WebCore {
class JSDOMGlobalObject; class JSDOMGlobalObject;
class JSAbstractEventListener : public EventListener { class JSEventListener : public EventListener {
public:
bool isInline() const { return m_isInline; }
protected:
JSAbstractEventListener(bool isInline)
: m_isInline(isInline)
{
}
private:
virtual void handleEvent(Event*, bool isWindowEvent);
virtual JSDOMGlobalObject* globalObject() const = 0;
virtual bool virtualIsInline() const;
bool m_isInline;
};
class JSEventListener : public JSAbstractEventListener {
public: public:
static PassRefPtr<JSEventListener> create(JSC::JSObject* listener, JSDOMGlobalObject* globalObject, bool isInline) static PassRefPtr<JSEventListener> create(JSC::JSObject* listener, JSDOMGlobalObject* globalObject, bool isInline)
{ {
...@@ -56,19 +38,21 @@ namespace WebCore { ...@@ -56,19 +38,21 @@ namespace WebCore {
void clearGlobalObject() { m_globalObject = 0; } void clearGlobalObject() { m_globalObject = 0; }
bool isInline() const { return m_isInline; }
virtual JSC::JSObject* jsFunction() const; virtual JSC::JSObject* jsFunction() const;
virtual void clearJSFunction();
virtual void markJSFunction();
private: private:
JSEventListener(JSC::JSObject* function, JSDOMGlobalObject*, bool isInline); virtual void markJSFunction();
virtual void handleEvent(Event*, bool isWindowEvent);
virtual JSDOMGlobalObject* globalObject() const; virtual bool virtualIsInline() const;
void clearJSFunctionInline(); void clearJSFunctionInline();
JSC::JSObject* m_jsFunction; protected:
JSEventListener(JSC::JSObject* function, JSDOMGlobalObject*, bool isInline);
mutable JSC::JSObject* m_jsFunction;
JSDOMGlobalObject* m_globalObject; JSDOMGlobalObject* m_globalObject;
bool m_isInline;
}; };
} // namespace WebCore } // namespace WebCore
......
...@@ -44,8 +44,7 @@ static const String& eventParameterName(bool isSVGEvent) ...@@ -44,8 +44,7 @@ static const String& eventParameterName(bool isSVGEvent)
} }
JSLazyEventListener::JSLazyEventListener(const String& functionName, const String& eventParameterName, const String& code, JSDOMGlobalObject* globalObject, Node* node, int lineNumber) JSLazyEventListener::JSLazyEventListener(const String& functionName, const String& eventParameterName, const String& code, JSDOMGlobalObject* globalObject, Node* node, int lineNumber)
: JSAbstractEventListener(true) : JSEventListener(0, globalObject, true)
, m_globalObject(globalObject)
, m_functionName(functionName) , m_functionName(functionName)
, m_eventParameterName(eventParameterName) , m_eventParameterName(eventParameterName)
, m_code(code) , m_code(code)
...@@ -88,7 +87,7 @@ void JSLazyEventListener::parseCode() const ...@@ -88,7 +87,7 @@ void JSLazyEventListener::parseCode() const
return; return;
if (m_globalObject->scriptExecutionContext()->isDocument()) { if (m_globalObject->scriptExecutionContext()->isDocument()) {
JSDOMWindow* window = static_cast<JSDOMWindow*>(m_globalObject.get()); JSDOMWindow* window = static_cast<JSDOMWindow*>(m_globalObject);
Frame* frame = window->impl()->frame(); Frame* frame = window->impl()->frame();
if (!frame) if (!frame)
return; return;
...@@ -111,7 +110,7 @@ void JSLazyEventListener::parseCode() const ...@@ -111,7 +110,7 @@ void JSLazyEventListener::parseCode() const
// have been added with setAttribute from a script, and we should pass String() in that case. // have been added with setAttribute from a script, and we should pass String() in that case.
m_jsFunction = constructFunction(exec, args, Identifier(exec, m_functionName), sourceURL, m_lineNumber); // FIXME: is globalExec ok? m_jsFunction = constructFunction(exec, args, Identifier(exec, m_functionName), sourceURL, m_lineNumber); // FIXME: is globalExec ok?
JSFunction* listenerAsFunction = static_cast<JSFunction*>(m_jsFunction.get()); JSFunction* listenerAsFunction = static_cast<JSFunction*>(m_jsFunction);
if (exec->hadException()) { if (exec->hadException()) {
exec->clearException(); exec->clearException();
...@@ -136,11 +135,6 @@ void JSLazyEventListener::parseCode() const ...@@ -136,11 +135,6 @@ void JSLazyEventListener::parseCode() const
m_eventParameterName = String(); m_eventParameterName = String();
} }
JSDOMGlobalObject* JSLazyEventListener::globalObject() const
{
return m_globalObject;
}
PassRefPtr<JSLazyEventListener> createInlineEventListener(Node* node, Attribute* attr) PassRefPtr<JSLazyEventListener> createInlineEventListener(Node* node, Attribute* attr)
{ {
ASSERT(node); ASSERT(node);
...@@ -156,6 +150,12 @@ PassRefPtr<JSLazyEventListener> createInlineEventListener(Node* node, Attribute* ...@@ -156,6 +150,12 @@ PassRefPtr<JSLazyEventListener> createInlineEventListener(Node* node, Attribute*
return 0; return 0;
JSDOMWindow* globalObject = scriptController->globalObject(); JSDOMWindow* globalObject = scriptController->globalObject();
// Ensure that 'node' has a JavaScript wrapper to mark the event listener we're creating.
{
JSLock lock(false);
toJS(globalObject->globalExec(), node);
}
return JSLazyEventListener::create(attr->localName().string(), eventParameterName(node->isSVGElement()), attr->value(), globalObject, node, scriptController->eventHandlerLineNumber()); return JSLazyEventListener::create(attr->localName().string(), eventParameterName(node->isSVGElement()), attr->value(), globalObject, node, scriptController->eventHandlerLineNumber());
} }
...@@ -169,6 +169,7 @@ PassRefPtr<JSLazyEventListener> createInlineEventListener(Frame* frame, Attribut ...@@ -169,6 +169,7 @@ PassRefPtr<JSLazyEventListener> createInlineEventListener(Frame* frame, Attribut
if (!scriptController->isEnabled()) if (!scriptController->isEnabled())
return 0; return 0;
// 'globalObject' is the JavaScript wrapper that will mark the event listener we're creating.
JSDOMWindow* globalObject = scriptController->globalObject(); JSDOMWindow* globalObject = scriptController->globalObject();
return JSLazyEventListener::create(attr->localName().string(), eventParameterName(frame->document()->isSVGDocument()), attr->value(), globalObject, 0, scriptController->eventHandlerLineNumber()); return JSLazyEventListener::create(attr->localName().string(), eventParameterName(frame->document()->isSVGDocument()), attr->value(), globalObject, 0, scriptController->eventHandlerLineNumber());
......
...@@ -28,7 +28,7 @@ namespace WebCore { ...@@ -28,7 +28,7 @@ namespace WebCore {
class Attribute; class Attribute;
class Node; class Node;
class JSLazyEventListener : public JSAbstractEventListener { class JSLazyEventListener : public JSEventListener {
public: public:
static PassRefPtr<JSLazyEventListener> create(const String& functionName, const String& eventParameterName, const String& code, JSDOMGlobalObject* globalObject, Node* node, int lineNumber) static PassRefPtr<JSLazyEventListener> create(const String& functionName, const String& eventParameterName, const String& code, JSDOMGlobalObject* globalObject, Node* node, int lineNumber)
{ {
...@@ -36,20 +36,14 @@ namespace WebCore { ...@@ -36,20 +36,14 @@ namespace WebCore {
} }
virtual ~JSLazyEventListener(); virtual ~JSLazyEventListener();
void clearGlobalObject() { m_globalObject = 0; }
private: private:
JSLazyEventListener(const String& functionName, const String& eventParameterName, const String& code, JSDOMGlobalObject*, Node*, int lineNumber); JSLazyEventListener(const String& functionName, const String& eventParameterName, const String& code, JSDOMGlobalObject*, Node*, int lineNumber);
virtual JSC::JSObject* jsFunction() const; virtual JSC::JSObject* jsFunction() const;
virtual JSDOMGlobalObject* globalObject() const;
virtual bool wasCreatedFromMarkup() const { return true; } virtual bool wasCreatedFromMarkup() const { return true; }
void parseCode() const; void parseCode() const;
mutable JSC::ProtectedPtr<JSC::JSObject> m_jsFunction;
JSC::ProtectedPtr<JSDOMGlobalObject> m_globalObject;
mutable String m_functionName; mutable String m_functionName;
mutable String m_eventParameterName; mutable String m_eventParameterName;
mutable String m_code; mutable String m_code;
......
...@@ -39,7 +39,6 @@ namespace WebCore { ...@@ -39,7 +39,6 @@ namespace WebCore {
#if USE(JSC) #if USE(JSC)
virtual JSC::JSObject* jsFunction() const { return 0; } virtual JSC::JSObject* jsFunction() const { return 0; }
virtual void clearJSFunction() { }
virtual void markJSFunction() { } virtual void markJSFunction() { }
#endif #endif
......
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