Reviewed by Darin.

	<rdar://problem/3977973> pages on ebay leak referenced JavaScript objects -- over time browsing becomes super-slow

	I fixed this by removing all event listeners for a document, it's
	children, and any disconnected nodes that used to be in the
	document at document detach time. Mozilla temporarily disables
	event listeners on such nodes, but re-enables them if you
	re-parant a node into a new document. However, in WebCore, you
	can't re-parent a node into another document, so there is no
	observable change in behavior.

	We have to do this to break the possible reference cycles between
	event listeners and the dom nodes they are attached to (e.g. via
	scope chain, as in this case).

        * khtml/xml/dom_docimpl.cpp:
        (DocumentImpl::detach):
        (DocumentImpl::removeAllEventListenersFromAllNodesx):
        (DocumentImpl::registerDisconnectedNodeWithEventListeners):
        (DocumentImpl::unregisterDisconnectedNodeWithEventListeners):
        (DocumentImpl::removeAllDisconnectedNodeEventListeners):
        * khtml/xml/dom_docimpl.h:
        * khtml/xml/dom_nodeimpl.cpp:
        (NodeImpl::~NodeImpl):
        (NodeImpl::addEventListener):
        (NodeImpl::removeEventListener):
        (NodeImpl::removeAllEventListeners):
        (NodeImpl::removeHTMLEventListener):
        (NodeImpl::insertedIntoDocument):
        (NodeImpl::removedFromDocument):
        * khtml/xml/dom_nodeimpl.h:

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

diff --git a/WebCore/ChangeLog-2005-08-23 b/WebCore/ChangeLog-2005-08-23
index c882c14e33dcfad87843a9fcea96e43009c73f31..824e48472a0f7e466d013a8f73eadb580ed7f6f7 100644 (file)
--- a/WebCore/ChangeLog-2005-08-23
+++ b/WebCore/ChangeLog-2005-08-23
@@ -1,3 +1,38 @@
+2005-02-08  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Darin.
+
+       <rdar://problem/3977973> pages on ebay leak referenced JavaScript objects -- over time browsing becomes super-slow
+
+       I fixed this by removing all event listeners for a document, it's
+       children, and any disconnected nodes that used to be in the
+       document at document detach time. Mozilla temporarily disables
+       event listeners on such nodes, but re-enables them if you
+       re-parant a node into a new document. However, in WebCore, you
+       can't re-parent a node into another document, so there is no
+       observable change in behavior.
+
+       We have to do this to break the possible reference cycles between
+       event listeners and the dom nodes they are attached to (e.g. via
+       scope chain, as in this case).
+
+        * khtml/xml/dom_docimpl.cpp:
+        (DocumentImpl::detach):
+        (DocumentImpl::removeAllEventListenersFromAllNodesx):
+        (DocumentImpl::registerDisconnectedNodeWithEventListeners):
+        (DocumentImpl::unregisterDisconnectedNodeWithEventListeners):
+        (DocumentImpl::removeAllDisconnectedNodeEventListeners):
+        * khtml/xml/dom_docimpl.h:
+        * khtml/xml/dom_nodeimpl.cpp:
+        (NodeImpl::~NodeImpl):
+        (NodeImpl::addEventListener):
+        (NodeImpl::removeEventListener):
+        (NodeImpl::removeAllEventListeners):
+        (NodeImpl::removeHTMLEventListener):
+        (NodeImpl::insertedIntoDocument):
+        (NodeImpl::removedFromDocument):
+        * khtml/xml/dom_nodeimpl.h:
+
 2005-02-09  Chris Blumenberg  <cblu@apple.com>
 
        Fixed: <rdar://problem/3999213> Sometimes 2 Windows Media Player plugin instances are loaded

diff --git a/WebCore/khtml/xml/dom_docimpl.cpp b/WebCore/khtml/xml/dom_docimpl.cpp
index c517343a347a5d04e878a87b5860eda8d52055ea..afee8f0f29b55980f3cad31ccfa3154d3987a863 100644 (file)
--- a/WebCore/khtml/xml/dom_docimpl.cpp
+++ b/WebCore/khtml/xml/dom_docimpl.cpp
@@ -1191,6 +1191,8 @@ void DocumentImpl::detach()
     m_imageLoadEventDispatchSoonList.clear();
     m_imageLoadEventDispatchingList.clear();
 
+    removeAllEventListenersFromAllNodes();
+
     NodeBaseImpl::detach();
 
     if ( render )
@@ -1206,6 +1208,34 @@ void DocumentImpl::detach()
     }
 }
 
+void DocumentImpl::removeAllEventListenersFromAllNodes()
+{
+    m_windowEventListeners.clear();
+    removeAllDisconnectedNodeEventListeners();
+    for (NodeImpl *n = this; n; n = n->traverseNextNode()) {
+        n->removeAllEventListeners();
+    }
+}
+
+void DocumentImpl::registerDisconnectedNodeWithEventListeners(NodeImpl *node)
+{
+    m_disconnectedNodesWithEventListeners.insert(node, node);
+}
+
+void DocumentImpl::unregisterDisconnectedNodeWithEventListeners(NodeImpl *node)
+{
+    m_disconnectedNodesWithEventListeners.remove(node);
+}
+
+void DocumentImpl::removeAllDisconnectedNodeEventListeners()
+{
+    for (QPtrDictIterator<NodeImpl> iter(m_disconnectedNodesWithEventListeners);
+         iter.current();
+         ++iter) {
+        iter.current()->removeAllEventListeners();
+    }
+}
+
 #if APPLE_CHANGES
 KWQAccObjectCache* DocumentImpl::getAccObjectCache()
 {

diff --git a/WebCore/khtml/xml/dom_docimpl.h b/WebCore/khtml/xml/dom_docimpl.h
index b4d159e3390e57849f58994805a8bb38474f0b74..c54bcf253b061624042ab69fd37f8116537f812a 100644 (file)
--- a/WebCore/khtml/xml/dom_docimpl.h
+++ b/WebCore/khtml/xml/dom_docimpl.h
@@ -702,6 +702,8 @@ protected:
 
     DOMString m_policyBaseURL;
 
+    QPtrDict<NodeImpl> m_disconnectedNodesWithEventListeners;
+
 #if APPLE_CHANGES
 public:
     KWQSignal m_finishedParsing;
@@ -733,7 +735,14 @@ public:
     const QValueList<khtml::DashboardRegionValue> & dashboardRegions() const;
     void setDashboardRegions (const QValueList<khtml::DashboardRegionValue>& regions);
     
+    void removeAllEventListenersFromAllNodes();
+
+    void registerDisconnectedNodeWithEventListeners(NodeImpl *node);
+    void unregisterDisconnectedNodeWithEventListeners(NodeImpl *node);
+
 private:
+    void removeAllDisconnectedNodeEventListeners();
+
     JSEditor *jsEditor();
 
     JSEditor *m_jsEditor;

diff --git a/WebCore/khtml/xml/dom_nodeimpl.cpp b/WebCore/khtml/xml/dom_nodeimpl.cpp
index 8c5e24667c4c5955c5a6e128138ea2637ebc831b..aa0a049deeade880dd66efe1b6454a794ec29493 100644 (file)
--- a/WebCore/khtml/xml/dom_nodeimpl.cpp
+++ b/WebCore/khtml/xml/dom_nodeimpl.cpp
@@ -110,6 +110,8 @@ NodeImpl::~NodeImpl()
 {
     if (m_render)
         detach();
+    if (m_regdListeners && !m_regdListeners->isEmpty() && getDocument() && !inDocument())
+        getDocument()->unregisterDisconnectedNodeWithEventListeners(this);
     delete m_regdListeners;
     delete m_nodeLists;
     if (document)
@@ -340,6 +342,9 @@ unsigned long NodeImpl::nodeIndex() const
 
 void NodeImpl::addEventListener(int id, EventListener *listener, const bool useCapture)
 {
+    if (getDocument() && !getDocument()->attached())
+        return;
+
     switch (id) {
        case EventImpl::DOMSUBTREEMODIFIED_EVENT:
            getDocument()->addListenerType(DocumentImpl::DOMSUBTREEMODIFIED_LISTENER);
@@ -378,6 +383,10 @@ void NodeImpl::addEventListener(int id, EventListener *listener, const bool useC
     // spec says that "duplicate instances are discarded" in this case.
     removeEventListener(id,listener,useCapture);
 
+    // adding the first one
+    if (m_regdListeners->isEmpty() && getDocument() && !inDocument())
+        getDocument()->registerDisconnectedNodeWithEventListeners(this);
+        
     m_regdListeners->append(rl);
     listener->deref();
 }
@@ -393,10 +402,19 @@ void NodeImpl::removeEventListener(int id, EventListener *listener, bool useCapt
     for (; it.current(); ++it)
         if (*(it.current()) == rl) {
             m_regdListeners->removeRef(it.current());
+            // removed last
+            if (m_regdListeners->isEmpty() && getDocument() && !inDocument())
+                getDocument()->unregisterDisconnectedNodeWithEventListeners(this);
             return;
         }
 }
 
+void NodeImpl::removeAllEventListeners()
+{
+    delete m_regdListeners;
+    m_regdListeners = 0;
+}
+
 void NodeImpl::removeHTMLEventListener(int id)
 {
     if (!m_regdListeners) // nothing to remove
@@ -407,6 +425,9 @@ void NodeImpl::removeHTMLEventListener(int id)
         if (it.current()->id == id &&
             it.current()->listener->eventListenerType() == "_khtml_HTMLEventListener") {
             m_regdListeners->removeRef(it.current());
+            // removed last
+            if (m_regdListeners->isEmpty() && getDocument() && !inDocument())
+                getDocument()->unregisterDisconnectedNodeWithEventListeners(this);
             return;
         }
 }
@@ -1094,11 +1115,17 @@ void NodeImpl::restoreState(QStringList &/*states*/)
 
 void NodeImpl::insertedIntoDocument()
 {
+    if (m_regdListeners && !m_regdListeners->isEmpty() && getDocument())
+        getDocument()->unregisterDisconnectedNodeWithEventListeners(this);
+
     setInDocument(true);
 }
 
 void NodeImpl::removedFromDocument()
 {
+    if (m_regdListeners && !m_regdListeners->isEmpty() && getDocument())
+        getDocument()->registerDisconnectedNodeWithEventListeners(this);
+
     setInDocument(false);
 }
 

diff --git a/WebCore/khtml/xml/dom_nodeimpl.h b/WebCore/khtml/xml/dom_nodeimpl.h
index 6547d18ac898b25a54f43f9fc2e4bfc27601267c..c241a201f2648e545c84ab6acf8f7812edf26fe5 100644 (file)
--- a/WebCore/khtml/xml/dom_nodeimpl.h
+++ b/WebCore/khtml/xml/dom_nodeimpl.h
@@ -277,6 +277,7 @@ public:
     void removeHTMLEventListener(int id);
     void setHTMLEventListener(int id, EventListener *listener);
     EventListener *getHTMLEventListener(int id);
+    void removeAllEventListeners();
 
     bool dispatchEvent(EventImpl *evt, int &exceptioncode, bool tempEvent = false);
     bool dispatchGenericEvent( EventImpl *evt, int &exceptioncode);