- Revision
- 136996
- Author
- [email protected]
- Date
- 2012-12-07 15:43:48 -0800 (Fri, 07 Dec 2012)
Log Message
MutationRecord addedNodes/removedNodes should never be null
https://bugs.webkit.org/show_bug.cgi?id=98921
Reviewed by Ryosuke Niwa.
Source/WebCore:
Per an update to the DOM4 spec that matches Gecko's behavior,
addedNodes/removedNodes should be empty NodeLists on 'attributes'
and 'characterData' records, rather than null.
This is accomplished with lazy initialization of addedNodes/removedNodes
attributes on 'attributes'/'characterData' records and the
addition of a new StaticNodeList::createEmpty() factory method.
* dom/MutationRecord.cpp:
* dom/MutationRecord.h:
(MutationRecord):
* dom/StaticNodeList.h:
(WebCore::StaticNodeList::adopt):
(StaticNodeList):
(WebCore::StaticNodeList::createEmpty):
(WebCore::StaticNodeList::StaticNodeList):
LayoutTests:
Updated nullity test to check for empty nodelists.
* fast/mutation/mutation-record-nullity-expected.txt:
* fast/mutation/mutation-record-nullity.html:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (136995 => 136996)
--- trunk/LayoutTests/ChangeLog 2012-12-07 23:42:02 UTC (rev 136995)
+++ trunk/LayoutTests/ChangeLog 2012-12-07 23:43:48 UTC (rev 136996)
@@ -1,3 +1,15 @@
+2012-12-07 Adam Klein <[email protected]>
+
+ MutationRecord addedNodes/removedNodes should never be null
+ https://bugs.webkit.org/show_bug.cgi?id=98921
+
+ Reviewed by Ryosuke Niwa.
+
+ Updated nullity test to check for empty nodelists.
+
+ * fast/mutation/mutation-record-nullity-expected.txt:
+ * fast/mutation/mutation-record-nullity.html:
+
2012-12-07 Emil A Eklund <[email protected]>
Unreviewed chromium-win rebaselines.
Modified: trunk/LayoutTests/fast/mutation/mutation-record-nullity-expected.txt (136995 => 136996)
--- trunk/LayoutTests/fast/mutation/mutation-record-nullity-expected.txt 2012-12-07 23:42:02 UTC (rev 136995)
+++ trunk/LayoutTests/fast/mutation/mutation-record-nullity-expected.txt 2012-12-07 23:43:48 UTC (rev 136996)
@@ -1,4 +1,4 @@
-Non-relevant properties on mutation records should be null
+Non-relevant properties on mutation records should be null, except for NodeLists, which should be empty
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
@@ -7,10 +7,10 @@
PASS record.attributeName is null
PASS record.attributeNamespace is null
PASS record.oldValue is null
-PASS record.addedNodes is null
-PASS record.removedNodes is null
PASS record.previousSibling is null
PASS record.nextSibling is null
+PASS record.addedNodes.length is 0
+PASS record.removedNodes.length is 0
childList record:
PASS record.attributeName is null
@@ -20,10 +20,10 @@
attributes record:
PASS record.attributeNamespace is null
PASS record.oldValue is null
-PASS record.addedNodes is null
-PASS record.removedNodes is null
PASS record.previousSibling is null
PASS record.nextSibling is null
+PASS record.addedNodes.length is 0
+PASS record.removedNodes.length is 0
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/fast/mutation/mutation-record-nullity.html (136995 => 136996)
--- trunk/LayoutTests/fast/mutation/mutation-record-nullity.html 2012-12-07 23:42:02 UTC (rev 136995)
+++ trunk/LayoutTests/fast/mutation/mutation-record-nullity.html 2012-12-07 23:43:48 UTC (rev 136996)
@@ -1,7 +1,7 @@
<!DOCTYPE html>
<script src=""
<script>
-description('Non-relevant properties on mutation records should be null');
+description('Non-relevant properties on mutation records should be null, except for NodeLists, which should be empty');
var observer = new WebKitMutationObserver(function() {});
var text = document.createTextNode('something');
@@ -12,10 +12,10 @@
shouldBeNull('record.attributeName');
shouldBeNull('record.attributeNamespace');
shouldBeNull('record.oldValue');
-shouldBeNull('record.addedNodes');
-shouldBeNull('record.removedNodes');
shouldBeNull('record.previousSibling');
shouldBeNull('record.nextSibling');
+shouldBe('record.addedNodes.length', '0');
+shouldBe('record.removedNodes.length', '0');
var div = document.createElement('div');
observer.observe(div, {childList: true});
@@ -32,9 +32,9 @@
debug('\nattributes record:');
shouldBeNull('record.attributeNamespace');
shouldBeNull('record.oldValue');
-shouldBeNull('record.addedNodes');
-shouldBeNull('record.removedNodes');
shouldBeNull('record.previousSibling');
shouldBeNull('record.nextSibling');
+shouldBe('record.addedNodes.length', '0');
+shouldBe('record.removedNodes.length', '0');
</script>
<script src=""
Modified: trunk/Source/WebCore/ChangeLog (136995 => 136996)
--- trunk/Source/WebCore/ChangeLog 2012-12-07 23:42:02 UTC (rev 136995)
+++ trunk/Source/WebCore/ChangeLog 2012-12-07 23:43:48 UTC (rev 136996)
@@ -1,3 +1,27 @@
+2012-12-07 Adam Klein <[email protected]>
+
+ MutationRecord addedNodes/removedNodes should never be null
+ https://bugs.webkit.org/show_bug.cgi?id=98921
+
+ Reviewed by Ryosuke Niwa.
+
+ Per an update to the DOM4 spec that matches Gecko's behavior,
+ addedNodes/removedNodes should be empty NodeLists on 'attributes'
+ and 'characterData' records, rather than null.
+
+ This is accomplished with lazy initialization of addedNodes/removedNodes
+ attributes on 'attributes'/'characterData' records and the
+ addition of a new StaticNodeList::createEmpty() factory method.
+
+ * dom/MutationRecord.cpp:
+ * dom/MutationRecord.h:
+ (MutationRecord):
+ * dom/StaticNodeList.h:
+ (WebCore::StaticNodeList::adopt):
+ (StaticNodeList):
+ (WebCore::StaticNodeList::createEmpty):
+ (WebCore::StaticNodeList::StaticNodeList):
+
2012-12-07 Brent Fulgham <[email protected]>
Unreviewed build correction after 136959.
Modified: trunk/Source/WebCore/dom/MutationRecord.cpp (136995 => 136996)
--- trunk/Source/WebCore/dom/MutationRecord.cpp 2012-12-07 23:42:02 UTC (rev 136995)
+++ trunk/Source/WebCore/dom/MutationRecord.cpp 2012-12-07 23:43:48 UTC (rev 136996)
@@ -37,6 +37,7 @@
#include "Node.h"
#include "NodeList.h"
#include "QualifiedName.h"
+#include "StaticNodeList.h"
#include <wtf/Assertions.h>
#include <wtf/StdLibExtras.h>
@@ -70,44 +71,60 @@
RefPtr<Node> m_nextSibling;
};
-class AttributesRecord : public MutationRecord {
+class RecordWithEmptyNodeLists : public MutationRecord {
public:
+ RecordWithEmptyNodeLists(PassRefPtr<Node> target, const String& oldValue)
+ : m_target(target)
+ , m_oldValue(oldValue)
+ {
+ }
+
+private:
+ virtual Node* target() OVERRIDE { return m_target.get(); }
+ virtual String oldValue() OVERRIDE { return m_oldValue; }
+ virtual NodeList* addedNodes() OVERRIDE { return lazilyInitializeEmptyNodeList(m_addedNodes); }
+ virtual NodeList* removedNodes() OVERRIDE { return lazilyInitializeEmptyNodeList(m_removedNodes); }
+
+ static NodeList* lazilyInitializeEmptyNodeList(RefPtr<NodeList>& nodeList)
+ {
+ if (!nodeList)
+ nodeList = StaticNodeList::createEmpty();
+ return nodeList.get();
+ }
+
+ RefPtr<Node> m_target;
+ String m_oldValue;
+ RefPtr<NodeList> m_addedNodes;
+ RefPtr<NodeList> m_removedNodes;
+};
+
+class AttributesRecord : public RecordWithEmptyNodeLists {
+public:
AttributesRecord(PassRefPtr<Node> target, const QualifiedName& name, const AtomicString& oldValue)
- : m_target(target)
+ : RecordWithEmptyNodeLists(target, oldValue)
, m_attributeName(name.localName())
, m_attributeNamespace(name.namespaceURI())
- , m_oldValue(oldValue)
{
}
private:
virtual const AtomicString& type() OVERRIDE;
- virtual Node* target() OVERRIDE { return m_target.get(); }
virtual const AtomicString& attributeName() OVERRIDE { return m_attributeName; }
virtual const AtomicString& attributeNamespace() OVERRIDE { return m_attributeNamespace; }
- virtual String oldValue() OVERRIDE { return m_oldValue; }
- RefPtr<Node> m_target;
AtomicString m_attributeName;
AtomicString m_attributeNamespace;
- AtomicString m_oldValue;
};
-class CharacterDataRecord : public MutationRecord {
+class CharacterDataRecord : public RecordWithEmptyNodeLists {
public:
CharacterDataRecord(PassRefPtr<Node> target, const String& oldValue)
- : m_target(target)
- , m_oldValue(oldValue)
+ : RecordWithEmptyNodeLists(target, oldValue)
{
}
private:
virtual const AtomicString& type() OVERRIDE;
- virtual Node* target() OVERRIDE { return m_target.get(); }
- virtual String oldValue() OVERRIDE { return m_oldValue; }
-
- RefPtr<Node> m_target;
- String m_oldValue;
};
class MutationRecordWithNullOldValue : public MutationRecord {
Modified: trunk/Source/WebCore/dom/MutationRecord.h (136995 => 136996)
--- trunk/Source/WebCore/dom/MutationRecord.h 2012-12-07 23:42:02 UTC (rev 136995)
+++ trunk/Source/WebCore/dom/MutationRecord.h 2012-12-07 23:43:48 UTC (rev 136996)
@@ -57,8 +57,8 @@
virtual const AtomicString& type() = 0;
virtual Node* target() = 0;
- virtual NodeList* addedNodes() { return 0; }
- virtual NodeList* removedNodes() { return 0; }
+ virtual NodeList* addedNodes() = 0;
+ virtual NodeList* removedNodes() = 0;
virtual Node* previousSibling() { return 0; }
virtual Node* nextSibling() { return 0; }
Modified: trunk/Source/WebCore/dom/StaticNodeList.h (136995 => 136996)
--- trunk/Source/WebCore/dom/StaticNodeList.h 2012-12-07 23:42:02 UTC (rev 136995)
+++ trunk/Source/WebCore/dom/StaticNodeList.h 2012-12-07 23:43:48 UTC (rev 136996)
@@ -40,21 +40,25 @@
class StaticNodeList : public NodeList {
public:
- // Adopts the contents of the nodes vector.
static PassRefPtr<StaticNodeList> adopt(Vector<RefPtr<Node> >& nodes)
{
- return adoptRef(new StaticNodeList(nodes));
+ RefPtr<StaticNodeList> nodeList = adoptRef(new StaticNodeList);
+ nodeList->m_nodes.swap(nodes);
+ return nodeList.release();
}
+ static PassRefPtr<StaticNodeList> createEmpty()
+ {
+ return adoptRef(new StaticNodeList);
+ }
+
virtual unsigned length() const OVERRIDE;
virtual Node* item(unsigned index) const OVERRIDE;
virtual Node* namedItem(const AtomicString&) const OVERRIDE;
private:
- explicit StaticNodeList(Vector<RefPtr<Node> >& nodes)
- {
- m_nodes.swap(nodes);
- }
+ StaticNodeList() { }
+
Vector<RefPtr<Node> > m_nodes;
};
