invalidateNodeListsCacheAfterAttributeChanged has too many callers
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Dec 2011 23:15:10 +0000 (23:15 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Dec 2011 23:15:10 +0000 (23:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=74692

Reviewed by Sam Weinig.

Source/WebCore:

Call invalidateNodeListsCacheAfterAttributeChanged in Element::updateAfterAttributeChanged instead of
parsedMappedAttribute of various elements. Also make invalidateNodeListsCacheAfterAttributeChanged take
the qualified name of the changed attribute so that we can exit early when the changed attribute isn't
one of attributes we care.

In addition, added a missing call to invalidateNodeListsCacheAfterAttributeChanged in Attr::setValue.

Test: fast/dom/Attr/invalidate-nodelist-after-attr-setvalue.html

* dom/Attr.cpp:
(WebCore::Attr::childrenChanged):
* dom/Element.cpp:
(WebCore::Element::updateAfterAttributeChanged):
* dom/NamedNodeMap.cpp:
(WebCore::NamedNodeMap::addAttribute):
(WebCore::NamedNodeMap::removeAttribute):
* dom/Node.cpp:
(WebCore::Node::invalidateNodeListsCacheAfterAttributeChanged):
* dom/Node.h:
* dom/StyledElement.cpp:
(WebCore::StyledElement::classAttributeChanged):
* html/HTMLAnchorElement.cpp:
(WebCore::HTMLAnchorElement::parseMappedAttribute):
* html/HTMLAppletElement.cpp:
(WebCore::HTMLAppletElement::parseMappedAttribute):
* html/HTMLElement.cpp:
(WebCore::HTMLElement::parseMappedAttribute):
* html/HTMLEmbedElement.cpp:
(WebCore::HTMLEmbedElement::parseMappedAttribute):
* html/HTMLFormElement.cpp:
(WebCore::HTMLFormElement::parseMappedAttribute):
* html/HTMLFrameElementBase.cpp:
(WebCore::HTMLFrameElementBase::parseMappedAttribute):
* html/HTMLIFrameElement.cpp:
(WebCore::HTMLIFrameElement::parseMappedAttribute):
* html/HTMLImageElement.cpp:
(WebCore::HTMLImageElement::parseMappedAttribute):
* html/HTMLMapElement.cpp:
(WebCore::HTMLMapElement::parseMappedAttribute):
* html/HTMLMetaElement.cpp:
(WebCore::HTMLMetaElement::parseMappedAttribute):
* html/HTMLObjectElement.cpp:
(WebCore::HTMLObjectElement::parseMappedAttribute):
* html/HTMLParamElement.cpp:
(WebCore::HTMLParamElement::parseMappedAttribute):

LayoutTests:

Add a regression test for setting Attr's value. WebKit should invalidate the cache as needed.

* fast/dom/Attr/invalidate-nodelist-after-attr-setvalue-expected.txt: Added.
* fast/dom/Attr/invalidate-nodelist-after-attr-setvalue.html: Added.

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

22 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/Attr/invalidate-nodelist-after-attr-setvalue-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Attr/invalidate-nodelist-after-attr-setvalue.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Attr.cpp
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/NamedNodeMap.cpp
Source/WebCore/dom/Node.cpp
Source/WebCore/dom/Node.h
Source/WebCore/dom/StyledElement.cpp
Source/WebCore/html/HTMLAnchorElement.cpp
Source/WebCore/html/HTMLAppletElement.cpp
Source/WebCore/html/HTMLElement.cpp
Source/WebCore/html/HTMLEmbedElement.cpp
Source/WebCore/html/HTMLFormElement.cpp
Source/WebCore/html/HTMLFrameElementBase.cpp
Source/WebCore/html/HTMLIFrameElement.cpp
Source/WebCore/html/HTMLImageElement.cpp
Source/WebCore/html/HTMLMapElement.cpp
Source/WebCore/html/HTMLMetaElement.cpp
Source/WebCore/html/HTMLObjectElement.cpp
Source/WebCore/html/HTMLParamElement.cpp

index 626205f297dc41de34f8da9fbb10f0c9d62de78b..545c4fe51de2b335c657b6a29c1c4459d46e2192 100644 (file)
@@ -1,3 +1,15 @@
+2011-12-16  Ryosuke Niwa  <rniwa@webkit.org>
+
+        invalidateNodeListsCacheAfterAttributeChanged has too many callers
+        https://bugs.webkit.org/show_bug.cgi?id=74692
+
+        Reviewed by Sam Weinig.
+
+        Add a regression test for setting Attr's value. WebKit should invalidate the cache as needed.
+
+        * fast/dom/Attr/invalidate-nodelist-after-attr-setvalue-expected.txt: Added.
+        * fast/dom/Attr/invalidate-nodelist-after-attr-setvalue.html: Added.
+
 2011-12-16  Andreas Kling  <kling@webkit.org>
 
         Cache and reuse HTMLCollections exposed by Document.
diff --git a/LayoutTests/fast/dom/Attr/invalidate-nodelist-after-attr-setvalue-expected.txt b/LayoutTests/fast/dom/Attr/invalidate-nodelist-after-attr-setvalue-expected.txt
new file mode 100644 (file)
index 0000000..004eeb5
--- /dev/null
@@ -0,0 +1,12 @@
+This tests setting the value of an attribute node after caching childNodes of the attribute node.
+The cache should be cleared and childNodes[0].data should return the new value.
+You should see PASS below:
+
+PASS nameAttrNode.childNodes.length is 1
+PASS nameAttrNode.childNodes[0] is oldValue
+PASS nameAttrNode.childNodes[0].data is "oldName"
+
+PASS nameAttrNode.value = 'newName'; nameAttrNode.value is "newName"
+PASS nameAttrNode.childNodes[0] is not oldValue
+PASS nameAttrNode.childNodes[0].data is "newName"
+
diff --git a/LayoutTests/fast/dom/Attr/invalidate-nodelist-after-attr-setvalue.html b/LayoutTests/fast/dom/Attr/invalidate-nodelist-after-attr-setvalue.html
new file mode 100644 (file)
index 0000000..884343c
--- /dev/null
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests setting the value of an attribute node after caching childNodes of the attribute node.<br>
+The cache should be cleared and childNodes[0].data should return the new value.<br>
+You should see PASS below:</p>
+<div id="console"></div>
+<script src="../../js/resources/js-test-pre.js"></script>
+<script>
+
+var element = document.createElement('div');
+var nameAttrNode = document.createAttribute('name');
+var oldValue = document.createTextNode('oldName');
+nameAttrNode.appendChild(oldValue);
+element.setAttributeNode(nameAttrNode);
+document.body.appendChild(element);
+
+shouldBe("nameAttrNode.childNodes.length", '1');
+shouldBe('nameAttrNode.childNodes[0]', 'oldValue');
+shouldBe('nameAttrNode.childNodes[0].data', '"oldName"');
+
+debug('');
+shouldBe("nameAttrNode.value = 'newName'; nameAttrNode.value", '"newName"');
+shouldNotBe("nameAttrNode.childNodes[0]", 'oldValue');
+shouldBe("nameAttrNode.childNodes[0].data", '"newName"');
+
+</script>
+</body>
+</html>
index 95fcb3667fc717c30d4e60711700bfdc49a136d1..01cdd65f3ba6d7133948fcd9ccec37815d0055b9 100644 (file)
@@ -1,3 +1,56 @@
+2011-12-16  Ryosuke Niwa  <rniwa@webkit.org>
+
+        invalidateNodeListsCacheAfterAttributeChanged has too many callers
+        https://bugs.webkit.org/show_bug.cgi?id=74692
+
+        Reviewed by Sam Weinig.
+
+        Call invalidateNodeListsCacheAfterAttributeChanged in Element::updateAfterAttributeChanged instead of
+        parsedMappedAttribute of various elements. Also make invalidateNodeListsCacheAfterAttributeChanged take
+        the qualified name of the changed attribute so that we can exit early when the changed attribute isn't
+        one of attributes we care.
+
+        In addition, added a missing call to invalidateNodeListsCacheAfterAttributeChanged in Attr::setValue.
+
+        Test: fast/dom/Attr/invalidate-nodelist-after-attr-setvalue.html
+
+        * dom/Attr.cpp:
+        (WebCore::Attr::childrenChanged):
+        * dom/Element.cpp:
+        (WebCore::Element::updateAfterAttributeChanged):
+        * dom/NamedNodeMap.cpp:
+        (WebCore::NamedNodeMap::addAttribute):
+        (WebCore::NamedNodeMap::removeAttribute):
+        * dom/Node.cpp:
+        (WebCore::Node::invalidateNodeListsCacheAfterAttributeChanged):
+        * dom/Node.h:
+        * dom/StyledElement.cpp:
+        (WebCore::StyledElement::classAttributeChanged):
+        * html/HTMLAnchorElement.cpp:
+        (WebCore::HTMLAnchorElement::parseMappedAttribute):
+        * html/HTMLAppletElement.cpp:
+        (WebCore::HTMLAppletElement::parseMappedAttribute):
+        * html/HTMLElement.cpp:
+        (WebCore::HTMLElement::parseMappedAttribute):
+        * html/HTMLEmbedElement.cpp:
+        (WebCore::HTMLEmbedElement::parseMappedAttribute):
+        * html/HTMLFormElement.cpp:
+        (WebCore::HTMLFormElement::parseMappedAttribute):
+        * html/HTMLFrameElementBase.cpp:
+        (WebCore::HTMLFrameElementBase::parseMappedAttribute):
+        * html/HTMLIFrameElement.cpp:
+        (WebCore::HTMLIFrameElement::parseMappedAttribute):
+        * html/HTMLImageElement.cpp:
+        (WebCore::HTMLImageElement::parseMappedAttribute):
+        * html/HTMLMapElement.cpp:
+        (WebCore::HTMLMapElement::parseMappedAttribute):
+        * html/HTMLMetaElement.cpp:
+        (WebCore::HTMLMetaElement::parseMappedAttribute):
+        * html/HTMLObjectElement.cpp:
+        (WebCore::HTMLObjectElement::parseMappedAttribute):
+        * html/HTMLParamElement.cpp:
+        (WebCore::HTMLParamElement::parseMappedAttribute):
+
 2011-12-16  Andreas Kling  <kling@webkit.org>
 
         Cache and reuse HTMLCollections exposed by Document.
index db69eb34670254bd19a371e702e99ef4b04669fa..7038490119d8fd7f4e2dce0f5ce73a5f0b46b866 100644 (file)
@@ -126,6 +126,8 @@ void Attr::setValue(const AtomicString& value)
     m_attribute->setValue(value);
     createTextChild();
     m_ignoreChildrenChanged--;
+
+    invalidateNodeListsCacheAfterAttributeChanged(m_attribute->name());
 }
 
 void Attr::setValue(const AtomicString& value, ExceptionCode&)
@@ -180,10 +182,10 @@ void Attr::childrenChanged(bool changedByParser, Node* beforeChange, Node* after
 
     Node::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
 
-    invalidateNodeListsCacheAfterAttributeChanged();
+    invalidateNodeListsCacheAfterAttributeChanged(m_attribute->name());
 
     // FIXME: We should include entity references in the value
-    
+
     String val = "";
     for (Node *n = firstChild(); n; n = n->nextSibling()) {
         if (n->isTextNode())
index 5651340688b83247ca685d25b93d434e1d2d1359..352771225aac23b40de94192902d8f24f82295c6 100644 (file)
@@ -697,6 +697,8 @@ void Element::attributeChanged(Attribute* attr, bool)
 
 void Element::updateAfterAttributeChanged(Attribute* attr)
 {
+    invalidateNodeListsCacheAfterAttributeChanged(attr->name());
+
     if (!AXObjectCache::accessibilityEnabled())
         return;
 
index cdc8089040bfb5bc4011dfe38aa33a9a2aa70c52..c6288b42b7314ebb89c63595be618bbd97c25c4e 100644 (file)
@@ -264,7 +264,6 @@ void NamedNodeMap::addAttribute(PassRefPtr<Attribute> prpAttribute)
         m_element->attributeChanged(attribute.get());
         // Because of our updateStyleAttribute() style modification events are never sent at the right time, so don't bother sending them.
         if (attribute->name() != styleAttr) {
-            m_element->invalidateNodeListsCacheAfterAttributeChanged();
             m_element->dispatchAttrAdditionEvent(attribute.get());
             m_element->dispatchSubtreeModifiedEvent();
         }
@@ -301,7 +300,6 @@ void NamedNodeMap::removeAttribute(const QualifiedName& name)
         attr->m_value = value;
     }
     if (m_element) {
-        m_element->invalidateNodeListsCacheAfterAttributeChanged();
         m_element->dispatchAttrRemovalEvent(attr.get());
         m_element->dispatchSubtreeModifiedEvent();
     }
index 94c6d8e701cdc6701e7e50cba51e2af680bef2bc..253ebdb84d21b5fe51bf211dd10f56ee8eaf35ba 100644 (file)
@@ -1020,7 +1020,7 @@ void Node::unregisterDynamicSubtreeNodeList(DynamicSubtreeNodeList* list)
     removeNodeListCacheIfPossible(this, data);
 }
 
-void Node::invalidateNodeListsCacheAfterAttributeChanged()
+void Node::invalidateNodeListsCacheAfterAttributeChanged(const QualifiedName& attrName)
 {
     if (hasRareData() && isAttributeNode()) {
         NodeRareData* data = rareData();
@@ -1028,6 +1028,15 @@ void Node::invalidateNodeListsCacheAfterAttributeChanged()
         data->clearChildNodeListCache();
     }
 
+    // This list should be sync'ed with NodeListsNodeData.
+    if (attrName != classAttr
+#if ENABLE(MICRODATA)
+        && attrName != itemscopeAttr
+        && attrName != itempropAttr
+#endif
+        && attrName != nameAttr)
+        return;
+
     if (!treeScope()->hasNodeListCaches())
         return;
 
index da8952bc79bfc2a77bd8d3e331ab2979da83b9dd..21955ad69fade14c373c3cdd13ca0755703cb2d3 100644 (file)
@@ -519,7 +519,7 @@ public:
 
     void registerDynamicSubtreeNodeList(DynamicSubtreeNodeList*);
     void unregisterDynamicSubtreeNodeList(DynamicSubtreeNodeList*);
-    void invalidateNodeListsCacheAfterAttributeChanged();
+    void invalidateNodeListsCacheAfterAttributeChanged(const QualifiedName&);
     void invalidateNodeListsCacheAfterChildrenChanged();
     void notifyLocalNodeListsLabelChanged();
     void removeCachedClassNodeList(ClassNodeList*, const String&);
index 54f473b639ce9e6c09eae1b46e88df7ef87f2aa2..6bea76ebe704d6c4e254f9ed437100b5cc21ee1b 100644 (file)
@@ -228,7 +228,6 @@ void StyledElement::classAttributeChanged(const AtomicString& newClassString)
     } else if (attributeMap())
         attributeMap()->clearClass();
     setNeedsStyleRecalc();
-    invalidateNodeListsCacheAfterAttributeChanged();
     dispatchSubtreeModifiedEvent();
 }
 
index fdb84d9def2e00709c55fc6d3245b8a2b4ed63d7..d601852825d6843d38bf77d3a0116727171b34b8 100644 (file)
@@ -223,9 +223,7 @@ void HTMLAnchorElement::parseMappedAttribute(Attribute* attr)
             }
         }
         invalidateCachedVisitedLinkHash();
-    } else if (attr->name() == nameAttr) {
-        invalidateNodeListsCacheAfterAttributeChanged();
-    } else if (attr->name() == titleAttr) {
+    } else if (attr->name() == nameAttr || attr->name() == titleAttr) {
         // Do nothing.
     } else if (attr->name() == relAttr)
         setRel(attr->value());
index 79da2f07f9533de775b9c258b50bddb887289a4d..cf6cc848aab46566fe88348b6ec51e46fbf3ba7e 100644 (file)
@@ -64,7 +64,6 @@ void HTMLAppletElement::parseMappedAttribute(Attribute* attr)
             document->addNamedItem(newName);
         }
         m_name = newName;
-        invalidateNodeListsCacheAfterAttributeChanged();
     } else if (isIdAttributeName(attr->name())) {
         const AtomicString& newId = attr->value();
         if (inDocument() && document()->isHTMLDocument()) {
index ad9e46e0e809e919d5abbb94142f693e6fe18977..f3106c3f4222db4c3bdcbd45a54ccdcb1ba51b7e 100644 (file)
@@ -210,8 +210,6 @@ void HTMLElement::parseMappedAttribute(Attribute* attr)
             addCSSProperty(attr, CSSPropertyWebkitUserSelect, CSSValueNone);
         } else if (equalIgnoringCase(value, "false"))
             addCSSProperty(attr, CSSPropertyWebkitUserDrag, CSSValueNone);
-    } else if (attr->name() == nameAttr) {
-        invalidateNodeListsCacheAfterAttributeChanged();
 #if ENABLE(MICRODATA)
     } else if (attr->name() == itempropAttr) {
         setItemProp(attr->value());
index fd412499d6782094b7bc3e4e29a1920fd4cc00fb..4d0a21559548128010b228aeefd0d32b37f5891e 100644 (file)
@@ -118,7 +118,6 @@ void HTMLEmbedElement::parseMappedAttribute(Attribute* attr)
             document->addNamedItem(value);
         }
         m_name = value;
-        invalidateNodeListsCacheAfterAttributeChanged();
     } else
         HTMLPlugInImageElement::parseMappedAttribute(attr);
 }
index 2ea9100f4a7b321e7f050c8ec07bed4ae796851f..556bf21523a31096cb8e8a54aac1d0f522d8794e 100644 (file)
@@ -392,7 +392,6 @@ void HTMLFormElement::parseMappedAttribute(Attribute* attr)
             document->addNamedItem(newName);
         }
         m_name = newName;
-        invalidateNodeListsCacheAfterAttributeChanged();
     } else
         HTMLElement::parseMappedAttribute(attr);
 }
index ebefd9a6b88f6d375b725bb9d7deca0d5a7a2ddc..f5d22a418d19fb64d1ec5d79c595e79ce95b18ea 100644 (file)
@@ -141,7 +141,6 @@ void HTMLFrameElementBase::parseMappedAttribute(Attribute* attr)
         m_frameName = attr->value();
     } else if (attr->name() == nameAttr) {
         m_frameName = attr->value();
-        invalidateNodeListsCacheAfterAttributeChanged();
         // FIXME: If we are already attached, this doesn't actually change the frame's name.
         // FIXME: If we are already attached, this doesn't check for frame name
         // conflicts and generate a unique frame name.
index bd8fec4f0eafc14b00400557b84e55742a2b87ab..ae60a1e748dc151538bbd9b5c67175e813aa319a 100644 (file)
@@ -84,7 +84,6 @@ void HTMLIFrameElement::parseMappedAttribute(Attribute* attr)
             document->addExtraNamedItem(newName);
         }
         m_name = newName;
-        invalidateNodeListsCacheAfterAttributeChanged();
     } else if (attr->name() == frameborderAttr) {
         // Frame border doesn't really match the HTML4 spec definition for iframes.  It simply adds
         // a presentational hint that the border should be off if set to zero.
index 32f35ec7591cd87f952d7cf0fa2e2cd0c37f3b79..42a4e7873391da6350e95f13ebcc7bee96937e44 100644 (file)
@@ -141,7 +141,6 @@ void HTMLImageElement::parseMappedAttribute(Attribute* attr)
             document->addNamedItem(newName);
         }
         m_name = newName;
-        invalidateNodeListsCacheAfterAttributeChanged();
     } else if (isIdAttributeName(attr->name())) {
         const AtomicString& newId = attr->value();
         if (inDocument() && document()->isHTMLDocument()) {
index 25b5158a3658d48a974e795bd51bb37c9a1a1d0b..9db15426d0ab260d8db485379891a9b8e52a0267 100644 (file)
@@ -120,8 +120,6 @@ void HTMLMapElement::parseMappedAttribute(Attribute* attribute)
         if (inDocument())
             treeScope()->addImageMap(this);
 
-        if (attrName == nameAttr)
-            invalidateNodeListsCacheAfterAttributeChanged();
         return;
     }
 
index f2c8d05b75b74919ed800fc718d3ee4da1298c09..c27b9ba237cf7cf401b03f6a89391b2a68a3f21b 100644 (file)
@@ -49,7 +49,7 @@ void HTMLMetaElement::parseMappedAttribute(Attribute* attr)
     else if (attr->name() == contentAttr)
         process();
     else if (attr->name() == nameAttr) {
-        invalidateNodeListsCacheAfterAttributeChanged();
+        // Do nothing
     } else
         HTMLElement::parseMappedAttribute(attr);
 }
index 431e7034dc690a020c4543501e307376ffe64e58..a9e1f04b166d0ec917a8a64761695df30d85854c 100644 (file)
@@ -122,7 +122,6 @@ void HTMLObjectElement::parseMappedAttribute(Attribute* attr)
             document->addNamedItem(newName);
         }
         m_name = newName;
-        invalidateNodeListsCacheAfterAttributeChanged();
     } else if (attr->name() == borderAttr)
         applyBorderAttribute(attr);
     else if (isIdAttributeName(attr->name())) {
index 78e8132691d0babe841f965c9bc0dc093784fb45..d1f53a14db741b61988595e0732458534c73fac2 100644 (file)
@@ -57,7 +57,6 @@ void HTMLParamElement::parseMappedAttribute(Attribute* attr)
         m_name = attr->value();
     } else if (attr->name() == nameAttr) {
         m_name = attr->value();
-        invalidateNodeListsCacheAfterAttributeChanged();
     } else if (attr->name() == valueAttr) {
         m_value = attr->value();
     } else