Reviewed by Richard.

	<rdar://problem/4040776> Dashboard (Weather widget) is a memory hog

	Change things around so the event listeners for XMLHttpRequest
	mark their JS listener objects instead of holding a hard
	reference, to avoid an unbreakable reference cycle.

	* khtml/ecma/kjs_events.cpp:
        (JSAbstractEventListener::JSAbstractEventListener):
        (JSAbstractEventListener::~JSAbstractEventListener):
        (JSAbstractEventListener::handleEvent):
        (JSAbstractEventListener::eventListenerType):
        (JSUnprotectedEventListener::JSUnprotectedEventListener):
        (JSUnprotectedEventListener::~JSUnprotectedEventListener):
        (JSUnprotectedEventListener::listenerObj):
        (JSUnprotectedEventListener::windowObj):
        (JSUnprotectedEventListener::mark):
        (JSEventListener::JSEventListener):
        (JSEventListener::~JSEventListener):
        (JSEventListener::listenerObj):
        (JSEventListener::windowObj):
        (JSLazyEventListener::JSLazyEventListener):
        * khtml/ecma/kjs_events.h:
        * khtml/ecma/kjs_html.h:
        * khtml/ecma/kjs_window.cpp:
        (Window::getJSEventListener):
        (Window::getJSUnprotectedEventListener):
        * khtml/ecma/kjs_window.h:
        * khtml/ecma/xmlhttprequest.cpp:
        (KJS::XMLHttpRequest::putValue):
        (KJS::XMLHttpRequest::mark):
        * khtml/ecma/xmlhttprequest.h:
        * khtml/khtml_part.h:

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

diff --git a/WebCore/ChangeLog-2005-08-23 b/WebCore/ChangeLog-2005-08-23
index ff4e2052ea3a24b2850c59a00a0b0755dee931c4..61a4b94a70eea08af81f300f8af5d9a7f4e99dd8 100644 (file)
--- a/WebCore/ChangeLog-2005-08-23
+++ b/WebCore/ChangeLog-2005-08-23
@@ -1,3 +1,40 @@
+2005-03-09  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Richard.
+
+       <rdar://problem/4040776> Dashboard (Weather widget) is a memory hog
+        
+       Change things around so the event listeners for XMLHttpRequest
+       mark their JS listener objects instead of holding a hard
+       reference, to avoid an unbreakable reference cycle.
+
+       * khtml/ecma/kjs_events.cpp:
+        (JSAbstractEventListener::JSAbstractEventListener):
+        (JSAbstractEventListener::~JSAbstractEventListener):
+        (JSAbstractEventListener::handleEvent):
+        (JSAbstractEventListener::eventListenerType):
+        (JSUnprotectedEventListener::JSUnprotectedEventListener):
+        (JSUnprotectedEventListener::~JSUnprotectedEventListener):
+        (JSUnprotectedEventListener::listenerObj):
+        (JSUnprotectedEventListener::windowObj):
+        (JSUnprotectedEventListener::mark):
+        (JSEventListener::JSEventListener):
+        (JSEventListener::~JSEventListener):
+        (JSEventListener::listenerObj):
+        (JSEventListener::windowObj):
+        (JSLazyEventListener::JSLazyEventListener):
+        * khtml/ecma/kjs_events.h:
+        * khtml/ecma/kjs_html.h:
+        * khtml/ecma/kjs_window.cpp:
+        (Window::getJSEventListener):
+        (Window::getJSUnprotectedEventListener):
+        * khtml/ecma/kjs_window.h:
+        * khtml/ecma/xmlhttprequest.cpp:
+        (KJS::XMLHttpRequest::putValue):
+        (KJS::XMLHttpRequest::mark):
+        * khtml/ecma/xmlhttprequest.h:
+        * khtml/khtml_part.h:
+
 2005-03-06  Maciej Stachowiak  <mjs@apple.com>
 
         Reviewed by Darin.

diff --git a/WebCore/khtml/ecma/kjs_events.cpp b/WebCore/khtml/ecma/kjs_events.cpp
index c626b87baa10e5a82c3b22783262c283b9ce9459..8e7cdd92dfef35b19dc6ced0931edf6dbca1b66e 100644 (file)
--- a/WebCore/khtml/ecma/kjs_events.cpp
+++ b/WebCore/khtml/ecma/kjs_events.cpp
@@ -40,31 +40,24 @@ using DOM::NodeImpl;
 
 // -------------------------------------------------------------------------
 
-JSEventListener::JSEventListener(Object _listener, const Object &_win, bool _html)
+JSAbstractEventListener::JSAbstractEventListener(bool _html)
+  : html(_html)
 {
-    listener = _listener;
-    //fprintf(stderr,"JSEventListener::JSEventListener this=%p listener=%p\n",this,listener.imp());
-    html = _html;
-    win = _win;
-    if (_listener.imp()) {
-      static_cast<Window*>(win.imp())->jsEventListeners.insert(_listener.imp(), this);
-    }
 }
 
-JSEventListener::~JSEventListener()
+JSAbstractEventListener::~JSAbstractEventListener()
 {
-    if (listener.imp()) {
-      static_cast<Window*>(win.imp())->jsEventListeners.remove(listener.imp());
-    }
-    //fprintf(stderr,"JSEventListener::~JSEventListener this=%p listener=%p\n",this,listener.imp());
 }
 
-void JSEventListener::handleEvent(DOM::Event &evt, bool isWindowEvent)
+void JSAbstractEventListener::handleEvent(DOM::Event &evt, bool isWindowEvent)
 {
 #ifdef KJS_DEBUGGER
   if (KJSDebugWin::instance() && KJSDebugWin::instance()->inSession())
     return;
 #endif
+  Object listener = listenerObj();
+  Object win = windowObj();
+
   KHTMLPart *part = static_cast<Window*>(win.imp())->part();
   KJSProxy *proxy = 0;
   if (part)
@@ -129,7 +122,7 @@ void JSEventListener::handleEvent(DOM::Event &evt, bool isWindowEvent)
   }
 }
 
-DOM::DOMString JSEventListener::eventListenerType()
+DOM::DOMString JSAbstractEventListener::eventListenerType()
 {
     if (html)
        return "_khtml_HTMLEventListener";
@@ -137,26 +130,89 @@ DOM::DOMString JSEventListener::eventListenerType()
        return "_khtml_JSEventListener";
 }
 
+// -------------------------------------------------------------------------
+
+JSUnprotectedEventListener::JSUnprotectedEventListener(Object _listener, const Object &_win, bool _html)
+  : JSAbstractEventListener(_html)
+  , listener(_listener)
+  , win(_win)
+{
+    if (_listener.imp()) {
+      static_cast<Window*>(win.imp())->jsUnprotectedEventListeners.insert(_listener.imp(), this);
+    }
+}
+
+JSUnprotectedEventListener::~JSUnprotectedEventListener()
+{
+    if (listener.imp()) {
+      static_cast<Window*>(win.imp())->jsUnprotectedEventListeners.remove(listener.imp());
+    }
+}
+
+Object JSUnprotectedEventListener::listenerObj() const
+{ 
+    return listener; 
+}
+
+Object JSUnprotectedEventListener::windowObj() const
+{
+    return win;
+}
+
+void JSUnprotectedEventListener::mark()
+{
+  ObjectImp *listenerImp = listener.imp();
+  if (listenerImp && !listenerImp->marked())
+    listenerImp->mark();
+}
+
+// -------------------------------------------------------------------------
+
+JSEventListener::JSEventListener(Object _listener, const Object &_win, bool _html)
+  : JSAbstractEventListener(_html)
+  , listener(_listener)
+  , win(_win)
+{
+    //fprintf(stderr,"JSEventListener::JSEventListener this=%p listener=%p\n",this,listener.imp());
+    if (_listener.imp()) {
+      static_cast<Window*>(win.imp())->jsEventListeners.insert(_listener.imp(), this);
+    }
+}
+
+JSEventListener::~JSEventListener()
+{
+    if (listener.imp()) {
+      static_cast<Window*>(win.imp())->jsEventListeners.remove(listener.imp());
+    }
+    //fprintf(stderr,"JSEventListener::~JSEventListener this=%p listener=%p\n",this,listener.imp());
+}
 
 Object JSEventListener::listenerObj() const
 { 
-  return listener; 
+    return listener; 
 }
 
+Object JSEventListener::windowObj() const
+{
+    return win;
+}
+
+// -------------------------------------------------------------------------
+
 JSLazyEventListener::JSLazyEventListener(QString _code, const Object &_win, NodeImpl *_originalNode, int lineno)
   : JSEventListener(Object(), _win, true),
     code(_code),
     parsed(false)
 {
-        lineNumber = lineno;
-
-        // We don't retain the original node, because we assume it
-        // will stay alive as long as this handler object is around
-        // and we need to avoid a reference cycle. If JS transfers
-        // this handler to another node, parseCode will be called and
-        // then originalNode is no longer needed.
+    lineNumber = lineno;
 
-        originalNode = _originalNode;
+    // We don't retain the original node, because we assume it
+    // will stay alive as long as this handler object is around
+    // and we need to avoid a reference cycle. If JS transfers
+    // this handler to another node, parseCode will be called and
+    // then originalNode is no longer needed.
+    
+    originalNode = _originalNode;
 }
 
 void JSLazyEventListener::handleEvent(DOM::Event &evt, bool isWindowEvent)

diff --git a/WebCore/khtml/ecma/kjs_events.h b/WebCore/khtml/ecma/kjs_events.h
index e3f65f1334f5f019c2bb5171bce8539e2be438c9..5cbb4446357c074bc9f067a585de4805560a1f58 100644 (file)
--- a/WebCore/khtml/ecma/kjs_events.h
+++ b/WebCore/khtml/ecma/kjs_events.h
@@ -34,17 +34,39 @@ namespace KJS {
   class Window;
   class Clipboard;
     
-  class JSEventListener : public DOM::EventListener {
+  class JSAbstractEventListener : public DOM::EventListener {
   public:
-    JSEventListener(Object _listener, const Object &_win, bool _html = false);
-    virtual ~JSEventListener();
+    JSAbstractEventListener(bool _html = false);
+    virtual ~JSAbstractEventListener();
     virtual void handleEvent(DOM::Event &evt, bool isWindowEvent);
     virtual DOM::DOMString eventListenerType();
-    virtual Object listenerObj() const;
+    virtual Object listenerObj() const = 0;
+    virtual Object windowObj() const = 0;
     ObjectImp *listenerObjImp() const { return listenerObj().imp(); }
   protected:
-    mutable ProtectedObject listener;
     bool html;
+  };
+
+  class JSUnprotectedEventListener : public JSAbstractEventListener {
+  public:
+    JSUnprotectedEventListener(Object _listener, const Object &_win, bool _html = false);
+    virtual ~JSUnprotectedEventListener();
+    virtual Object listenerObj() const;
+    virtual Object windowObj() const;
+    void mark();
+  protected:
+    Object listener;
+    Object win;
+  };
+
+  class JSEventListener : public JSAbstractEventListener {
+  public:
+    JSEventListener(Object _listener, const Object &_win, bool _html = false);
+    virtual ~JSEventListener();
+    virtual Object listenerObj() const;
+    virtual Object windowObj() const;
+  protected:
+    mutable ProtectedObject listener;
     ProtectedObject win;
   };
 

diff --git a/WebCore/khtml/ecma/kjs_html.h b/WebCore/khtml/ecma/kjs_html.h
index 0fa7c70e0cfc69a4f77baba51879cb89b7af4ef6..562fe83dabaf17541b266c98ee323b2f998cd39b 100644 (file)
--- a/WebCore/khtml/ecma/kjs_html.h
+++ b/WebCore/khtml/ecma/kjs_html.h
@@ -39,6 +39,8 @@ class HTMLElement;
 
 namespace KJS {
 
+  class JSAbstractEventListener;
+
   class HTMLDocument : public DOMDocument {
   public:
     HTMLDocument(ExecState *exec, const DOM::HTMLDocument &d) : DOMDocument(exec, d) { }
@@ -241,7 +243,7 @@ namespace KJS {
     UString src;
     QGuardedPtr<DOM::DocumentImpl> doc;
     khtml::CachedImage* img;
-    JSEventListener *onLoadListener;
+    JSAbstractEventListener *onLoadListener;
     bool widthSet;
     bool heightSet;
     int width;

diff --git a/WebCore/khtml/ecma/kjs_window.cpp b/WebCore/khtml/ecma/kjs_window.cpp
index df1e9d885dc6c4addb97c98f97b091395b45cff4..1b6380ad99f81a641050ce08630827d407e61e29 100644 (file)
--- a/WebCore/khtml/ecma/kjs_window.cpp
+++ b/WebCore/khtml/ecma/kjs_window.cpp
@@ -1260,7 +1260,6 @@ Value Window::getListener(ExecState *exec, int eventId) const
     return Null();
 }
 
-
 JSEventListener *Window::getJSEventListener(const Value& val, bool html)
 {
   // This function is so hot that it's worth coding it directly with imps.
@@ -1276,6 +1275,22 @@ JSEventListener *Window::getJSEventListener(const Value& val, bool html)
   return new JSEventListener(Object(listenerObject), Object(this), html);
 }
 
+JSUnprotectedEventListener *Window::getJSUnprotectedEventListener(const Value& val, bool html)
+{
+  // This function is so hot that it's worth coding it directly with imps.
+  if (val.type() != ObjectType)
+    return 0;
+  ObjectImp *listenerObject = static_cast<ObjectImp *>(val.imp());
+
+  JSUnprotectedEventListener *existingListener = jsUnprotectedEventListeners[listenerObject];
+  if (existingListener)
+    return existingListener;
+
+  // Note that the JSUnprotectedEventListener constructor adds it to
+  // our jsUnprotectedEventListeners list
+  return new JSUnprotectedEventListener(Object(listenerObject), Object(this), html);
+}
+
 JSLazyEventListener *Window::getJSLazyEventListener(const QString& code, DOM::NodeImpl *node, int lineNumber)
 {
   return new JSLazyEventListener(code, Object(this), node, lineNumber);

diff --git a/WebCore/khtml/ecma/kjs_window.h b/WebCore/khtml/ecma/kjs_window.h
index bdb6dbbc142dd21c74d49c0446af956b334badd0..848ce57c5d59c0d1e9d3a1b862d604f9d5f843da 100644 (file)
--- a/WebCore/khtml/ecma/kjs_window.h
+++ b/WebCore/khtml/ecma/kjs_window.h
@@ -43,6 +43,7 @@ namespace KJS {
   class History;
   class FrameArray;
   class JSEventListener;
+  class JSUnprotectedEventListener;
   class JSLazyEventListener;
 
   class Screen : public ObjectImp {
@@ -115,6 +116,7 @@ namespace KJS {
     BarInfo *statusbar(ExecState *exec) const;
     BarInfo *toolbar(ExecState *exec) const;
     JSEventListener *getJSEventListener(const Value &val, bool html = false);
+    JSUnprotectedEventListener *getJSUnprotectedEventListener(const Value &val, bool html = false);
     JSLazyEventListener *getJSLazyEventListener(const QString &code, DOM::NodeImpl *node, int lineno = 0);
     void clear( ExecState *exec );
     virtual UString toString(ExecState *exec) const;
@@ -123,6 +125,7 @@ namespace KJS {
     void setCurrentEvent( DOM::Event *evt );
 
     QPtrDict<JSEventListener> jsEventListeners;
+    QPtrDict<JSUnprotectedEventListener> jsUnprotectedEventListeners;
     virtual const ClassInfo* classInfo() const { return &info; }
     static const ClassInfo info;
     enum { Closed, Crypto, DefaultStatus, Status, Document, Node, EventCtor, Range,

diff --git a/WebCore/khtml/ecma/xmlhttprequest.cpp b/WebCore/khtml/ecma/xmlhttprequest.cpp
index 52ca14f1809c489e0185bf05f4ff5584b15d264b..42cbad7718662ebe9ad6984fefddac2ed802aa6b 100644 (file)
--- a/WebCore/khtml/ecma/xmlhttprequest.cpp
+++ b/WebCore/khtml/ecma/xmlhttprequest.cpp
@@ -204,11 +204,11 @@ void XMLHttpRequest::putValue(ExecState *exec, int token, const Value& value, in
 {
   switch(token) {
   case Onreadystatechange:
-    onReadyStateChangeListener = Window::retrieveActive(exec)->getJSEventListener(value, true);
+    onReadyStateChangeListener = Window::retrieveActive(exec)->getJSUnprotectedEventListener(value, true);
     if (onReadyStateChangeListener) onReadyStateChangeListener->ref();
     break;
   case Onload:
-    onLoadListener = Window::retrieveActive(exec)->getJSEventListener(value, true);
+    onLoadListener = Window::retrieveActive(exec)->getJSUnprotectedEventListener(value, true);
     if (onLoadListener) onLoadListener->ref();
     break;
   default:
@@ -216,6 +216,18 @@ void XMLHttpRequest::putValue(ExecState *exec, int token, const Value& value, in
   }
 }
 
+void XMLHttpRequest::mark()
+{
+  DOMObject::mark();
+
+  if (onReadyStateChangeListener)
+    onReadyStateChangeListener->mark();
+
+  if (onLoadListener)
+    onLoadListener->mark();
+}
+
+
 XMLHttpRequest::XMLHttpRequest(ExecState *exec, const DOM::Document &d)
   : DOMObject(XMLHttpRequestProto::self(exec)),
     qObject(new XMLHttpRequestQObject(this)),

diff --git a/WebCore/khtml/ecma/xmlhttprequest.h b/WebCore/khtml/ecma/xmlhttprequest.h
index 78fedeba053bf080506bcf4d87c76895c4ac54ba..afb05fef90f3dff573e1c2765c8adbcd544f5866 100644 (file)
--- a/WebCore/khtml/ecma/xmlhttprequest.h
+++ b/WebCore/khtml/ecma/xmlhttprequest.h
@@ -28,7 +28,7 @@
 
 namespace KJS {
 
-  class JSEventListener;
+  class JSUnprotectedEventListener;
   class XMLHttpRequestQObject;
 
   // these exact numeric values are important because JS expects them
@@ -58,6 +58,8 @@ namespace KJS {
     virtual void tryPut(ExecState *exec, const Identifier &propertyName, const Value& value, int attr = None);
     void putValue(ExecState *exec, int token, const Value& value, int /*attr*/);
     virtual bool toBoolean(ExecState *) const { return true; }
+    virtual void mark();
+
     virtual const ClassInfo* classInfo() const { return &info; }
     static const ClassInfo info;
     enum { Onload, Onreadystatechange, ReadyState, ResponseText, ResponseXML, Status,
@@ -105,8 +107,8 @@ namespace KJS {
     KIO::TransferJob * job;
 
     XMLHttpRequestState state;
-    JSEventListener *onReadyStateChangeListener;
-    JSEventListener *onLoadListener;
+    JSUnprotectedEventListener *onReadyStateChangeListener;
+    JSUnprotectedEventListener *onLoadListener;
 
     khtml::Decoder *decoder;
     QString encoding;

diff --git a/WebCore/khtml/khtml_part.h b/WebCore/khtml/khtml_part.h
index d2cabbf83c285b1fa47220eea65eb3b8b8e8e065..555ed24de04f69198d948dade546906da9881845 100644 (file)
--- a/WebCore/khtml/khtml_part.h
+++ b/WebCore/khtml/khtml_part.h
@@ -90,7 +90,6 @@ namespace khtml
 
 namespace KJS {
     class DOMDocument;
-    class JSEventListener;
     class Selection;
     class SelectionFunc;
     class Window;
@@ -160,7 +159,6 @@ class KHTMLPart : public KParts::ReadOnlyPart
   friend class KJS::SelectionFunc;
   friend class KJS::Window;
   friend class KJS::WindowFunc;
-  friend class KJS::JSEventListener;
   friend class KJS::DOMDocument;
   friend class KJSProxy;
   friend class KHTMLPartBrowserExtension;