invalidateNodeListCachesInAncestors walks up ancestors even when an attribute that...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Jul 2012 20:56:32 +0000 (20:56 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Jul 2012 20:56:32 +0000 (20:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=91530

Reviewed by Ojan Vafai.

Source/WebCore:

The bug was caused by invalidateNodeListCachesInAncestors not calling Document::shouldInvalidateNodeListCaches with
attrName. Done that.

This chance revealed a bug in shouldInvalidateTypeOnAttributeChange that we weren't checking form attribute changes for
RadioNodeList and HTMLCollection, so fixed the bug.

Also renamed Document::clearNodeListCaches to invalidateNodeListCaches to match the name convention used elsewhere,
and added a new version of DynamicNodeListCacheBase::invalidateCache that takes attrName to reduce the code duplication.

Test: fast/forms/elements-invalidate-on-form-attribute-invalidation.html

* dom/Document.cpp:
(WebCore::Document::invalidateNodeListCaches):
* dom/Document.h:
(Document):
* dom/DynamicNodeList.h:
(WebCore::DynamicNodeListCacheBase::invalidateCache):
(WebCore::DynamicNodeListCacheBase::shouldInvalidateTypeOnAttributeChange):
* dom/Node.cpp:
(WebCore::Node::invalidateNodeListCachesInAncestors):
(WebCore::NodeListsNodeData::invalidateCaches):

LayoutTests:

Add a regression test for invalidating HTMLFormColletion on form attribute changes. This invalidation worked before
because we weren't properly exiting early in Node::invalidateNodeListCachesInAncestors and
Document::invalidateNodeListCaches invalidated all node lists regardless of the attribute type.

* fast/forms/elements-invalidate-on-form-attribute-invalidation-expected.txt: Added.
* fast/forms/elements-invalidate-on-form-attribute-invalidation.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/forms/elements-invalidate-on-form-attribute-invalidation-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/elements-invalidate-on-form-attribute-invalidation.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/dom/DynamicNodeList.h
Source/WebCore/dom/Node.cpp

index 2b36ac9..96aefdf 100644 (file)
@@ -1,3 +1,17 @@
+2012-07-17  Ryosuke Niwa  <rniwa@webkit.org>
+
+        invalidateNodeListCachesInAncestors walks up ancestors even when an attribute that doesn't invalidate node lists changes
+        https://bugs.webkit.org/show_bug.cgi?id=91530
+
+        Reviewed by Ojan Vafai.
+
+        Add a regression test for invalidating HTMLFormColletion on form attribute changes. This invalidation worked before
+        because we weren't properly exiting early in Node::invalidateNodeListCachesInAncestors and 
+        Document::invalidateNodeListCaches invalidated all node lists regardless of the attribute type.
+
+        * fast/forms/elements-invalidate-on-form-attribute-invalidation-expected.txt: Added.
+        * fast/forms/elements-invalidate-on-form-attribute-invalidation.html: Added.
+
 2012-07-17  Florin Malita  <fmalita@chromium.org>
 
         SVG getBBox does not update bound after path data change
diff --git a/LayoutTests/fast/forms/elements-invalidate-on-form-attribute-invalidation-expected.txt b/LayoutTests/fast/forms/elements-invalidate-on-form-attribute-invalidation-expected.txt
new file mode 100644 (file)
index 0000000..4be89cf
--- /dev/null
@@ -0,0 +1,10 @@
+Tests form.elements is invalidated when input element's form attribute is changed.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+collection = document.getElementById('someForm').elements;
+PASS collection.length is 1
+PASS collection.length; input.setAttribute('form', 'otherForm'); collection.length is 0
+PASS collection.length; input.setAttribute('form', 'someForm'); collection.length is 1
+
diff --git a/LayoutTests/fast/forms/elements-invalidate-on-form-attribute-invalidation.html b/LayoutTests/fast/forms/elements-invalidate-on-form-attribute-invalidation.html
new file mode 100644 (file)
index 0000000..3b2fa20
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="container">
+<form id="someForm"></form>
+<input type="text" name="someName" form="someForm">
+</div>
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+<script>
+
+description("Tests form.elements is invalidated when input element's form attribute is changed.");
+
+var input = document.querySelector('input');
+var collection;
+evalAndLog("collection = document.getElementById('someForm').elements;");
+shouldBe("collection.length", "1");
+shouldBe("collection.length; input.setAttribute('form', 'otherForm'); collection.length", "0");
+shouldBe("collection.length; input.setAttribute('form', 'someForm'); collection.length", "1");
+
+container.style.display = 'none';
+
+</script>
+</body>
+</html>
index 230beb4..06605ab 100644 (file)
@@ -1,3 +1,32 @@
+2012-07-17  Ryosuke Niwa  <rniwa@webkit.org>
+
+        invalidateNodeListCachesInAncestors walks up ancestors even when an attribute that doesn't invalidate node lists changes
+        https://bugs.webkit.org/show_bug.cgi?id=91530
+
+        Reviewed by Ojan Vafai.
+
+        The bug was caused by invalidateNodeListCachesInAncestors not calling Document::shouldInvalidateNodeListCaches with
+        attrName. Done that.
+        
+        This chance revealed a bug in shouldInvalidateTypeOnAttributeChange that we weren't checking form attribute changes for
+        RadioNodeList and HTMLCollection, so fixed the bug.
+
+        Also renamed Document::clearNodeListCaches to invalidateNodeListCaches to match the name convention used elsewhere,
+        and added a new version of DynamicNodeListCacheBase::invalidateCache that takes attrName to reduce the code duplication.
+
+        Test: fast/forms/elements-invalidate-on-form-attribute-invalidation.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::invalidateNodeListCaches):
+        * dom/Document.h:
+        (Document):
+        * dom/DynamicNodeList.h:
+        (WebCore::DynamicNodeListCacheBase::invalidateCache):
+        (WebCore::DynamicNodeListCacheBase::shouldInvalidateTypeOnAttributeChange):
+        * dom/Node.cpp:
+        (WebCore::Node::invalidateNodeListCachesInAncestors):
+        (WebCore::NodeListsNodeData::invalidateCaches):
+
 2012-07-17  Max Vujovic  <mvujovic@adobe.com>
 
         Update ANGLE in WebKit
index 724f11d..fc8e1b5 100644 (file)
@@ -3904,11 +3904,11 @@ bool Document::shouldInvalidateNodeListCaches(const QualifiedName* attrName) con
     return false;
 }
 
-void Document::clearNodeListCaches()
+void Document::invalidateNodeListCaches(const QualifiedName* attrName)
 {
     HashSet<DynamicNodeListCacheBase*>::iterator end = m_listsInvalidatedAtDocument.end();
     for (HashSet<DynamicNodeListCacheBase*>::iterator it = m_listsInvalidatedAtDocument.begin(); it != end; ++it)
-        (*it)->invalidateCache();
+        (*it)->invalidateCache(attrName);
 }
 
 void Document::attachNodeIterator(NodeIterator* ni)
index 6b9d0a3..b636184 100644 (file)
@@ -738,7 +738,7 @@ public:
     void registerNodeListCache(DynamicNodeListCacheBase*);
     void unregisterNodeListCache(DynamicNodeListCacheBase*);
     bool shouldInvalidateNodeListCaches(const QualifiedName* attrName = 0) const;
-    void clearNodeListCaches();
+    void invalidateNodeListCaches(const QualifiedName* attrName);
 
     void attachNodeIterator(NodeIterator*);
     void detachNodeIterator(NodeIterator*);
index a3c12c2..c1c05f4 100644 (file)
@@ -68,6 +68,11 @@ public:
     ALWAYS_INLINE NodeListInvalidationType invalidationType() const { return static_cast<NodeListInvalidationType>(m_invalidationType); }
     ALWAYS_INLINE CollectionType type() const { return static_cast<CollectionType>(m_collectionType); }
 
+    ALWAYS_INLINE void invalidateCache(const QualifiedName* attrName) const
+    {
+        if (!attrName || shouldInvalidateTypeOnAttributeChange(invalidationType(), *attrName))
+            invalidateCache();
+    }
     void invalidateCache() const;
 
     static bool shouldInvalidateTypeOnAttributeChange(NodeListInvalidationType, const QualifiedName&);
@@ -124,7 +129,8 @@ ALWAYS_INLINE bool DynamicNodeListCacheBase::shouldInvalidateTypeOnAttributeChan
     case InvalidateOnForAttrChange:
         return attrName == HTMLNames::forAttr;
     case InvalidateForFormControls:
-        return attrName == HTMLNames::nameAttr || attrName == HTMLNames::idAttr || attrName == HTMLNames::forAttr || attrName == HTMLNames::typeAttr;
+        return attrName == HTMLNames::nameAttr || attrName == HTMLNames::idAttr || attrName == HTMLNames::forAttr
+            || attrName == HTMLNames::formAttr || attrName == HTMLNames::typeAttr;
     case InvalidateOnHRefAttrChange:
         return attrName == HTMLNames::hrefAttr;
     case InvalidateOnItemAttrChange:
index 6214b29..8fd6d5c 100644 (file)
@@ -974,7 +974,7 @@ void Node::invalidateNodeListCachesInAncestors(const QualifiedName* attrName, El
     if (!document()->shouldInvalidateNodeListCaches())
         return;
 
-    document()->clearNodeListCaches();
+    document()->invalidateNodeListCaches(attrName);
 
     for (Node* node = this; node; node = node->parentNode()) {
         if (!node->hasRareData())
@@ -2214,18 +2214,12 @@ void Node::showTreeForThisAcrossFrame() const
 void NodeListsNodeData::invalidateCaches(const QualifiedName* attrName)
 {
     NodeListAtomicNameCacheMap::const_iterator atomicNameCacheEnd = m_atomicNameCaches.end();
-    for (NodeListAtomicNameCacheMap::const_iterator it = m_atomicNameCaches.begin(); it != atomicNameCacheEnd; ++it) {
-        DynamicNodeList* list = it->second;
-        if (!attrName || DynamicNodeListCacheBase::shouldInvalidateTypeOnAttributeChange(list->invalidationType(), *attrName))
-            list->invalidateCache();
-    }
+    for (NodeListAtomicNameCacheMap::const_iterator it = m_atomicNameCaches.begin(); it != atomicNameCacheEnd; ++it)
+        it->second->invalidateCache(attrName);
 
     NodeListNameCacheMap::const_iterator nameCacheEnd = m_nameCaches.end();
-    for (NodeListNameCacheMap::const_iterator it = m_nameCaches.begin(); it != nameCacheEnd; ++it) {
-        DynamicNodeList* list = it->second;
-        if (!attrName || DynamicNodeListCacheBase::shouldInvalidateTypeOnAttributeChange(list->invalidationType(), *attrName))
-            list->invalidateCache();
-    }
+    for (NodeListNameCacheMap::const_iterator it = m_nameCaches.begin(); it != nameCacheEnd; ++it)
+        it->second->invalidateCache(attrName);
 
     if (!attrName)
         return;