Changeset 8557 in webkit

Timestamp:

Feb 10, 2005, 12:40:56 AM (20 years ago)

Author:

mjs

Message:

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:

Location:

trunk/WebCore

Files:

• ChangeLog-2005-08-23 (modified) (1 diff)
• khtml/xml/dom_docimpl.cpp (modified) (2 diffs)
• khtml/xml/dom_docimpl.h (modified) (2 diffs)
• khtml/xml/dom_nodeimpl.cpp (modified) (7 diffs)
• khtml/xml/dom_nodeimpl.h (modified) (1 diff)

trunk/WebCore/ChangeLog-2005-08-23

@@ +1 @@
+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>
 

trunk/WebCore/khtml/xml/dom_docimpl.cpp

@@ -1192 +1192 @@
     m_imageLoadEventDispatchingList.clear();
 
+    removeAllEventListenersFromAllNodes();
+
     NodeBaseImpl::detach();
 
@@ -1204 +1206 @@
         delete m_renderArena;
         m_renderArena = 0;
+    }
+}
+
+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();
     }
 }

trunk/WebCore/khtml/xml/dom_docimpl.h

@@ -703 +703 @@
     DOMString m_policyBaseURL;
 
+    QPtrDict<NodeImpl> m_disconnectedNodesWithEventListeners;
+
 #if APPLE_CHANGES
 public:
@@ -734 +736 @@
     void setDashboardRegions (const QValueList<khtml::DashboardRegionValue>& regions);
     
+    void removeAllEventListenersFromAllNodes();
+
+    void registerDisconnectedNodeWithEventListeners(NodeImpl *node);
+    void unregisterDisconnectedNodeWithEventListeners(NodeImpl *node);
+
 private:
+    void removeAllDisconnectedNodeEventListeners();
+
     JSEditor *jsEditor();
 

trunk/WebCore/khtml/xml/dom_nodeimpl.cpp

@@ -111 +111 @@
     if (m_render)
         detach();
+    if (m_regdListeners && !m_regdListeners->isEmpty() && getDocument() && !inDocument())
+        getDocument()->unregisterDisconnectedNodeWithEventListeners(this);
     delete m_regdListeners;
     delete m_nodeLists;
@@ -341 +343 @@
 void NodeImpl::addEventListener(int id, EventListener *listener, const bool useCapture)
 {
+    if (getDocument() && !getDocument()->attached())
+        return;
+
     switch (id) {
         case EventImpl::DOMSUBTREEMODIFIED_EVENT:
@@ -379 +384 @@
     removeEventListener(id,listener,useCapture);
 
+    // adding the first one
+    if (m_regdListeners->isEmpty() && getDocument() && !inDocument())
+        getDocument()->registerDisconnectedNodeWithEventListeners(this);
+        
     m_regdListeners->append(rl);
     listener->deref();
@@ -394 +403 @@
         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;
 }
 
@@ -408 +426 @@
             it.current()->listener->eventListenerType() == "_khtml_HTMLEventListener") {
             m_regdListeners->removeRef(it.current());
+            // removed last
+            if (m_regdListeners->isEmpty() && getDocument() && !inDocument())
+                getDocument()->unregisterDisconnectedNodeWithEventListeners(this);
             return;
         }
@@ -1095 +1116 @@
 void NodeImpl::insertedIntoDocument()
 {
+    if (m_regdListeners && !m_regdListeners->isEmpty() && getDocument())
+        getDocument()->unregisterDisconnectedNodeWithEventListeners(this);
+
     setInDocument(true);
 }
@@ -1100 +1124 @@
 void NodeImpl::removedFromDocument()
 {
+    if (m_regdListeners && !m_regdListeners->isEmpty() && getDocument())
+        getDocument()->registerDisconnectedNodeWithEventListeners(this);
+
     setInDocument(false);
 }

trunk/WebCore/khtml/xml/dom_nodeimpl.h

@@ -278 +278 @@
     void setHTMLEventListener(int id, EventListener *listener);
     EventListener *getHTMLEventListener(int id);
+    void removeAllEventListeners();
 
     bool dispatchEvent(EventImpl *evt, int &exceptioncode, bool tempEvent = false);