Title: [150072] trunk
- Revision
- 150072
- Author
- rn...@webkit.org
- Date
- 2013-05-14 09:52:30 -0700 (Tue, 14 May 2013)
Log Message
Removing Attr can delete a wrong Attribute in ElementData
https://bugs.webkit.org/show_bug.cgi?id=116077
Source/WebCore:
Reviewed by Benjamin Poulain.
Merge https://chromium.googlesource.com/chromium/blink/+/e861452a292e185501e48940305947aa6a4e23c2
after simplifying and renaming functions to be more WebKit style.
The XML parser can produce elements with attributes whose names have
distinct prefixes, but the same expanded name. When one of these
attributes is put up for adoption, it may be its similarly named
sibling that is removed from its owner element. As a result the
original owner hangs onto the adopted attribute, despite the fact that
it is now in a different document. Sometimes it's just hard to let go.
Test: fast/dom/adopt-attribute-crash.svg
* dom/Element.cpp:
(WebCore::Element::setAttributeNode):
(WebCore::Element::removeAttributeNode):
(WebCore::ElementData::getAttributeItemIndex):
* dom/Element.h:
(ElementData):
(Element):
LayoutTests:
Reviewed by Benjamin Poulain.
Add a regression test by importing
https://chromium.googlesource.com/chromium/blink/+/e861452a292e185501e48940305947aa6a4e23c2
* fast/dom/adopt-attribute-crash-expected.txt: Added.
* fast/dom/adopt-attribute-crash.svg: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (150071 => 150072)
--- trunk/LayoutTests/ChangeLog 2013-05-14 16:17:09 UTC (rev 150071)
+++ trunk/LayoutTests/ChangeLog 2013-05-14 16:52:30 UTC (rev 150072)
@@ -1,3 +1,16 @@
+2013-05-13 Ryosuke Niwa <rn...@webkit.org>
+
+ Removing Attr can delete a wrong Attribute in ElementData
+ https://bugs.webkit.org/show_bug.cgi?id=116077
+
+ Reviewed by Benjamin Poulain.
+
+ Add a regression test by importing
+ https://chromium.googlesource.com/chromium/blink/+/e861452a292e185501e48940305947aa6a4e23c2
+
+ * fast/dom/adopt-attribute-crash-expected.txt: Added.
+ * fast/dom/adopt-attribute-crash.svg: Added.
+
2013-05-14 Andrei Bucur <abu...@adobe.com>
Modify checkLayout to receive the log container as an optional parameter
Added: trunk/LayoutTests/fast/dom/adopt-attribute-crash-expected.txt (0 => 150072)
--- trunk/LayoutTests/fast/dom/adopt-attribute-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/adopt-attribute-crash-expected.txt 2013-05-14 16:52:30 UTC (rev 150072)
@@ -0,0 +1,8 @@
+ALERT: Tests adopting two Attr nodes of the same qualified name. WebKit shouldn't crash or assert.
+This page contains the following errors:
+
+error on line 3 at column 35: Namespaced Attribute href in 'http://www.w3.org/1999/xlink' redefined
+error on line 18 at column 10: Extra content at the end of the document
+Below is a rendering of the page up to the first error.
+
+
Added: trunk/LayoutTests/fast/dom/adopt-attribute-crash.svg (0 => 150072)
--- trunk/LayoutTests/fast/dom/adopt-attribute-crash.svg (rev 0)
+++ trunk/LayoutTests/fast/dom/adopt-attribute-crash.svg 2013-05-14 16:52:30 UTC (rev 150072)
@@ -0,0 +1,18 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<svg xmlns:xl="http://www.w3.org/1999/xlink" xmlns:xlink="http://www.w3.org/1999/xlink">
+<linearGradient xl:href="" xlink:href=""
+</svg>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+var element = document.querySelector('linearGradient');
+var attr1 = element.attributes[0];
+var attr2 = element.attributes[1];
+var doc2 = document.implementation.createHTMLDocument('Doc 2');
+var attr3 = doc2.adoptNode(attr2);
+var doc3 = document.implementation.createHTMLDocument('Doc 3');
+var attr4 = doc3.adoptNode(attr3);
+
+alert("Tests adopting two Attr nodes of the same qualified name. WebKit shouldn't crash or assert.");
+</script>
Modified: trunk/Source/WebCore/ChangeLog (150071 => 150072)
--- trunk/Source/WebCore/ChangeLog 2013-05-14 16:17:09 UTC (rev 150071)
+++ trunk/Source/WebCore/ChangeLog 2013-05-14 16:52:30 UTC (rev 150072)
@@ -1,3 +1,30 @@
+2013-05-13 Ryosuke Niwa <rn...@webkit.org>
+
+ Removing Attr can delete a wrong Attribute in ElementData
+ https://bugs.webkit.org/show_bug.cgi?id=116077
+
+ Reviewed by Benjamin Poulain.
+
+ Merge https://chromium.googlesource.com/chromium/blink/+/e861452a292e185501e48940305947aa6a4e23c2
+ after simplifying and renaming functions to be more WebKit style.
+
+ The XML parser can produce elements with attributes whose names have
+ distinct prefixes, but the same expanded name. When one of these
+ attributes is put up for adoption, it may be its similarly named
+ sibling that is removed from its owner element. As a result the
+ original owner hangs onto the adopted attribute, despite the fact that
+ it is now in a different document. Sometimes it's just hard to let go.
+
+ Test: fast/dom/adopt-attribute-crash.svg
+
+ * dom/Element.cpp:
+ (WebCore::Element::setAttributeNode):
+ (WebCore::Element::removeAttributeNode):
+ (WebCore::ElementData::getAttributeItemIndex):
+ * dom/Element.h:
+ (ElementData):
+ (Element):
+
2013-05-14 Antti Koivisto <an...@apple.com>
Remove ::-webkit-distributed()
Modified: trunk/Source/WebCore/dom/Element.cpp (150071 => 150072)
--- trunk/Source/WebCore/dom/Element.cpp 2013-05-14 16:17:09 UTC (rev 150071)
+++ trunk/Source/WebCore/dom/Element.cpp 2013-05-14 16:52:30 UTC (rev 150072)
@@ -1729,7 +1729,7 @@
synchronizeAllAttributes();
UniqueElementData* elementData = ensureUniqueElementData();
- unsigned index = elementData->getAttributeItemIndex(attrNode->qualifiedName());
+ unsigned index = elementData->getAttributeItemIndexForAttributeNode(attrNode);
if (index != ElementData::attributeNotFound) {
if (oldAttrNode)
detachAttrNodeFromElementWithValue(oldAttrNode.get(), elementData->attributeItem(index)->value());
@@ -1765,13 +1765,16 @@
synchronizeAttribute(attr->qualifiedName());
- unsigned index = elementData()->getAttributeItemIndex(attr->qualifiedName());
+ unsigned index = elementData()->getAttributeItemIndexForAttributeNode(attr);
if (index == ElementData::attributeNotFound) {
ec = NOT_FOUND_ERR;
return 0;
}
- return detachAttribute(index);
+ RefPtr<Attr> attrNode = attr;
+ detachAttrNodeFromElementWithValue(attr, elementData()->attributeItem(index)->value());
+ removeAttributeInternal(index, NotInSynchronizationOfLazyAttribute);
+ return attrNode.release();
}
bool Element::parseAttributeName(QualifiedName& out, const AtomicString& namespaceURI, const AtomicString& qualifiedName, ExceptionCode& ec)
@@ -3140,9 +3143,22 @@
return attributeNotFound;
}
+unsigned ElementData::getAttributeItemIndexForAttributeNode(const Attr* attr) const
+{
+ ASSERT(attr);
+ const Attribute* attributes = attributeBase();
+ unsigned count = length();
+ for (unsigned i = 0; i < count; ++i) {
+ if (attributes[i].name() == attr->qualifiedName())
+ return i;
+ }
+ return attributeNotFound;
+}
+
Attribute* UniqueElementData::getAttributeItem(const QualifiedName& name)
{
- for (unsigned i = 0; i < length(); ++i) {
+ unsigned count = length();
+ for (unsigned i = 0; i < count; ++i) {
if (m_attributeVector.at(i).name().matches(name))
return &m_attributeVector.at(i);
}
Modified: trunk/Source/WebCore/dom/Element.h (150071 => 150072)
--- trunk/Source/WebCore/dom/Element.h 2013-05-14 16:17:09 UTC (rev 150071)
+++ trunk/Source/WebCore/dom/Element.h 2013-05-14 16:52:30 UTC (rev 150072)
@@ -78,6 +78,7 @@
const Attribute* getAttributeItem(const QualifiedName&) const;
unsigned getAttributeItemIndex(const QualifiedName&) const;
unsigned getAttributeItemIndex(const AtomicString& name, bool shouldIgnoreAttributeCase) const;
+ unsigned getAttributeItemIndexForAttributeNode(const Attr*) const;
bool hasID() const { return !m_idForStyleResolution.isNull(); }
bool hasClass() const { return !m_classNames.isNull(); }
@@ -729,6 +730,7 @@
void detachAllAttrNodesFromElement();
void detachAttrNodeFromElementWithValue(Attr*, const AtomicString& value);
+ void detachAttrNodeAtIndex(Attr*, size_t index);
void createRendererIfNeeded();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
