Changeset 136996 in webkit


Ignore:
Timestamp:
Dec 7, 2012 3:43:48 PM (11 years ago)
Author:
adamk@chromium.org
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:
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r136994 r136996  
     12012-12-07  Adam Klein  <adamk@chromium.org>
     2
     3        MutationRecord addedNodes/removedNodes should never be null
     4        https://bugs.webkit.org/show_bug.cgi?id=98921
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Updated nullity test to check for empty nodelists.
     9
     10        * fast/mutation/mutation-record-nullity-expected.txt:
     11        * fast/mutation/mutation-record-nullity.html:
     12
    1132012-12-07  Emil A Eklund  <eae@chromium.org>
    214

  • trunk/LayoutTests/fast/mutation/mutation-record-nullity-expected.txt

    r130442 r136996  
    1 Non-relevant properties on mutation records should be null
     1Non-relevant properties on mutation records should be null, except for NodeLists, which should be empty
    22
    33On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
    … …  
    88PASS record.attributeNamespace is null
    99PASS record.oldValue is null
    10 PASS record.addedNodes is null
    11 PASS record.removedNodes is null
    1210PASS record.previousSibling is null
    1311PASS record.nextSibling is null
     12PASS record.addedNodes.length is 0
     13PASS record.removedNodes.length is 0
    1414
    1515childList record:
    … …  
    2121PASS record.attributeNamespace is null
    2222PASS record.oldValue is null
    23 PASS record.addedNodes is null
    24 PASS record.removedNodes is null
    2523PASS record.previousSibling is null
    2624PASS record.nextSibling is null
     25PASS record.addedNodes.length is 0
     26PASS record.removedNodes.length is 0
    2727PASS successfullyParsed is true
    2828

  • trunk/LayoutTests/fast/mutation/mutation-record-nullity.html

    r130442 r136996  
    22<script src="../js/resources/js-test-pre.js"></script>
    33<script>
    4 description('Non-relevant properties on mutation records should be null');
     4description('Non-relevant properties on mutation records should be null, except for NodeLists, which should be empty');
    55var observer = new WebKitMutationObserver(function() {});
    66
    … …  
    1313shouldBeNull('record.attributeNamespace');
    1414shouldBeNull('record.oldValue');
    15 shouldBeNull('record.addedNodes');
    16 shouldBeNull('record.removedNodes');
    1715shouldBeNull('record.previousSibling');
    1816shouldBeNull('record.nextSibling');
     17shouldBe('record.addedNodes.length', '0');
     18shouldBe('record.removedNodes.length', '0');
    1919
    2020var div = document.createElement('div');
    … …  
    3333shouldBeNull('record.attributeNamespace');
    3434shouldBeNull('record.oldValue');
    35 shouldBeNull('record.addedNodes');
    36 shouldBeNull('record.removedNodes');
    3735shouldBeNull('record.previousSibling');
    3836shouldBeNull('record.nextSibling');
     37shouldBe('record.addedNodes.length', '0');
     38shouldBe('record.removedNodes.length', '0');
    3939</script>
    4040<script src="../js/resources/js-test-post.js"></script>

  • trunk/Source/WebCore/ChangeLog

    r136995 r136996  
     12012-12-07  Adam Klein  <adamk@chromium.org>
     2
     3        MutationRecord addedNodes/removedNodes should never be null
     4        https://bugs.webkit.org/show_bug.cgi?id=98921
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Per an update to the DOM4 spec that matches Gecko's behavior,
     9        addedNodes/removedNodes should be empty NodeLists on 'attributes'
     10        and 'characterData' records, rather than null.
     11
     12        This is accomplished with lazy initialization of addedNodes/removedNodes
     13        attributes on 'attributes'/'characterData' records and the
     14        addition of a new StaticNodeList::createEmpty() factory method.
     15
     16        * dom/MutationRecord.cpp:
     17        * dom/MutationRecord.h:
     18        (MutationRecord):
     19        * dom/StaticNodeList.h:
     20        (WebCore::StaticNodeList::adopt):
     21        (StaticNodeList):
     22        (WebCore::StaticNodeList::createEmpty):
     23        (WebCore::StaticNodeList::StaticNodeList):
     24
    1252012-12-07  Brent Fulgham  <bfulgham@webkit.org>
    226

  • trunk/Source/WebCore/dom/MutationRecord.cpp

    r133976 r136996  
    3838#include "NodeList.h"
    3939#include "QualifiedName.h"
     40#include "StaticNodeList.h"
    4041#include <wtf/Assertions.h>
    4142#include <wtf/StdLibExtras.h>
    … …  
    7172};
    7273
    73 class AttributesRecord : public MutationRecord {
     74class RecordWithEmptyNodeLists : public MutationRecord {
    7475public:
    75     AttributesRecord(PassRefPtr<Node> target, const QualifiedName& name, const AtomicString& oldValue)
    76         : m_target(target)
    77         , m_attributeName(name.localName())
    78         , m_attributeNamespace(name.namespaceURI())
    79         , m_oldValue(oldValue)
    80     {
    81     }
    82 
    83 private:
    84     virtual const AtomicString& type() OVERRIDE;
    85     virtual Node* target() OVERRIDE { return m_target.get(); }
    86     virtual const AtomicString& attributeName() OVERRIDE { return m_attributeName; }
    87     virtual const AtomicString& attributeNamespace() OVERRIDE { return m_attributeNamespace; }
    88     virtual String oldValue() OVERRIDE { return m_oldValue; }
    89 
    90     RefPtr<Node> m_target;
    91     AtomicString m_attributeName;
    92     AtomicString m_attributeNamespace;
    93     AtomicString m_oldValue;
    94 };
    95 
    96 class CharacterDataRecord : public MutationRecord {
    97 public:
    98     CharacterDataRecord(PassRefPtr<Node> target, const String& oldValue)
     76    RecordWithEmptyNodeLists(PassRefPtr<Node> target, const String& oldValue)
    9977        : m_target(target)
    10078        , m_oldValue(oldValue)
    … …  
    10381
    10482private:
    105     virtual const AtomicString& type() OVERRIDE;
    10683    virtual Node* target() OVERRIDE { return m_target.get(); }
    10784    virtual String oldValue() OVERRIDE { return m_oldValue; }
     85    virtual NodeList* addedNodes() OVERRIDE { return lazilyInitializeEmptyNodeList(m_addedNodes); }
     86    virtual NodeList* removedNodes() OVERRIDE { return lazilyInitializeEmptyNodeList(m_removedNodes); }
     87
     88    static NodeList* lazilyInitializeEmptyNodeList(RefPtr<NodeList>& nodeList)
     89    {
     90        if (!nodeList)
     91            nodeList = StaticNodeList::createEmpty();
     92        return nodeList.get();
     93    }
    10894
    10995    RefPtr<Node> m_target;
    11096    String m_oldValue;
     97    RefPtr<NodeList> m_addedNodes;
     98    RefPtr<NodeList> m_removedNodes;
     99};
     100
     101class AttributesRecord : public RecordWithEmptyNodeLists {
     102public:
     103    AttributesRecord(PassRefPtr<Node> target, const QualifiedName& name, const AtomicString& oldValue)
     104        : RecordWithEmptyNodeLists(target, oldValue)
     105        , m_attributeName(name.localName())
     106        , m_attributeNamespace(name.namespaceURI())
     107    {
     108    }
     109
     110private:
     111    virtual const AtomicString& type() OVERRIDE;
     112    virtual const AtomicString& attributeName() OVERRIDE { return m_attributeName; }
     113    virtual const AtomicString& attributeNamespace() OVERRIDE { return m_attributeNamespace; }
     114
     115    AtomicString m_attributeName;
     116    AtomicString m_attributeNamespace;
     117};
     118
     119class CharacterDataRecord : public RecordWithEmptyNodeLists {
     120public:
     121    CharacterDataRecord(PassRefPtr<Node> target, const String& oldValue)
     122        : RecordWithEmptyNodeLists(target, oldValue)
     123    {
     124    }
     125
     126private:
     127    virtual const AtomicString& type() OVERRIDE;
    111128};
    112129

  • trunk/Source/WebCore/dom/MutationRecord.h

    r127757 r136996  
    5858    virtual Node* target() = 0;
    5959
    60     virtual NodeList* addedNodes() { return 0; }
    61     virtual NodeList* removedNodes() { return 0; }
     60    virtual NodeList* addedNodes() = 0;
     61    virtual NodeList* removedNodes() = 0;
    6262    virtual Node* previousSibling() { return 0; }
    6363    virtual Node* nextSibling() { return 0; }

  • trunk/Source/WebCore/dom/StaticNodeList.h

    r135667 r136996  
    4141    class StaticNodeList : public NodeList {
    4242    public:
    43         // Adopts the contents of the nodes vector.
    4443        static PassRefPtr<StaticNodeList> adopt(Vector<RefPtr<Node> >& nodes)
    4544        {
    46             return adoptRef(new StaticNodeList(nodes));
     45            RefPtr<StaticNodeList> nodeList = adoptRef(new StaticNodeList);
     46            nodeList->m_nodes.swap(nodes);
     47            return nodeList.release();
     48        }
     49
     50        static PassRefPtr<StaticNodeList> createEmpty()
     51        {
     52            return adoptRef(new StaticNodeList);
    4753        }
    4854
    … …  
    5258
    5359    private:
    54         explicit StaticNodeList(Vector<RefPtr<Node> >& nodes)
    55         {
    56             m_nodes.swap(nodes);
    57         }
     60        StaticNodeList() { }
     61
    5862        Vector<RefPtr<Node> > m_nodes;
    5963    };
Note: See TracChangeset for help on using the changeset viewer.