WebCore:
authorweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Mar 2008 01:42:00 +0000 (01:42 +0000)
committerweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Mar 2008 01:42:00 +0000 (01:42 +0000)
2008-03-18  Sam Weinig  <sam@webkit.org>

        Reviewed by Anders Carlsson.

        Fix for http://bugs.webkit.org/show_bug.cgi?id=17057
        REGRESSION: Frequent random crashes in WebCore::JSNodeList::indexGetter
        <rdar://problem/5725058>

        Tests: fast/dom/NodeList/5725058-crash-scenario-1.html
               fast/dom/NodeList/5725058-crash-scenario-2.html
               fast/dom/NodeList/5725058-crash-scenario-3.html

        * dom/ChildNodeList.cpp:
        (WebCore::ChildNodeList::ChildNodeList):
        * dom/ChildNodeList.h:
        Remove rootNodeChildrenChanged() method and fix the constructor to not
        pass in a needsNotifications argument to DynamicNodeList, as it no longer
        takes one.

        * dom/ClassNodeList.cpp:
        (WebCore::ClassNodeList::ClassNodeList):
        Don't pass the needsNotifications argument to DynamicNodeList.

        * dom/ContainerNode.cpp:
        (WebCore::ContainerNode::childrenChanged):
        Rename call to hasNodeLists() to hasNodeListCaches().

        * dom/Document.cpp:
        (WebCore::Document::Document):
        (WebCore::Document::~Document): Zero out the m_document variable to signify
        to destructors down the destruction chain that this is a Document type node
        being destructed, and thus, accessing document() is prohibited.
        * dom/Document.h:
        (WebCore::Document::addNodeListCache): Renamed from addNodeList.
        (WebCore::Document::removeNodeListCache): Renamed from removeNodeList, adds assertion.
        (WebCore::Document::hasNodeListCaches): Renamed from hasNodeListCaches.
        Rename m_numNodeLists to m_numNodeListCaches.

        * dom/DynamicNodeList.cpp:
        (WebCore::DynamicNodeList::DynamicNodeList):
        (WebCore::DynamicNodeList::~DynamicNodeList):
        (WebCore::DynamicNodeList::invalidateCache):
        (WebCore::DynamicNodeList::Caches::Caches):
        * dom/DynamicNodeList.h:
        (WebCore::DynamicNodeList::hasOwnCaches):
        Remove the needsNotifications concept from DynamicNodeList, instead, manually
        invalidate the cache for lists that own their own cache.

        * dom/NameNodeList.cpp:
        (WebCore::NameNodeList::NameNodeList):
        * dom/NameNodeList.h:
        Remove rootNodeAttributeChanged() method and fix the constructor to not
        pass in a needsNotifications argument to DynamicNodeList, as it no longer
        takes one.

        * dom/Node.cpp:
        (WebCore::Node::~Node): Decrement the document's nodeListCache count
        if we had a NodeListsNodeData cache and this is not the Document being
        destructor, as tagged by a null m_document.
        (WebCore::Node::childNodes): Increment the document's nodeListCache count
        if we need create the NodeListsNodeData.
        (WebCore::Node::registerDynamicNodeList): Increment the document's nodeListCache count
        if we need create the NodeListsNodeData.  Change to invalidate all the caches, instead
        of just the ChildNodeList, if document has had no NodeListCaches.
        (WebCore::Node::unregisterDynamicNodeList): Change to remove the cache from the m_listsWithCaches
        set if it is owned by the NodeList and clear the m_nodeLists if it is empty.
        (WebCore::Node::notifyLocalNodeListsAttributeChanged): Move logic to
        NodeListsNodeData::invalidateAttributeCaches and clear the cache pointer if it is empty.
        (WebCore::Node::notifyLocalNodeListsChildrenChanged): Move logic to
        NodeListsNodeData::invalidateCaches and clear the cache pointer if it is empty.
        (WebCore::Node::notifyNodeListsChildrenChanged): Cleanup.
        (WebCore::Node::getElementsByName): Increment the document's nodeListCache count
        if we need create the NodeListsNodeData.
        (WebCore::Node::getElementsByClassName): Increment the document's nodeListCache count
        if we need create the NodeListsNodeData.

        (WebCore::NodeListsNodeData::invalidateCaches): Added.
        (WebCore::NodeListsNodeData::invalidateAttributeCaches): Added.
        (WebCore::NodeListsNodeData::isEmpty): Added.

        * dom/TagNodeList.cpp:
        (WebCore::TagNodeList::TagNodeList):
        Don't pass the needsNotifications argument to DynamicNodeList.

LayoutTests:

2008-03-18  Sam Weinig  <sam@webkit.org>

        Reviewed by Anders Carlsson.

        Tests for http://bugs.webkit.org/show_bug.cgi?id=17057
        REGRESSION: Frequent random crashes in WebCore::JSNodeList::indexGetter
        <rdar://problem/5725058>

        * fast/dom/NodeList/5725058-crash-scenario-1-expected.txt: Added.
        * fast/dom/NodeList/5725058-crash-scenario-1.html: Added.
        * fast/dom/NodeList/5725058-crash-scenario-2-expected.txt: Added.
        * fast/dom/NodeList/5725058-crash-scenario-2.html: Added.
        * fast/dom/NodeList/5725058-crash-scenario-3-expected.txt: Added.
        * fast/dom/NodeList/5725058-crash-scenario-3.html: Added.

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

20 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/NodeList/5725058-crash-scenario-1-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/NodeList/5725058-crash-scenario-1.html [new file with mode: 0644]
LayoutTests/fast/dom/NodeList/5725058-crash-scenario-2-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/NodeList/5725058-crash-scenario-2.html [new file with mode: 0644]
LayoutTests/fast/dom/NodeList/5725058-crash-scenario-3-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/NodeList/5725058-crash-scenario-3.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/dom/ChildNodeList.cpp
WebCore/dom/ChildNodeList.h
WebCore/dom/ClassNodeList.cpp
WebCore/dom/ContainerNode.cpp
WebCore/dom/Document.cpp
WebCore/dom/Document.h
WebCore/dom/DynamicNodeList.cpp
WebCore/dom/DynamicNodeList.h
WebCore/dom/NameNodeList.cpp
WebCore/dom/NameNodeList.h
WebCore/dom/Node.cpp
WebCore/dom/TagNodeList.cpp

index 33df5d5..a8fef2f 100644 (file)
@@ -1,5 +1,20 @@
 2008-03-18  Sam Weinig  <sam@webkit.org>
 
+        Reviewed by Anders Carlsson.
+
+        Tests for http://bugs.webkit.org/show_bug.cgi?id=17057
+        REGRESSION: Frequent random crashes in WebCore::JSNodeList::indexGetter
+        <rdar://problem/5725058>
+
+        * fast/dom/NodeList/5725058-crash-scenario-1-expected.txt: Added.
+        * fast/dom/NodeList/5725058-crash-scenario-1.html: Added.
+        * fast/dom/NodeList/5725058-crash-scenario-2-expected.txt: Added.
+        * fast/dom/NodeList/5725058-crash-scenario-2.html: Added.
+        * fast/dom/NodeList/5725058-crash-scenario-3-expected.txt: Added.
+        * fast/dom/NodeList/5725058-crash-scenario-3.html: Added.
+
+2008-03-18  Sam Weinig  <sam@webkit.org>
+
         Reviewed by Darin Adler.
 
         Make domListEnumeration.html test not depend on the order of property enumeration
diff --git a/LayoutTests/fast/dom/NodeList/5725058-crash-scenario-1-expected.txt b/LayoutTests/fast/dom/NodeList/5725058-crash-scenario-1-expected.txt
new file mode 100644 (file)
index 0000000..c5b8ca4
--- /dev/null
@@ -0,0 +1,3 @@
+Test for (rdar://problem/5725058). If you see this text, then all is well and no crash has occurred.
+
+
diff --git a/LayoutTests/fast/dom/NodeList/5725058-crash-scenario-1.html b/LayoutTests/fast/dom/NodeList/5725058-crash-scenario-1.html
new file mode 100644 (file)
index 0000000..a8a7eb1
--- /dev/null
@@ -0,0 +1,25 @@
+<p>Test for (rdar://problem/5725058). If you see this text, then all is well and no crash has occurred.</p>
+
+<span>
+    <div name='test'></div>
+</span>
+<script type='text/javascript'>
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+
+    function triggerGarbageCollection()
+    {
+        if (window.GCController) {
+            GCController.collect();
+            return;
+        }
+        for (var i = 0; i < 10000; ++i)
+            var garbage = { };
+    }
+
+    document.getElementsByName('test')[0];
+    triggerGarbageCollection();
+    document.getElementsByTagName('span')[0].innerHTML = '';
+    triggerGarbageCollection();
+    document.getElementsByName('test')[0];
+</script>
diff --git a/LayoutTests/fast/dom/NodeList/5725058-crash-scenario-2-expected.txt b/LayoutTests/fast/dom/NodeList/5725058-crash-scenario-2-expected.txt
new file mode 100644 (file)
index 0000000..38de29a
--- /dev/null
@@ -0,0 +1,3 @@
+Test for (rdar://problem/5725058). If you see this text, then all is well and no crash has occurred.
+
+paragraph b
diff --git a/LayoutTests/fast/dom/NodeList/5725058-crash-scenario-2.html b/LayoutTests/fast/dom/NodeList/5725058-crash-scenario-2.html
new file mode 100644 (file)
index 0000000..ef40e92
--- /dev/null
@@ -0,0 +1,42 @@
+<p>Test for (rdar://problem/5725058). If you see this text, then all is well and no crash has occurred.</p>
+
+<p id="a">paragraph a</p>
+<p id="b">paragraph b</p>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+function triggerGarbageCollection()
+{
+    if (window.GCController) {
+        GCController.collect();
+        return;
+    }
+    for (var i = 0; i < 10000; ++i)
+        var garbage = { };
+}
+
+function setUp()
+{
+    // This only works if in a function. I'm not sure why.
+
+    // Get node and length from paragraph A into the cache.
+    document.getElementById("a").childNodes[0];
+    document.getElementById("a").childNodes.length;
+}
+
+setUp();
+
+// Get back to "zero node lists".
+triggerGarbageCollection();
+
+// Remove the child node of paragraph A. Use innerHTML to avoid getting a reference to the node being removed.
+document.getElementById("a").innerHTML = "";
+
+// Get back to "one node list".
+var childNodesB = document.getElementById("b").childNodes;
+
+// Now try the original list.
+var x = document.getElementById("a").childNodes[0];
+x = document.getElementById("a").childNodes.length;
+</script>
diff --git a/LayoutTests/fast/dom/NodeList/5725058-crash-scenario-3-expected.txt b/LayoutTests/fast/dom/NodeList/5725058-crash-scenario-3-expected.txt
new file mode 100644 (file)
index 0000000..c5b8ca4
--- /dev/null
@@ -0,0 +1,3 @@
+Test for (rdar://problem/5725058). If you see this text, then all is well and no crash has occurred.
+
+
diff --git a/LayoutTests/fast/dom/NodeList/5725058-crash-scenario-3.html b/LayoutTests/fast/dom/NodeList/5725058-crash-scenario-3.html
new file mode 100644 (file)
index 0000000..e3b8d61
--- /dev/null
@@ -0,0 +1,18 @@
+<p>Test for (rdar://problem/5725058). If you see this text, then all is well and no crash has occurred.</p>
+
+<p id="a"><a name="anchor">paragraph a</a></p>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+var list = document.getElementsByName("anchor");
+var x = list.length
+x = list[0];
+
+// Remove the child node of paragraph A. Use innerHTML to avoid getting a reference to the node being removed.
+document.getElementById("a").innerHTML = "";
+
+// Now try the original list.
+x = list.length;
+x = list[0];
+</script>
index 4276416..eb04253 100644 (file)
@@ -1,3 +1,87 @@
+2008-03-18  Sam Weinig  <sam@webkit.org>
+
+        Reviewed by Anders Carlsson.
+
+        Fix for http://bugs.webkit.org/show_bug.cgi?id=17057
+        REGRESSION: Frequent random crashes in WebCore::JSNodeList::indexGetter
+        <rdar://problem/5725058>
+
+        Tests: fast/dom/NodeList/5725058-crash-scenario-1.html
+               fast/dom/NodeList/5725058-crash-scenario-2.html
+               fast/dom/NodeList/5725058-crash-scenario-3.html
+
+        * dom/ChildNodeList.cpp: 
+        (WebCore::ChildNodeList::ChildNodeList):
+        * dom/ChildNodeList.h:
+        Remove rootNodeChildrenChanged() method and fix the constructor to not 
+        pass in a needsNotifications argument to DynamicNodeList, as it no longer
+        takes one.
+
+        * dom/ClassNodeList.cpp:
+        (WebCore::ClassNodeList::ClassNodeList):
+        Don't pass the needsNotifications argument to DynamicNodeList.
+
+        * dom/ContainerNode.cpp:
+        (WebCore::ContainerNode::childrenChanged):
+        Rename call to hasNodeLists() to hasNodeListCaches().
+
+        * dom/Document.cpp:
+        (WebCore::Document::Document):
+        (WebCore::Document::~Document): Zero out the m_document variable to signify
+        to destructors down the destruction chain that this is a Document type node
+        being destructed, and thus, accessing document() is prohibited.
+        * dom/Document.h:
+        (WebCore::Document::addNodeListCache): Renamed from addNodeList.
+        (WebCore::Document::removeNodeListCache): Renamed from removeNodeList, adds assertion.
+        (WebCore::Document::hasNodeListCaches): Renamed from hasNodeListCaches.
+        Rename m_numNodeLists to m_numNodeListCaches.
+
+        * dom/DynamicNodeList.cpp:
+        (WebCore::DynamicNodeList::DynamicNodeList):
+        (WebCore::DynamicNodeList::~DynamicNodeList):
+        (WebCore::DynamicNodeList::invalidateCache):
+        (WebCore::DynamicNodeList::Caches::Caches):
+        * dom/DynamicNodeList.h:
+        (WebCore::DynamicNodeList::hasOwnCaches):
+        Remove the needsNotifications concept from DynamicNodeList, instead, manually
+        invalidate the cache for lists that own their own cache.
+
+        * dom/NameNodeList.cpp:
+        (WebCore::NameNodeList::NameNodeList):
+        * dom/NameNodeList.h:
+        Remove rootNodeAttributeChanged() method and fix the constructor to not 
+        pass in a needsNotifications argument to DynamicNodeList, as it no longer
+        takes one.
+
+        * dom/Node.cpp:
+        (WebCore::Node::~Node): Decrement the document's nodeListCache count
+        if we had a NodeListsNodeData cache and this is not the Document being
+        destructor, as tagged by a null m_document.
+        (WebCore::Node::childNodes): Increment the document's nodeListCache count 
+        if we need create the NodeListsNodeData.
+        (WebCore::Node::registerDynamicNodeList): Increment the document's nodeListCache count 
+        if we need create the NodeListsNodeData.  Change to invalidate all the caches, instead 
+        of just the ChildNodeList, if document has had no NodeListCaches.
+        (WebCore::Node::unregisterDynamicNodeList): Change to remove the cache from the m_listsWithCaches
+        set if it is owned by the NodeList and clear the m_nodeLists if it is empty.
+        (WebCore::Node::notifyLocalNodeListsAttributeChanged): Move logic to 
+        NodeListsNodeData::invalidateAttributeCaches and clear the cache pointer if it is empty.
+        (WebCore::Node::notifyLocalNodeListsChildrenChanged): Move logic to 
+        NodeListsNodeData::invalidateCaches and clear the cache pointer if it is empty.
+        (WebCore::Node::notifyNodeListsChildrenChanged): Cleanup.
+        (WebCore::Node::getElementsByName): Increment the document's nodeListCache count 
+        if we need create the NodeListsNodeData.
+        (WebCore::Node::getElementsByClassName): Increment the document's nodeListCache count 
+        if we need create the NodeListsNodeData.
+
+        (WebCore::NodeListsNodeData::invalidateCaches): Added.
+        (WebCore::NodeListsNodeData::invalidateAttributeCaches): Added.
+        (WebCore::NodeListsNodeData::isEmpty): Added.
+
+        * dom/TagNodeList.cpp:
+        (WebCore::TagNodeList::TagNodeList):
+        Don't pass the needsNotifications argument to DynamicNodeList.
+
 2008-03-18  Matt Lilek  <webkit@mattlilek.com>
 
         Not reviewed, build fix.
index 4befe40..8b5ab0c 100644 (file)
@@ -28,7 +28,7 @@
 namespace WebCore {
 
 ChildNodeList::ChildNodeList(PassRefPtr<Node> rootNode, DynamicNodeList::Caches* info)
-    : DynamicNodeList(rootNode, info, false)
+    : DynamicNodeList(rootNode, info)
 {
 }
 
@@ -103,10 +103,4 @@ bool ChildNodeList::nodeMatches(Node* testNode) const
     return testNode->parentNode() == m_rootNode;
 }
 
-void ChildNodeList::rootNodeChildrenChanged()
-{
-    // For child node lists, the common cache is reset in Node::notifyLocalNodeListsChildrenChanged()
-    ASSERT(!m_ownsCaches);
-}
-
 } // namespace WebCore
index efdc606..7e8b1f5 100644 (file)
@@ -35,8 +35,6 @@ namespace WebCore {
         virtual unsigned length() const;
         virtual Node* item(unsigned index) const;
 
-        virtual void rootNodeChildrenChanged();
-
     protected:
         virtual bool nodeMatches(Node*) const;
     };
index eb1f837..f8d8c91 100644 (file)
@@ -37,7 +37,7 @@
 namespace WebCore {
 
 ClassNodeList::ClassNodeList(PassRefPtr<Node> rootNode, const String& classNames, DynamicNodeList::Caches* caches)
-    : DynamicNodeList(rootNode, caches, true)
+    : DynamicNodeList(rootNode, caches)
 {
     m_classNames.parseClassAttribute(classNames, m_rootNode->document()->inCompatMode());
 }
index 7f68ab7..49b7abf 100644 (file)
@@ -700,7 +700,7 @@ void ContainerNode::childrenChanged(bool changedByParser, Node* beforeChange, No
     Node::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
     if (!changedByParser && childCountDelta)
         document()->nodeChildrenChanged(this, beforeChange, afterChange, childCountDelta);
-    if (document()->hasNodeLists())
+    if (document()->hasNodeListCaches())
         notifyNodeListsChildrenChanged();
 }
 
index 84f5ee1..71d3e3f 100644 (file)
@@ -296,7 +296,7 @@ Document::Document(DOMImplementation* impl, Frame* frame, bool isXHTML)
     , m_isAllowedToLoadLocalResources(false)
     , m_useSecureKeyboardEntryWhenActive(false)
     , m_isXHTML(isXHTML)
-    , m_numNodeLists(0)
+    , m_numNodeListCaches(0)
 #if ENABLE(DATABASE)
     , m_hasOpenDatabases(false)
 #endif
@@ -455,6 +455,8 @@ Document::~Document()
 
     if (m_styleSheets)
         m_styleSheets->documentDestroyed();
+
+    m_document = 0;
 }
 
 void Document::resetLinkColor()
index 4d4b91f..6ca3137 100644 (file)
@@ -84,10 +84,8 @@ namespace WebCore {
     class HTMLMapElement;
     class IntPoint;
     class MouseEventWithHitTestResults;
-    class NameNodeList;
     class NodeFilter;
     class NodeIterator;
-    class NodeList;
     class Page;
     class PlatformMouseEvent;
     class ProcessingInstruction;
@@ -700,9 +698,9 @@ public:
     void setLowBandwidthDisplay(bool lowBandWidth) { m_inLowBandwidthDisplay = lowBandWidth; }
 #endif
     
-    void addNodeList() { m_numNodeLists++; }
-    void removeNodeList() { m_numNodeLists--; }
-    bool hasNodeLists() const { return m_numNodeLists != 0; }
+    void addNodeListCache() { ++m_numNodeListCaches; }
+    void removeNodeListCache() { ASSERT(m_numNodeListCaches > 0); --m_numNodeListCaches; }
+    bool hasNodeListCaches() const { return m_numNodeListCaches; }
 
     void updateFocusAppearanceSoon();
     void cancelFocusAppearanceUpdate();
@@ -959,7 +957,7 @@ private:
 
     bool m_isXHTML;
 
-    unsigned m_numNodeLists;
+    unsigned m_numNodeListCaches;
 
 #if ENABLE(DATABASE)
     RefPtr<DatabaseThread> m_databaseThread;
index 5a515d2..c6ef839 100644 (file)
 
 namespace WebCore {
 
-DynamicNodeList::DynamicNodeList(PassRefPtr<Node> rootNode, bool needsNotifications)
+DynamicNodeList::DynamicNodeList(PassRefPtr<Node> rootNode)
     : m_rootNode(rootNode)
     , m_caches(new Caches)
     , m_ownsCaches(true)
-    , m_needsNotifications(needsNotifications)
 {
     m_rootNode->registerDynamicNodeList(this);
 }    
 
-DynamicNodeList::DynamicNodeList(PassRefPtr<Node> rootNode, DynamicNodeList::Caches* info, bool needsNotifications)
+DynamicNodeList::DynamicNodeList(PassRefPtr<Node> rootNode, DynamicNodeList::Caches* caches)
     : m_rootNode(rootNode)
-    , m_caches(info)
+    , m_caches(caches)
     , m_ownsCaches(false)
-    , m_needsNotifications(needsNotifications)
 {
     m_rootNode->registerDynamicNodeList(this);
+    ++caches->refCount;
 }    
 
 DynamicNodeList::~DynamicNodeList()
@@ -51,6 +50,8 @@ DynamicNodeList::~DynamicNodeList()
     m_rootNode->unregisterDynamicNodeList(this);
     if (m_ownsCaches)
         delete m_caches;
+    else
+        --m_caches->refCount;
 }
 
 unsigned DynamicNodeList::length() const
@@ -157,19 +158,18 @@ Node* DynamicNodeList::itemWithName(const AtomicString& elementId) const
     return 0;
 }
 
-void DynamicNodeList::rootNodeChildrenChanged()
+void DynamicNodeList::invalidateCache()
 {
+    // This should only be called for node lists that own their own caches.
+    ASSERT(m_ownsCaches);
     m_caches->reset();
 }
 
-void DynamicNodeList::rootNodeAttributeChanged()
-{
-}
-
 DynamicNodeList::Caches::Caches()
     : lastItem(0)
     , isLengthCacheValid(false)
     , isItemCacheValid(false)
+    , refCount(0)
 {
 }
 
index 58e50fd..8674ae1 100644 (file)
@@ -45,13 +45,14 @@ namespace WebCore {
             unsigned lastItemOffset;
             bool isLengthCacheValid : 1;
             bool isItemCacheValid : 1;
+            unsigned refCount;
         };
 
-        DynamicNodeList(PassRefPtr<Node> rootNode, bool needsNotifications);
-        DynamicNodeList(PassRefPtr<Node> rootNode, Caches*, bool needsNotifications);
+        DynamicNodeList(PassRefPtr<Node> rootNode);
+        DynamicNodeList(PassRefPtr<Node> rootNode, Caches*);
         virtual ~DynamicNodeList();
 
-        bool needsNotifications() const { return m_needsNotifications; }
+        bool hasOwnCaches() const { return m_ownsCaches; }
 
         // DOM methods & attributes for NodeList
         virtual unsigned length() const;
@@ -59,8 +60,7 @@ namespace WebCore {
         virtual Node* itemWithName(const AtomicString&) const;
 
         // Other methods (not part of DOM)
-        virtual void rootNodeChildrenChanged();
-        virtual void rootNodeAttributeChanged();
+        void invalidateCache();
 
     protected:
         virtual bool nodeMatches(Node*) const = 0;
@@ -68,7 +68,6 @@ namespace WebCore {
         RefPtr<Node> m_rootNode;
         mutable Caches* m_caches;
         bool m_ownsCaches;
-        bool m_needsNotifications;
 
     private:
         Node* itemForwardsFromCurrent(Node* start, unsigned offset, int remainingOffset) const;
index be6e72b..904dcc8 100644 (file)
@@ -32,16 +32,11 @@ namespace WebCore {
 using namespace HTMLNames;
 
 NameNodeList::NameNodeList(PassRefPtr<Node> rootNode, const String& name, DynamicNodeList::Caches* caches)
-    : DynamicNodeList(rootNode, caches, true)
+    : DynamicNodeList(rootNode, caches)
     , m_nodeName(name)
 {
 }
 
-void NameNodeList::rootNodeAttributeChanged()
-{
-    DynamicNodeList::rootNodeChildrenChanged();
-}
-
 bool NameNodeList::nodeMatches(Node* testNode) const
 {
     ASSERT(testNode->isElementNode());
index bb8c7f4..eab548a 100644 (file)
@@ -36,8 +36,6 @@ namespace WebCore {
     public:
         NameNodeList(PassRefPtr<Node> rootNode, const String& name, DynamicNodeList::Caches*);
 
-        virtual void rootNodeAttributeChanged();
-
     private:
         virtual bool nodeMatches(Node*) const;
 
index ba4cd14..5f4c9d1 100644 (file)
@@ -57,22 +57,29 @@ namespace WebCore {
 
 using namespace HTMLNames;
 
-typedef HashSet<DynamicNodeList*> NodeListSet;
 struct NodeListsNodeData {
-    NodeListSet m_listsToNotify;
+    typedef HashSet<DynamicNodeList*> NodeListSet;
+    NodeListSet m_listsWithCaches;
+
     DynamicNodeList::Caches m_childNodeListCaches;
-    
+
     typedef HashMap<String, DynamicNodeList::Caches*> CacheMap;
     CacheMap m_classNodeListCaches;
     CacheMap m_nameNodeListCaches;
-    
+
     ~NodeListsNodeData()
     {
         deleteAllValues(m_classNodeListCaches);
         deleteAllValues(m_nameNodeListCaches);
     }
+
+    void invalidateCaches();
+    void invalidateCachesThatDependOnAttributes();
+    bool isEmpty() const;
 };
 
+// --------
+
 bool Node::isSupported(const String& feature, const String& version)
 {
     return DOMImplementation::instance()->hasFeature(feature, version);
@@ -166,6 +173,10 @@ Node::~Node()
     else
         --NodeCounter::count;
 #endif
+
+    if (m_nodeLists && m_document)
+        document()->removeNodeListCache();
+
     if (renderer())
         detach();
 
@@ -193,8 +204,10 @@ void Node::setNodeValue(const String& /*nodeValue*/, ExceptionCode& ec)
 
 PassRefPtr<NodeList> Node::childNodes()
 {
-    if (!m_nodeLists)
+    if (!m_nodeLists) {
         m_nodeLists.set(new NodeListsNodeData);
+        document()->addNodeListCache();
+    }
 
     return new ChildNodeList(this, &m_nodeLists->m_childNodeListCaches);
 }
@@ -406,23 +419,28 @@ unsigned Node::nodeIndex() const
 
 void Node::registerDynamicNodeList(DynamicNodeList* list)
 {
-    if (!m_nodeLists)
+    if (!m_nodeLists) {
         m_nodeLists.set(new NodeListsNodeData);
-    else if (!m_document->hasNodeLists())
+        document()->addNodeListCache();
+    } else if (!m_document->hasNodeListCaches()) {
         // We haven't been receiving notifications while there were no registered lists, so the cache is invalid now.
-        m_nodeLists->m_childNodeListCaches.reset();
+        m_nodeLists->invalidateCaches();
+    }
 
-    if (list->needsNotifications())
-        m_nodeLists->m_listsToNotify.add(list);
-    m_document->addNodeList();
+    if (list->hasOwnCaches())
+        m_nodeLists->m_listsWithCaches.add(list);
 }
 
 void Node::unregisterDynamicNodeList(DynamicNodeList* list)
 {
     ASSERT(m_nodeLists);
-    m_document->removeNodeList();
-    if (list->needsNotifications())
-        m_nodeLists->m_listsToNotify.remove(list);
+    if (list->hasOwnCaches()) {
+        m_nodeLists->m_listsWithCaches.remove(list);
+        if (m_nodeLists->isEmpty()) {
+            m_nodeLists.clear();
+            document()->removeNodeListCache();
+        }
+    }
 }
 
 void Node::notifyLocalNodeListsAttributeChanged()
@@ -430,9 +448,12 @@ void Node::notifyLocalNodeListsAttributeChanged()
     if (!m_nodeLists)
         return;
 
-    NodeListSet::iterator end = m_nodeLists->m_listsToNotify.end();
-    for (NodeListSet::iterator i = m_nodeLists->m_listsToNotify.begin(); i != end; ++i)
-        (*i)->rootNodeAttributeChanged();
+    m_nodeLists->invalidateCachesThatDependOnAttributes();
+
+    if (m_nodeLists->isEmpty()) {
+        m_nodeLists.clear();
+        document()->removeNodeListCache();
+    }
 }
 
 void Node::notifyNodeListsAttributeChanged()
@@ -446,16 +467,21 @@ void Node::notifyLocalNodeListsChildrenChanged()
     if (!m_nodeLists)
         return;
 
-    m_nodeLists->m_childNodeListCaches.reset();
+    m_nodeLists->invalidateCaches();
+
+    NodeListsNodeData::NodeListSet::iterator end = m_nodeLists->m_listsWithCaches.end();
+    for (NodeListsNodeData::NodeListSet::iterator i = m_nodeLists->m_listsWithCaches.begin(); i != end; ++i)
+        (*i)->invalidateCache();
 
-    NodeListSet::iterator end = m_nodeLists->m_listsToNotify.end();
-    for (NodeListSet::iterator i = m_nodeLists->m_listsToNotify.begin(); i != end; ++i)
-        (*i)->rootNodeChildrenChanged();
+    if (m_nodeLists->isEmpty()) {
+        m_nodeLists.clear();
+        document()->removeNodeListCache();
+    }
 }
 
 void Node::notifyNodeListsChildrenChanged()
 {
-    for (Node *n = this; n; n = n->parentNode())
+    for (Noden = this; n; n = n->parentNode())
         n->notifyLocalNodeListsChildrenChanged();
 }
 
@@ -1175,8 +1201,10 @@ PassRefPtr<NodeList> Node::getElementsByTagNameNS(const String& namespaceURI, co
 
 PassRefPtr<NodeList> Node::getElementsByName(const String& elementName)
 {
-    if (!m_nodeLists)
+    if (!m_nodeLists) {
         m_nodeLists.set(new NodeListsNodeData);
+        document()->addNodeListCache();
+    }
 
     pair<NodeListsNodeData::CacheMap::iterator, bool> result = m_nodeLists->m_nameNodeListCaches.add(elementName, 0);
     if (result.second)
@@ -1187,8 +1215,10 @@ PassRefPtr<NodeList> Node::getElementsByName(const String& elementName)
 
 PassRefPtr<NodeList> Node::getElementsByClassName(const String& classNames)
 {
-    if (!m_nodeLists)
+    if (!m_nodeLists) {
         m_nodeLists.set(new NodeListsNodeData);
+        document()->addNodeListCache();
+    }
 
     pair<NodeListsNodeData::CacheMap::iterator, bool> result = m_nodeLists->m_classNodeListCaches.add(classNames, 0);
     if (result.second)
@@ -1641,8 +1671,52 @@ void Node::formatForDebugger(char* buffer, unsigned length) const
 
 #endif
 
+// --------
+
+void NodeListsNodeData::invalidateCaches()
+{
+    m_childNodeListCaches.reset();
+    invalidateCachesThatDependOnAttributes();
+}
+
+void NodeListsNodeData::invalidateCachesThatDependOnAttributes()
+{
+    CacheMap::iterator classCachesEnd = m_classNodeListCaches.end();
+    for (CacheMap::iterator it = m_classNodeListCaches.begin(); it != classCachesEnd; ++it)
+        it->second->reset();
+
+    CacheMap::iterator nameCachesEnd = m_nameNodeListCaches.end();
+    for (CacheMap::iterator it = m_nameNodeListCaches.begin(); it != nameCachesEnd; ++it)
+        it->second->reset();
 }
 
+bool NodeListsNodeData::isEmpty() const
+{
+    if (!m_listsWithCaches.isEmpty())
+        return false;
+
+    if (m_childNodeListCaches.refCount)
+        return false;
+
+    CacheMap::const_iterator classCachesEnd = m_classNodeListCaches.end();
+    for (CacheMap::const_iterator it = m_classNodeListCaches.begin(); it != classCachesEnd; ++it) {
+        if (it->second->refCount)
+            return false;
+    }
+
+    CacheMap::const_iterator nameCachesEnd = m_nameNodeListCaches.end();
+    for (CacheMap::const_iterator it = m_nameNodeListCaches.begin(); it != nameCachesEnd; ++it) {
+        if (it->second->refCount)
+            return false;
+    }
+
+    return true;
+}
+
+// --------
+
+} // namespace WebCore
+
 #ifndef NDEBUG
 
 void showTree(const WebCore::Node* node)
index 81ac8e7..28602c8 100644 (file)
@@ -30,7 +30,7 @@
 namespace WebCore {
 
 TagNodeList::TagNodeList(PassRefPtr<Node> rootNode, const AtomicString& namespaceURI, const AtomicString& localName)
-    : DynamicNodeList(rootNode, true)
+    : DynamicNodeList(rootNode)
     , m_namespaceURI(namespaceURI)
     , m_localName(localName)
 {