Diff
Modified: trunk/LayoutTests/ChangeLog (103115 => 103116)
--- trunk/LayoutTests/ChangeLog 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/LayoutTests/ChangeLog 2011-12-16 23:15:10 UTC (rev 103116)
@@ -1,3 +1,15 @@
+2011-12-16 Ryosuke Niwa <[email protected]>
+
+ 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 <[email protected]>
Cache and reuse HTMLCollections exposed by Document.
Added: trunk/LayoutTests/fast/dom/Attr/invalidate-nodelist-after-attr-setvalue-expected.txt (0 => 103116)
--- trunk/LayoutTests/fast/dom/Attr/invalidate-nodelist-after-attr-setvalue-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/Attr/invalidate-nodelist-after-attr-setvalue-expected.txt 2011-12-16 23:15:10 UTC (rev 103116)
@@ -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"
+
Added: trunk/LayoutTests/fast/dom/Attr/invalidate-nodelist-after-attr-setvalue.html (0 => 103116)
--- trunk/LayoutTests/fast/dom/Attr/invalidate-nodelist-after-attr-setvalue.html (rev 0)
+++ trunk/LayoutTests/fast/dom/Attr/invalidate-nodelist-after-attr-setvalue.html 2011-12-16 23:15:10 UTC (rev 103116)
@@ -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=""
+<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>
Modified: trunk/Source/WebCore/ChangeLog (103115 => 103116)
--- trunk/Source/WebCore/ChangeLog 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/ChangeLog 2011-12-16 23:15:10 UTC (rev 103116)
@@ -1,3 +1,56 @@
+2011-12-16 Ryosuke Niwa <[email protected]>
+
+ 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 <[email protected]>
Cache and reuse HTMLCollections exposed by Document.
Modified: trunk/Source/WebCore/dom/Attr.cpp (103115 => 103116)
--- trunk/Source/WebCore/dom/Attr.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/dom/Attr.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -126,6 +126,8 @@
m_attribute->setValue(value);
createTextChild();
m_ignoreChildrenChanged--;
+
+ invalidateNodeListsCacheAfterAttributeChanged(m_attribute->name());
}
void Attr::setValue(const AtomicString& value, ExceptionCode&)
@@ -180,10 +182,10 @@
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())
Modified: trunk/Source/WebCore/dom/Element.cpp (103115 => 103116)
--- trunk/Source/WebCore/dom/Element.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/dom/Element.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -697,6 +697,8 @@
void Element::updateAfterAttributeChanged(Attribute* attr)
{
+ invalidateNodeListsCacheAfterAttributeChanged(attr->name());
+
if (!AXObjectCache::accessibilityEnabled())
return;
Modified: trunk/Source/WebCore/dom/NamedNodeMap.cpp (103115 => 103116)
--- trunk/Source/WebCore/dom/NamedNodeMap.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/dom/NamedNodeMap.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -264,7 +264,6 @@
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 @@
attr->m_value = value;
}
if (m_element) {
- m_element->invalidateNodeListsCacheAfterAttributeChanged();
m_element->dispatchAttrRemovalEvent(attr.get());
m_element->dispatchSubtreeModifiedEvent();
}
Modified: trunk/Source/WebCore/dom/Node.cpp (103115 => 103116)
--- trunk/Source/WebCore/dom/Node.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/dom/Node.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -1020,7 +1020,7 @@
removeNodeListCacheIfPossible(this, data);
}
-void Node::invalidateNodeListsCacheAfterAttributeChanged()
+void Node::invalidateNodeListsCacheAfterAttributeChanged(const QualifiedName& attrName)
{
if (hasRareData() && isAttributeNode()) {
NodeRareData* data = ""
@@ -1028,6 +1028,15 @@
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;
Modified: trunk/Source/WebCore/dom/Node.h (103115 => 103116)
--- trunk/Source/WebCore/dom/Node.h 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/dom/Node.h 2011-12-16 23:15:10 UTC (rev 103116)
@@ -519,7 +519,7 @@
void registerDynamicSubtreeNodeList(DynamicSubtreeNodeList*);
void unregisterDynamicSubtreeNodeList(DynamicSubtreeNodeList*);
- void invalidateNodeListsCacheAfterAttributeChanged();
+ void invalidateNodeListsCacheAfterAttributeChanged(const QualifiedName&);
void invalidateNodeListsCacheAfterChildrenChanged();
void notifyLocalNodeListsLabelChanged();
void removeCachedClassNodeList(ClassNodeList*, const String&);
Modified: trunk/Source/WebCore/dom/StyledElement.cpp (103115 => 103116)
--- trunk/Source/WebCore/dom/StyledElement.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/dom/StyledElement.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -228,7 +228,6 @@
} else if (attributeMap())
attributeMap()->clearClass();
setNeedsStyleRecalc();
- invalidateNodeListsCacheAfterAttributeChanged();
dispatchSubtreeModifiedEvent();
}
Modified: trunk/Source/WebCore/html/HTMLAnchorElement.cpp (103115 => 103116)
--- trunk/Source/WebCore/html/HTMLAnchorElement.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/html/HTMLAnchorElement.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -223,9 +223,7 @@
}
}
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());
Modified: trunk/Source/WebCore/html/HTMLAppletElement.cpp (103115 => 103116)
--- trunk/Source/WebCore/html/HTMLAppletElement.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/html/HTMLAppletElement.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -64,7 +64,6 @@
document->addNamedItem(newName);
}
m_name = newName;
- invalidateNodeListsCacheAfterAttributeChanged();
} else if (isIdAttributeName(attr->name())) {
const AtomicString& newId = attr->value();
if (inDocument() && document()->isHTMLDocument()) {
Modified: trunk/Source/WebCore/html/HTMLElement.cpp (103115 => 103116)
--- trunk/Source/WebCore/html/HTMLElement.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/html/HTMLElement.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -210,8 +210,6 @@
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());
Modified: trunk/Source/WebCore/html/HTMLEmbedElement.cpp (103115 => 103116)
--- trunk/Source/WebCore/html/HTMLEmbedElement.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/html/HTMLEmbedElement.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -118,7 +118,6 @@
document->addNamedItem(value);
}
m_name = value;
- invalidateNodeListsCacheAfterAttributeChanged();
} else
HTMLPlugInImageElement::parseMappedAttribute(attr);
}
Modified: trunk/Source/WebCore/html/HTMLFormElement.cpp (103115 => 103116)
--- trunk/Source/WebCore/html/HTMLFormElement.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/html/HTMLFormElement.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -392,7 +392,6 @@
document->addNamedItem(newName);
}
m_name = newName;
- invalidateNodeListsCacheAfterAttributeChanged();
} else
HTMLElement::parseMappedAttribute(attr);
}
Modified: trunk/Source/WebCore/html/HTMLFrameElementBase.cpp (103115 => 103116)
--- trunk/Source/WebCore/html/HTMLFrameElementBase.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/html/HTMLFrameElementBase.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -141,7 +141,6 @@
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.
Modified: trunk/Source/WebCore/html/HTMLIFrameElement.cpp (103115 => 103116)
--- trunk/Source/WebCore/html/HTMLIFrameElement.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/html/HTMLIFrameElement.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -84,7 +84,6 @@
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.
Modified: trunk/Source/WebCore/html/HTMLImageElement.cpp (103115 => 103116)
--- trunk/Source/WebCore/html/HTMLImageElement.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/html/HTMLImageElement.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -141,7 +141,6 @@
document->addNamedItem(newName);
}
m_name = newName;
- invalidateNodeListsCacheAfterAttributeChanged();
} else if (isIdAttributeName(attr->name())) {
const AtomicString& newId = attr->value();
if (inDocument() && document()->isHTMLDocument()) {
Modified: trunk/Source/WebCore/html/HTMLMapElement.cpp (103115 => 103116)
--- trunk/Source/WebCore/html/HTMLMapElement.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/html/HTMLMapElement.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -120,8 +120,6 @@
if (inDocument())
treeScope()->addImageMap(this);
- if (attrName == nameAttr)
- invalidateNodeListsCacheAfterAttributeChanged();
return;
}
Modified: trunk/Source/WebCore/html/HTMLMetaElement.cpp (103115 => 103116)
--- trunk/Source/WebCore/html/HTMLMetaElement.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/html/HTMLMetaElement.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -49,7 +49,7 @@
else if (attr->name() == contentAttr)
process();
else if (attr->name() == nameAttr) {
- invalidateNodeListsCacheAfterAttributeChanged();
+ // Do nothing
} else
HTMLElement::parseMappedAttribute(attr);
}
Modified: trunk/Source/WebCore/html/HTMLObjectElement.cpp (103115 => 103116)
--- trunk/Source/WebCore/html/HTMLObjectElement.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/html/HTMLObjectElement.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -122,7 +122,6 @@
document->addNamedItem(newName);
}
m_name = newName;
- invalidateNodeListsCacheAfterAttributeChanged();
} else if (attr->name() == borderAttr)
applyBorderAttribute(attr);
else if (isIdAttributeName(attr->name())) {
Modified: trunk/Source/WebCore/html/HTMLParamElement.cpp (103115 => 103116)
--- trunk/Source/WebCore/html/HTMLParamElement.cpp 2011-12-16 23:11:11 UTC (rev 103115)
+++ trunk/Source/WebCore/html/HTMLParamElement.cpp 2011-12-16 23:15:10 UTC (rev 103116)
@@ -57,7 +57,6 @@
m_name = attr->value();
} else if (attr->name() == nameAttr) {
m_name = attr->value();
- invalidateNodeListsCacheAfterAttributeChanged();
} else if (attr->name() == valueAttr) {
m_value = attr->value();
} else
