appendChild shouldn't invalidate LiveNodeLists and HTMLCollections if they don't...

https://bugs.webkit.org/show_bug.cgi?id=129727

Reviewed by Andreas Kling.

Before this patch, invalidateNodeListAndCollectionCachesInAncestors invalidated node lists and HTML
collections on ancestors of a node whenever we're inserting or removing a child node. This patch
makes HTMLCollections and LiveNodeLists register themselves with Document only when they have valid
caches.

Each user of CollectionIndexCache now implements willValidateIndexCache member function that gets
called when CollectionIndexCache caches any state and necessitates the registration with document.

* dom/ChildNodeList.h: Added an empty willValidateIndexCache since child node lists are never
registered with document.

* dom/CollectionIndexCache.h:
(WebCore::CollectionIndexCache::hasValidCache): Added.
(WebCore::CollectionIndexCache::nodeCount): Calls willValidateIndexCache when caching node count.
(WebCore::CollectionIndexCache::nodeAfterCached): Ditto. Also assert that hasValidCache() true in
the cases where we're simply updating our caches or adding more caches.
(WebCore::CollectionIndexCache::nodeAt): Ditto. Also added a code to set the length cache when
we've reached the end of the list. This should be a slight speed up on some cases.

* dom/Document.cpp:
(WebCore::Document::Document): Initializes a variable used by assertions.
(WebCore::Document::unregisterNodeList): Added an early exit for when m_listsInvalidatedAtDocument
is empty since invalidateNodeListAndCollectionCaches swaps out the list.
(WebCore::Document::registerCollection): Removed the boolean hasIdNameMap since we now explicitly
call collectionCachedIdNameMap in HTMLCollection.
(WebCore::Document::unregisterCollection): Ditto. Exit early if m_collectionsInvalidatedAtDocument
is empty since invalidateNodeListAndCollectionCaches swaps out the list.
* dom/Document.h:

* dom/LiveNodeList.cpp:
(WebCore::LiveNodeList::invalidateCache): Unregister the node list with document if we had caches.
* dom/LiveNodeList.h:
(WebCore::LiveNodeList::LiveNodeList):
(WebCore::LiveNodeList::~LiveNodeList): Ditto.
(WebCore::LiveNodeList::invalidateCache): Pass around document. This is necessary since document()
had already moved to the new document inside NodeListsNodeData::invalidateCaches.
(WebCore::LiveNodeList::willValidateIndexCache): Added. Registers itself with document.

* dom/Node.cpp:
(WebCore::Document::invalidateNodeListAndCollectionCaches): Swap the lists since invalidateCache
tries to unregister node lists and HTML collections with document. Since this is the only case in
which node lists and HTML collections being removed may not be in the lists in unregisterNodeList
and unregisterCollection, assert this condition via m_inInvalidateNodeListAndCollectionCaches.
(WebCore::NodeListsNodeData::invalidateCaches):

* dom/NodeRareData.h:
(WebCore::NodeListsNodeData::adoptDocument): Unregister node lists and HTML collections from old
document via invalidateCache. We need to explicitly pass in oldDocument here since owner node's
document had already been changed to newDocument at this point. Since we're invalidating caches,
there is no need to register node lists and HTML collections with newDocument.

* html/HTMLCollection.cpp:
(WebCore::HTMLCollection::HTMLCollection):
(WebCore::HTMLCollection::~HTMLCollection): Unregister the node list with document if we had caches.
(WebCore::HTMLCollection::invalidateCache): Ditto.
(WebCore::HTMLCollection::invalidateNamedElementCache):
* html/HTMLCollection.h:
(WebCore::HTMLCollection::invalidateCache): Pass around document as done in LiveNodeList.
(WebCore::HTMLCollection::willValidateIndexCache): Ditto.

* html/HTMLFormControlsCollection.cpp:
(WebCore::HTMLFormControlsCollection::invalidateCache): Ditto.
* html/HTMLFormControlsCollection.h:

* html/HTMLSelectElement.cpp:
(WebCore::HTMLSelectElement::invalidateSelectedItems): Ditto.
(WebCore::HTMLSelectElement::setRecalcListItems): Ditto.

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 16498e0643a04af14787ffa7a531816b6cd0647a..57a29e96d3ea64423376475e6be5fc01f032b90d 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,78 @@
+2014-03-05  Ryosuke Niwa  <rniwa@webkit.org>
+
+        appendChild shouldn't invalidate LiveNodeLists and HTMLCollections if they don't have valid caches
+        https://bugs.webkit.org/show_bug.cgi?id=129727
+
+        Reviewed by Andreas Kling.
+
+        Before this patch, invalidateNodeListAndCollectionCachesInAncestors invalidated node lists and HTML
+        collections on ancestors of a node whenever we're inserting or removing a child node. This patch
+        makes HTMLCollections and LiveNodeLists register themselves with Document only when they have valid
+        caches.
+
+        Each user of CollectionIndexCache now implements willValidateIndexCache member function that gets
+        called when CollectionIndexCache caches any state and necessitates the registration with document.
+
+        * dom/ChildNodeList.h: Added an empty willValidateIndexCache since child node lists are never
+        registered with document.
+
+        * dom/CollectionIndexCache.h:
+        (WebCore::CollectionIndexCache::hasValidCache): Added.
+        (WebCore::CollectionIndexCache::nodeCount): Calls willValidateIndexCache when caching node count.
+        (WebCore::CollectionIndexCache::nodeAfterCached): Ditto. Also assert that hasValidCache() true in
+        the cases where we're simply updating our caches or adding more caches.
+        (WebCore::CollectionIndexCache::nodeAt): Ditto. Also added a code to set the length cache when
+        we've reached the end of the list. This should be a slight speed up on some cases.
+
+        * dom/Document.cpp:
+        (WebCore::Document::Document): Initializes a variable used by assertions.
+        (WebCore::Document::unregisterNodeList): Added an early exit for when m_listsInvalidatedAtDocument
+        is empty since invalidateNodeListAndCollectionCaches swaps out the list.
+        (WebCore::Document::registerCollection): Removed the boolean hasIdNameMap since we now explicitly
+        call collectionCachedIdNameMap in HTMLCollection.
+        (WebCore::Document::unregisterCollection): Ditto. Exit early if m_collectionsInvalidatedAtDocument
+        is empty since invalidateNodeListAndCollectionCaches swaps out the list.
+        * dom/Document.h:
+
+        * dom/LiveNodeList.cpp:
+        (WebCore::LiveNodeList::invalidateCache): Unregister the node list with document if we had caches.
+        * dom/LiveNodeList.h:
+        (WebCore::LiveNodeList::LiveNodeList):
+        (WebCore::LiveNodeList::~LiveNodeList): Ditto.
+        (WebCore::LiveNodeList::invalidateCache): Pass around document. This is necessary since document()
+        had already moved to the new document inside NodeListsNodeData::invalidateCaches.
+        (WebCore::LiveNodeList::willValidateIndexCache): Added. Registers itself with document.
+
+        * dom/Node.cpp:
+        (WebCore::Document::invalidateNodeListAndCollectionCaches): Swap the lists since invalidateCache
+        tries to unregister node lists and HTML collections with document. Since this is the only case in
+        which node lists and HTML collections being removed may not be in the lists in unregisterNodeList
+        and unregisterCollection, assert this condition via m_inInvalidateNodeListAndCollectionCaches.
+        (WebCore::NodeListsNodeData::invalidateCaches):
+
+        * dom/NodeRareData.h:
+        (WebCore::NodeListsNodeData::adoptDocument): Unregister node lists and HTML collections from old
+        document via invalidateCache. We need to explicitly pass in oldDocument here since owner node's
+        document had already been changed to newDocument at this point. Since we're invalidating caches,
+        there is no need to register node lists and HTML collections with newDocument.
+
+        * html/HTMLCollection.cpp:
+        (WebCore::HTMLCollection::HTMLCollection):
+        (WebCore::HTMLCollection::~HTMLCollection): Unregister the node list with document if we had caches.
+        (WebCore::HTMLCollection::invalidateCache): Ditto.
+        (WebCore::HTMLCollection::invalidateNamedElementCache):
+        * html/HTMLCollection.h:
+        (WebCore::HTMLCollection::invalidateCache): Pass around document as done in LiveNodeList.
+        (WebCore::HTMLCollection::willValidateIndexCache): Ditto.
+
+        * html/HTMLFormControlsCollection.cpp:
+        (WebCore::HTMLFormControlsCollection::invalidateCache): Ditto.
+        * html/HTMLFormControlsCollection.h:
+
+        * html/HTMLSelectElement.cpp:
+        (WebCore::HTMLSelectElement::invalidateSelectedItems): Ditto.
+        (WebCore::HTMLSelectElement::setRecalcListItems): Ditto.
+
 2014-03-05  Jon Lee  <jonlee@apple.com>
 
         Fix linker error after r165087

diff --git a/Source/WebCore/dom/ChildNodeList.h b/Source/WebCore/dom/ChildNodeList.h
index 0a262030fe9fd152b0395793d9af3951c985f21e..ecc0b9a3f9160690bbae671bb3cf11e1420df9cc 100644 (file)
--- a/Source/WebCore/dom/ChildNodeList.h
+++ b/Source/WebCore/dom/ChildNodeList.h
@@ -75,6 +75,7 @@ public:
     Node* collectionTraverseForward(Node&, unsigned count, unsigned& traversedCount) const;
     Node* collectionTraverseBackward(Node&, unsigned count) const;
     bool collectionCanTraverseBackward() const { return true; }
+    void willValidateIndexCache() const { }
 
 private:
     explicit ChildNodeList(ContainerNode& parent);

diff --git a/Source/WebCore/dom/CollectionIndexCache.h b/Source/WebCore/dom/CollectionIndexCache.h
index 2d98c6fe9fadedda07eec67b0bf5a14549d68a61..e8c4d0b66c39e22f0ba6ef05c16972c464ece593 100644 (file)
--- a/Source/WebCore/dom/CollectionIndexCache.h
+++ b/Source/WebCore/dom/CollectionIndexCache.h
@@ -40,10 +40,12 @@ public:
     unsigned nodeCount(const Collection&);
     NodeType* nodeAt(const Collection&, unsigned index);
 
+    bool hasValidCache() const { return m_currentNode || m_nodeCountValid || m_listValid; }
     void invalidate();
     size_t memoryCost() { return m_cachedList.capacity() * sizeof(NodeType*); }
 
 private:
+
     unsigned computeNodeCountUpdatingListCache(const Collection&);
     NodeType* nodeBeforeCached(const Collection&, unsigned);
     NodeType* nodeAfterCached(const Collection&, unsigned);
@@ -70,6 +72,8 @@ template <class Collection, class NodeType>
 inline unsigned CollectionIndexCache<Collection, NodeType>::nodeCount(const Collection& collection)
 {
     if (!m_nodeCountValid) {
+        if (!hasValidCache())
+            collection.willValidateIndexCache();
         m_nodeCount = computeNodeCountUpdatingListCache(collection);
         m_nodeCountValid = true;
     }
@@ -132,6 +136,7 @@ inline NodeType* CollectionIndexCache<Collection, NodeType>::nodeAfterCached(con
 
     bool lastIsCloser = m_nodeCountValid && m_nodeCount - index < index - m_currentIndex;
     if (lastIsCloser && collection.collectionCanTraverseBackward()) {
+        ASSERT(hasValidCache());
         m_currentNode = collection.collectionLast();
         if (index < m_nodeCount - 1)
             m_currentNode = collection.collectionTraverseBackward(*m_currentNode, m_nodeCount - index - 1);
@@ -140,6 +145,9 @@ inline NodeType* CollectionIndexCache<Collection, NodeType>::nodeAfterCached(con
         return m_currentNode;
     }
 
+    if (!hasValidCache())
+        collection.willValidateIndexCache();
+
     unsigned traversedCount;
     m_currentNode = collection.collectionTraverseForward(*m_currentNode, index - m_currentIndex, traversedCount);
     m_currentIndex = m_currentIndex + traversedCount;
@@ -151,6 +159,7 @@ inline NodeType* CollectionIndexCache<Collection, NodeType>::nodeAfterCached(con
         m_nodeCount = m_currentIndex + 1;
         m_nodeCountValid = true;
     }
+    ASSERT(hasValidCache());
     return m_currentNode;
 }
 
@@ -173,6 +182,7 @@ inline NodeType* CollectionIndexCache<Collection, NodeType>::nodeAt(const Collec
 
     bool lastIsCloser = m_nodeCountValid && m_nodeCount - index < index;
     if (lastIsCloser && collection.collectionCanTraverseBackward()) {
+        ASSERT(hasValidCache());
         m_currentNode = collection.collectionLast();
         if (index < m_nodeCount - 1)
             m_currentNode = collection.collectionTraverseBackward(*m_currentNode, m_nodeCount - index - 1);
@@ -181,12 +191,21 @@ inline NodeType* CollectionIndexCache<Collection, NodeType>::nodeAt(const Collec
         return m_currentNode;
     }
 
+    if (!hasValidCache())
+        collection.willValidateIndexCache();
+
     m_currentNode = collection.collectionFirst();
     m_currentIndex = 0;
     if (index && m_currentNode) {
         m_currentNode = collection.collectionTraverseForward(*m_currentNode, index, m_currentIndex);
         ASSERT(m_currentNode || m_currentIndex < index);
     }
+    if (!m_currentNode && !m_nodeCountValid) {
+        // Failed to find the index but at least we now know the size.
+        m_nodeCount = m_currentIndex + 1;
+        m_nodeCountValid = true;
+    }
+    ASSERT(hasValidCache());
     return m_currentNode;
 }
 

diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
index 0c72ba488d8822898a7eac3ad798d1c05badff0f..941462e16b3a9bb52c519ad74b37debab695856a 100644 (file)
--- a/Source/WebCore/dom/Document.cpp
+++ b/Source/WebCore/dom/Document.cpp
@@ -454,6 +454,9 @@ Document::Document(Frame* frame, const URL& url, unsigned documentClasses, unsig
     , m_xmlStandalone(StandaloneUnspecified)
     , m_hasXMLDeclaration(false)
     , m_designMode(inherit)
+#if !ASSERT_DISABLED
+    , m_inInvalidateNodeListAndCollectionCaches(false)
+#endif
 #if ENABLE(DASHBOARD_SUPPORT)
     , m_hasAnnotatedRegions(false)
     , m_annotatedRegionsDirty(false)
@@ -3459,27 +3462,31 @@ void Document::unregisterNodeList(LiveNodeList& list)
 {
     m_nodeListAndCollectionCounts[list.invalidationType()]--;
     if (list.isRootedAtDocument()) {
+        if (!m_listsInvalidatedAtDocument.size()) {
+            ASSERT(m_inInvalidateNodeListAndCollectionCaches);
+            return;
+        }
         ASSERT(m_listsInvalidatedAtDocument.contains(&list));
         m_listsInvalidatedAtDocument.remove(&list);
     }
 }
 
-void Document::registerCollection(HTMLCollection& collection, bool hasIdNameMap)
+void Document::registerCollection(HTMLCollection& collection)
 {
-    if (hasIdNameMap)
-        collectionCachedIdNameMap(collection);
     m_nodeListAndCollectionCounts[collection.invalidationType()]++;
     if (collection.isRootedAtDocument())
         m_collectionsInvalidatedAtDocument.add(&collection);
 }
 
-void Document::unregisterCollection(HTMLCollection& collection, bool hasIdNameMap)
+void Document::unregisterCollection(HTMLCollection& collection)
 {
-    if (hasIdNameMap)
-        collectionWillClearIdNameMap(collection);
     ASSERT(m_nodeListAndCollectionCounts[collection.invalidationType()]);
     m_nodeListAndCollectionCounts[collection.invalidationType()]--;
     if (collection.isRootedAtDocument()) {
+        if (!m_collectionsInvalidatedAtDocument.size()) {
+            ASSERT(m_inInvalidateNodeListAndCollectionCaches);
+            return;
+        }
         ASSERT(m_collectionsInvalidatedAtDocument.contains(&collection));
         m_collectionsInvalidatedAtDocument.remove(&collection);
     }

diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
index 14da1b2a10331e9b7290c641c0660a3ea5fbb682..c124586b328133e0d47f8af9c2df9adc6b0e9c35 100644 (file)
--- a/Source/WebCore/dom/Document.h
+++ b/Source/WebCore/dom/Document.h
@@ -766,8 +766,8 @@ public:
 
     void registerNodeList(LiveNodeList&);
     void unregisterNodeList(LiveNodeList&);
-    void registerCollection(HTMLCollection&, bool hasNamedElementCache);
-    void unregisterCollection(HTMLCollection&, bool hasNamedElementCache);
+    void registerCollection(HTMLCollection&);
+    void unregisterCollection(HTMLCollection&);
     void collectionCachedIdNameMap(const HTMLCollection&);
     void collectionWillClearIdNameMap(const HTMLCollection&);
     bool shouldInvalidateNodeListAndCollectionCaches(const QualifiedName* attrName = nullptr) const;
@@ -1506,6 +1506,9 @@ private:
 
     HashSet<LiveNodeList*> m_listsInvalidatedAtDocument;
     HashSet<HTMLCollection*> m_collectionsInvalidatedAtDocument;
+#if !ASSERT_DISABLED
+    bool m_inInvalidateNodeListAndCollectionCaches;
+#endif
 
     unsigned m_nodeListAndCollectionCounts[numNodeListInvalidationTypes];
 

diff --git a/Source/WebCore/dom/LiveNodeList.cpp b/Source/WebCore/dom/LiveNodeList.cpp
index 2a2864a935e4c9fc5e7846cad359007b58939a50..c539913c73e8c0f43da2f57ba78d06ef0eb0d721 100644 (file)
--- a/Source/WebCore/dom/LiveNodeList.cpp
+++ b/Source/WebCore/dom/LiveNodeList.cpp
@@ -148,8 +148,11 @@ size_t LiveNodeList::memoryCost() const
     return m_indexCache.memoryCost();
 }
 
-void LiveNodeList::invalidateCache() const
+void LiveNodeList::invalidateCache(Document& document) const
 {
+    if (!m_indexCache.hasValidCache())
+        return;
+    document.unregisterNodeList(const_cast<LiveNodeList&>(*this));
     m_indexCache.invalidate();
 }
 

diff --git a/Source/WebCore/dom/LiveNodeList.h b/Source/WebCore/dom/LiveNodeList.h
index b2940ae1a42268589b3595bf58f3d50eef1d629c..5e69e9a0d8606f9327092643504c3c9f68839cf5 100644 (file)
--- a/Source/WebCore/dom/LiveNodeList.h
+++ b/Source/WebCore/dom/LiveNodeList.h
@@ -63,15 +63,14 @@ public:
         ASSERT(m_rootType == static_cast<unsigned>(rootType));
         ASSERT(m_invalidationType == static_cast<unsigned>(invalidationType));
         ASSERT(m_type == static_cast<unsigned>(type));
-
-        document().registerNodeList(*this);
     }
     virtual Node* namedItem(const AtomicString&) const override final;
     virtual bool nodeMatches(Element*) const = 0;
 
     virtual ~LiveNodeList()
     {
-        document().unregisterNodeList(*this);
+        if (m_indexCache.hasValidCache())
+            document().unregisterNodeList(*this);
     }
 
     // DOM API
@@ -86,9 +85,9 @@ public:
     ALWAYS_INLINE void invalidateCache(const QualifiedName* attrName) const
     {
         if (!attrName || shouldInvalidateTypeOnAttributeChange(invalidationType(), *attrName))
-            invalidateCache();
+            invalidateCache(document());
     }
-    void invalidateCache() const;
+    void invalidateCache(Document&) const;
 
     // For CollectionIndexCache
     Element* collectionFirst() const;
@@ -96,6 +95,7 @@ public:
     Element* collectionTraverseForward(Element&, unsigned count, unsigned& traversedCount) const;
     Element* collectionTraverseBackward(Element&, unsigned count) const;
     bool collectionCanTraverseBackward() const { return true; }
+    void willValidateIndexCache() const { document().registerNodeList(const_cast<LiveNodeList&>(*this)); }
 
 protected:
     Document& document() const { return m_ownerNode->document(); }

diff --git a/Source/WebCore/dom/Node.cpp b/Source/WebCore/dom/Node.cpp
index a99aea45b2079598a46d61c3f10b9c4ac1b99cf4..43d3fcfcafd4a8fde159597286a918770c775876 100644 (file)
--- a/Source/WebCore/dom/Node.cpp
+++ b/Source/WebCore/dom/Node.cpp
@@ -721,10 +721,19 @@ bool Document::shouldInvalidateNodeListAndCollectionCaches(const QualifiedName*
 
 void Document::invalidateNodeListAndCollectionCaches(const QualifiedName* attrName)
 {
-    for (HashSet<LiveNodeList*>::iterator it = m_listsInvalidatedAtDocument.begin(), end = m_listsInvalidatedAtDocument.end(); it != end; ++it)
-        (*it)->invalidateCache(attrName);
-    for (HashSet<HTMLCollection*>::iterator it = m_collectionsInvalidatedAtDocument.begin(), end = m_collectionsInvalidatedAtDocument.end(); it != end; ++it)
-        (*it)->invalidateCache(attrName);
+#if !ASSERT_DISABLED
+    m_inInvalidateNodeListAndCollectionCaches = true;
+#endif
+    HashSet<LiveNodeList*> liveNodeLists = std::move(m_listsInvalidatedAtDocument);
+    for (auto it : liveNodeLists)
+        it->invalidateCache(attrName);
+
+    HashSet<HTMLCollection*> collectionLists = std::move(m_collectionsInvalidatedAtDocument);
+    for (auto it : collectionLists)
+        it->invalidateCache(attrName);
+#if !ASSERT_DISABLED
+    m_inInvalidateNodeListAndCollectionCaches = false;
+#endif
 }
 
 void Node::invalidateNodeListAndCollectionCachesInAncestors(const QualifiedName* attrName, Element* attributeOwnerElement)
@@ -1689,7 +1698,7 @@ void NodeListsNodeData::invalidateCaches(const QualifiedName* attrName)
         return;
 
     for (auto& tagNodeList : m_tagNodeListCacheNS)
-        tagNodeList.value->invalidateCache();
+        tagNodeList.value->invalidateCache(nullptr);
 }
 
 void Node::getSubresourceURLs(ListHashSet<URL>& urls) const

diff --git a/Source/WebCore/dom/NodeRareData.h b/Source/WebCore/dom/NodeRareData.h
index f8f3d7e8954e45ccdcba785f613d9a777e3bebc5..ef736baacd16bc8db0857aa3103879f516a59795 100644 (file)
--- a/Source/WebCore/dom/NodeRareData.h
+++ b/Source/WebCore/dom/NodeRareData.h
@@ -225,39 +225,26 @@ public:
 
     void adoptDocument(Document* oldDocument, Document* newDocument)
     {
+        ASSERT(oldDocument);
         if (oldDocument == newDocument) {
             invalidateCaches();
             return;
         }
 
-        for (auto it : m_atomicNameCaches) {
-            LiveNodeList& list = *it.value;
-            oldDocument->unregisterNodeList(list);
-            newDocument->registerNodeList(list);
-            list.invalidateCache();
-        }
+        for (auto it : m_atomicNameCaches)
+            it.value->invalidateCache(*oldDocument);
 
-        for (auto it : m_nameCaches) {
-            LiveNodeList& list = *it.value;
-            oldDocument->unregisterNodeList(list);
-            newDocument->registerNodeList(list);
-            list.invalidateCache();
-        }
+        for (auto it : m_nameCaches)
+            it.value->invalidateCache(*oldDocument);
 
         for (auto it : m_tagNodeListCacheNS) {
             LiveNodeList& list = *it.value;
             ASSERT(!list.isRootedAtDocument());
-            oldDocument->unregisterNodeList(list);
-            newDocument->registerNodeList(list);
-            list.invalidateCache();
+            list.invalidateCache(*oldDocument);
         }
 
-        for (auto it : m_cachedCollections) {
-            HTMLCollection& collection = *it.value;
-            oldDocument->unregisterCollection(collection, collection.hasNamedElementCache());
-            newDocument->registerCollection(collection, collection.hasNamedElementCache());
-            collection.invalidateCache();
-        }
+        for (auto it : m_cachedCollections)
+            it.value->invalidateCache(*oldDocument);
     }
 
 private:

diff --git a/Source/WebCore/html/HTMLCollection.cpp b/Source/WebCore/html/HTMLCollection.cpp
index 34dff4278de787861781e030b2cbd3a20d1c3e53..65a137296020ae5ae7e27e9edf8e3f27cb504f5a 100644 (file)
--- a/Source/WebCore/html/HTMLCollection.cpp
+++ b/Source/WebCore/html/HTMLCollection.cpp
@@ -142,8 +142,6 @@ HTMLCollection::HTMLCollection(ContainerNode& ownerNode, CollectionType type, El
     ASSERT(m_rootType == static_cast<unsigned>(rootTypeFromCollectionType(type)));
     ASSERT(m_invalidationType == static_cast<unsigned>(invalidationTypeExcludingIdAndNameAttributes(type)));
     ASSERT(m_collectionType == static_cast<unsigned>(type));
-
-    document().registerCollection(*this, hasNamedElementCache());
 }
 
 PassRefPtr<HTMLCollection> HTMLCollection::create(ContainerNode& base, CollectionType type)
@@ -153,7 +151,10 @@ PassRefPtr<HTMLCollection> HTMLCollection::create(ContainerNode& base, Collectio
 
 HTMLCollection::~HTMLCollection()
 {
-    document().unregisterCollection(*this, hasNamedElementCache());
+    if (m_indexCache.hasValidCache())
+        document().unregisterCollection(*this);
+    if (hasNamedElementCache())
+        document().collectionWillClearIdNameMap(*this);
     // HTMLNameCollection removes cache by itself.
     if (type() != WindowNamedItems && type() != DocumentNamedItems)
         ownerNode().nodeLists()->removeCachedCollection(this);
@@ -362,17 +363,20 @@ Element* HTMLCollection::collectionTraverseBackward(Element& current, unsigned c
     return element;
 }
 
-void HTMLCollection::invalidateCache() const
+void HTMLCollection::invalidateCache(Document& document) const
 {
-    m_indexCache.invalidate();
+    if (m_indexCache.hasValidCache()) {
+        document.unregisterCollection(const_cast<HTMLCollection&>(*this));
+        m_indexCache.invalidate();
+    }
     if (hasNamedElementCache())
-        invalidateNamedElementCache();
+        invalidateNamedElementCache(document);
 }
 
-void HTMLCollection::invalidateNamedElementCache() const
+void HTMLCollection::invalidateNamedElementCache(Document& document) const
 {
     ASSERT(hasNamedElementCache());
-    document().collectionWillClearIdNameMap(*this);
+    document.collectionWillClearIdNameMap(*this);
     m_namedElementCache = nullptr;
 }
 

diff --git a/Source/WebCore/html/HTMLCollection.h b/Source/WebCore/html/HTMLCollection.h
index c9ad372057deaa5f20e521f5923c04c3f496088f..26f388f8ae450018c54caae3fbb8732708dca6e5 100644 (file)
--- a/Source/WebCore/html/HTMLCollection.h
+++ b/Source/WebCore/html/HTMLCollection.h
@@ -107,11 +107,11 @@ public:
     void invalidateCache(const QualifiedName* attrName) const
     {
         if (!attrName || shouldInvalidateTypeOnAttributeChange(invalidationType(), *attrName))
-            invalidateCache();
+            invalidateCache(document());
         else if (hasNamedElementCache() && (*attrName == HTMLNames::idAttr || *attrName == HTMLNames::nameAttr))
-            invalidateNamedElementCache();
+            invalidateNamedElementCache(document());
     }
-    virtual void invalidateCache() const;
+    virtual void invalidateCache(Document&) const;
 
     // For CollectionIndexCache
     Element* collectionFirst() const;
@@ -119,6 +119,7 @@ public:
     Element* collectionTraverseForward(Element&, unsigned count, unsigned& traversedCount) const;
     Element* collectionTraverseBackward(Element&, unsigned count) const;
     bool collectionCanTraverseBackward() const { return !m_usesCustomForwardOnlyTraversal; }
+    void willValidateIndexCache() const { document().registerCollection(const_cast<HTMLCollection&>(*this)); }
 
     bool hasNamedElementCache() const { return !!m_namedElementCache; }
 
@@ -155,7 +156,7 @@ private:
 
     virtual Element* customElementAfter(Element*) const { ASSERT_NOT_REACHED(); return nullptr; }
     
-    void invalidateNamedElementCache() const;
+    void invalidateNamedElementCache(Document&) const;
 
     Ref<ContainerNode> m_ownerNode;
 

diff --git a/Source/WebCore/html/HTMLFormControlsCollection.cpp b/Source/WebCore/html/HTMLFormControlsCollection.cpp
index a52937ee23446cb1beceffbc324100a39f5e545b..67c2fe4c81475eee1702d9d26aa9048164fa944f 100644 (file)
--- a/Source/WebCore/html/HTMLFormControlsCollection.cpp
+++ b/Source/WebCore/html/HTMLFormControlsCollection.cpp
@@ -177,9 +177,9 @@ void HTMLFormControlsCollection::updateNamedElementCache() const
     cache.didPopulate();
 }
 
-void HTMLFormControlsCollection::invalidateCache() const
+void HTMLFormControlsCollection::invalidateCache(Document& document) const
 {
-    HTMLCollection::invalidateCache();
+    HTMLCollection::invalidateCache(document);
     m_cachedElement = nullptr;
     m_cachedElementOffsetInArray = 0;
 }

diff --git a/Source/WebCore/html/HTMLFormControlsCollection.h b/Source/WebCore/html/HTMLFormControlsCollection.h
index 32cc3bb38aa6042ae572f4398580e17751f89211..e113e30e858646312f23265fe4687d5fa7d4efc9 100644 (file)
--- a/Source/WebCore/html/HTMLFormControlsCollection.h
+++ b/Source/WebCore/html/HTMLFormControlsCollection.h
@@ -46,7 +46,7 @@ public:
 private:
     explicit HTMLFormControlsCollection(ContainerNode&);
 
-    virtual void invalidateCache() const override;
+    virtual void invalidateCache(Document&) const override;
     virtual void updateNamedElementCache() const override;
 
     const Vector<FormAssociatedElement*>& formControlElements() const;

diff --git a/Source/WebCore/html/HTMLSelectElement.cpp b/Source/WebCore/html/HTMLSelectElement.cpp
index 88285435198f22d3ac7c44314bd730302a7ee68b..2c81217f9622c18e389d183e1fc1ddd75caf7b0f 100644 (file)
--- a/Source/WebCore/html/HTMLSelectElement.cpp
+++ b/Source/WebCore/html/HTMLSelectElement.cpp
@@ -750,7 +750,7 @@ const Vector<HTMLElement*>& HTMLSelectElement::listItems() const
 void HTMLSelectElement::invalidateSelectedItems()
 {
     if (HTMLCollection* collection = cachedHTMLCollection(SelectedOptions))
-        collection->invalidateCache();
+        collection->invalidateCache(document());
 }
 
 void HTMLSelectElement::setRecalcListItems()
@@ -762,7 +762,7 @@ void HTMLSelectElement::setRecalcListItems()
     setNeedsStyleRecalc();
     if (!inDocument()) {
         if (HTMLCollection* collection = cachedHTMLCollection(SelectOptions))
-            collection->invalidateCache();
+            collection->invalidateCache(document());
     }
     if (!inDocument())
         invalidateSelectedItems();