Title: [102431] trunk/Source/WebCore
Revision
102431
Author
rn...@webkit.org
Date
2011-12-08 21:26:58 -0800 (Thu, 08 Dec 2011)

Log Message

It's semantically incorrect to call notifyNodeListsAttributeChanged in dispatchSubtreeModifiedEvent
https://bugs.webkit.org/show_bug.cgi?id=74028

Reviewed by Darin Adler.

Remove a call to notifyNodeListsAttributeChanged in dispatchSubtreeModified and add explicit calls
to notifyNodeListsAttributeChanged at appropriate places.

Also merge notifyNodeListsChildrenChanged with notifyLocalNodeListsChildrenChanged, and
notifyNodeListsAttributeChanged with notifyLocalNodeListsAttributeChanged, and rename them to
invalidateNodeListsCacheAfterAttributeChanges and invalidateNodeListsCacheAfterNodeChanges respectively.

* dom/Attr.cpp:
(WebCore::Attr::childrenChanged):
* dom/ContainerNode.cpp:
(WebCore::ContainerNode::childrenChanged):
* dom/Document.cpp:
(WebCore::Document::updateRangesAfterNodeChanges):
* dom/Document.h:
* dom/NamedNodeMap.cpp:
(WebCore::NamedNodeMap::addAttribute):
(WebCore::NamedNodeMap::removeAttribute):
* dom/Node.cpp:
(WebCore::removeNodeListCacheIfPossible):
(WebCore::Node::unregisterDynamicNodeList):
(WebCore::Node::invalidateNodeListsCacheAfterAttributeChanges):
(WebCore::Node::invalidateNodeListsCacheAfterNodeChanges):
(WebCore::Node::dispatchSubtreeModifiedEvent):
* dom/Node.h:
* dom/NodeRareData.h:
* dom/StyledElement.cpp:
(WebCore::StyledElement::classAttributeChanged):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (102430 => 102431)


--- trunk/Source/WebCore/ChangeLog	2011-12-09 05:23:18 UTC (rev 102430)
+++ trunk/Source/WebCore/ChangeLog	2011-12-09 05:26:58 UTC (rev 102431)
@@ -1,3 +1,38 @@
+2011-12-08  Ryosuke Niwa  <rn...@webkit.org>
+
+        It's semantically incorrect to call notifyNodeListsAttributeChanged in dispatchSubtreeModifiedEvent
+        https://bugs.webkit.org/show_bug.cgi?id=74028
+
+        Reviewed by Darin Adler.
+
+        Remove a call to notifyNodeListsAttributeChanged in dispatchSubtreeModified and add explicit calls
+        to notifyNodeListsAttributeChanged at appropriate places.
+
+        Also merge notifyNodeListsChildrenChanged with notifyLocalNodeListsChildrenChanged, and
+        notifyNodeListsAttributeChanged with notifyLocalNodeListsAttributeChanged, and rename them to
+        invalidateNodeListsCacheAfterAttributeChanges and invalidateNodeListsCacheAfterNodeChanges respectively.
+
+        * dom/Attr.cpp:
+        (WebCore::Attr::childrenChanged):
+        * dom/ContainerNode.cpp:
+        (WebCore::ContainerNode::childrenChanged):
+        * dom/Document.cpp:
+        (WebCore::Document::updateRangesAfterNodeChanges):
+        * dom/Document.h:
+        * dom/NamedNodeMap.cpp:
+        (WebCore::NamedNodeMap::addAttribute):
+        (WebCore::NamedNodeMap::removeAttribute):
+        * dom/Node.cpp:
+        (WebCore::removeNodeListCacheIfPossible):
+        (WebCore::Node::unregisterDynamicNodeList):
+        (WebCore::Node::invalidateNodeListsCacheAfterAttributeChanges):
+        (WebCore::Node::invalidateNodeListsCacheAfterNodeChanges):
+        (WebCore::Node::dispatchSubtreeModifiedEvent):
+        * dom/Node.h:
+        * dom/NodeRareData.h:
+        * dom/StyledElement.cpp:
+        (WebCore::StyledElement::classAttributeChanged):
+
 2011-12-08  Kenichi Ishibashi  <ba...@chromium.org>
 
         Unreviewed, rolling out r102418.

Modified: trunk/Source/WebCore/dom/Attr.cpp (102430 => 102431)


--- trunk/Source/WebCore/dom/Attr.cpp	2011-12-09 05:23:18 UTC (rev 102430)
+++ trunk/Source/WebCore/dom/Attr.cpp	2011-12-09 05:26:58 UTC (rev 102431)
@@ -170,6 +170,8 @@
  
     Node::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
 
+    invalidateNodeListsCacheAfterAttributeChanged();
+
     // FIXME: We should include entity references in the value
     
     String val = "";

Modified: trunk/Source/WebCore/dom/ContainerNode.cpp (102430 => 102431)


--- trunk/Source/WebCore/dom/ContainerNode.cpp	2011-12-09 05:23:18 UTC (rev 102430)
+++ trunk/Source/WebCore/dom/ContainerNode.cpp	2011-12-09 05:26:58 UTC (rev 102431)
@@ -843,9 +843,9 @@
 {
     Node::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
     if (!changedByParser && childCountDelta)
-        document()->nodeChildrenChanged(this);
+        document()->updateRangesAfterChildrenChanged(this);
     if (treeScope()->hasNodeListCaches())
-        notifyNodeListsChildrenChanged();
+        invalidateNodeListsCacheAfterChildrenChanged();
 }
 
 void ContainerNode::cloneChildNodes(ContainerNode *clone)

Modified: trunk/Source/WebCore/dom/Document.cpp (102430 => 102431)


--- trunk/Source/WebCore/dom/Document.cpp	2011-12-09 05:23:18 UTC (rev 102430)
+++ trunk/Source/WebCore/dom/Document.cpp	2011-12-09 05:26:58 UTC (rev 102431)
@@ -3410,7 +3410,7 @@
     }
 }
 
-void Document::nodeChildrenChanged(ContainerNode* container)
+void Document::updateRangesAfterChildrenChanged(ContainerNode* container)
 {
     if (!disableRangeMutation(page()) && !m_ranges.isEmpty()) {
         HashSet<Range*>::const_iterator end = m_ranges.end();

Modified: trunk/Source/WebCore/dom/Document.h (102430 => 102431)


--- trunk/Source/WebCore/dom/Document.h	2011-12-09 05:23:18 UTC (rev 102430)
+++ trunk/Source/WebCore/dom/Document.h	2011-12-09 05:26:58 UTC (rev 102431)
@@ -740,7 +740,7 @@
     void attachRange(Range*);
     void detachRange(Range*);
 
-    void nodeChildrenChanged(ContainerNode*);
+    void updateRangesAfterChildrenChanged(ContainerNode*);
     // nodeChildrenWillBeRemoved is used when removing all node children at once.
     void nodeChildrenWillBeRemoved(ContainerNode*);
     // nodeWillBeRemoved is only safe when removing one node at a time.

Modified: trunk/Source/WebCore/dom/NamedNodeMap.cpp (102430 => 102431)


--- trunk/Source/WebCore/dom/NamedNodeMap.cpp	2011-12-09 05:23:18 UTC (rev 102430)
+++ trunk/Source/WebCore/dom/NamedNodeMap.cpp	2011-12-09 05:26:58 UTC (rev 102431)
@@ -255,6 +255,7 @@
         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();
         }
@@ -291,6 +292,7 @@
         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 (102430 => 102431)


--- trunk/Source/WebCore/dom/Node.cpp	2011-12-09 05:23:18 UTC (rev 102430)
+++ trunk/Source/WebCore/dom/Node.cpp	2011-12-09 05:26:58 UTC (rev 102431)
@@ -996,6 +996,14 @@
     return count;
 }
 
+static void removeNodeListCacheIfPossible(Node* node, NodeRareData* data)
+{
+    if (!data->nodeLists()->isEmpty())
+        return;
+    data->clearNodeLists();
+    node->treeScope()->removeNodeListCache();
+}
+
 void Node::registerDynamicNodeList(DynamicNodeList* list)
 {
     NodeRareData* data = ""
@@ -1014,66 +1022,48 @@
     if (list->hasOwnCaches()) {
         NodeRareData* data = ""
         data->nodeLists()->m_listsWithCaches.remove(list);
-        removeNodeListCacheIfPossible();
+        removeNodeListCacheIfPossible(this, data);
     }
 }
 
-inline void Node::notifyLocalNodeListsAttributeChanged()
+void Node::invalidateNodeListsCacheAfterAttributeChanged()
 {
-    if (!hasRareData())
-        return;
-    NodeRareData* data = ""
-    if (!data->nodeLists())
-        return;
+    for (Node* node = this; node; node = node->parentNode()) {
+        if (!node->hasRareData())
+            continue;
+        NodeRareData* data = ""
+        if (!data->nodeLists())
+            continue;
 
-    if (!isAttributeNode())
-        data->nodeLists()->invalidateCachesThatDependOnAttributes();
-    else
-        data->nodeLists()->invalidateCaches();
+        // For attribute nodes, we need to invalidate childNodes as well.
+        if (node->isAttributeNode())
+            data->nodeLists()->invalidateCaches();
+        else
+            data->nodeLists()->invalidateCachesThatDependOnAttributes();
 
-    removeNodeListCacheIfPossible();
+        removeNodeListCacheIfPossible(node, data);
+    }
 }
 
-void Node::notifyNodeListsAttributeChanged()
+void Node::invalidateNodeListsCacheAfterChildrenChanged()
 {
-    for (Node *n = this; n; n = n->parentNode())
-        n->notifyLocalNodeListsAttributeChanged();
-}
+    for (Node* node = this; node; node = node->parentNode()) {
+        if (!node->hasRareData())
+            continue;
+        NodeRareData* data = ""
+        if (!data->nodeLists())
+            continue;
 
-inline void Node::notifyLocalNodeListsChildrenChanged()
-{
-    if (!hasRareData())
-        return;
-    NodeRareData* data = ""
-    if (!data->nodeLists())
-        return;
+        data->nodeLists()->invalidateCaches();
 
-    data->nodeLists()->invalidateCaches();
+        NodeListsNodeData::NodeListSet::iterator end = data->nodeLists()->m_listsWithCaches.end();
+        for (NodeListsNodeData::NodeListSet::iterator it = data->nodeLists()->m_listsWithCaches.begin(); it != end; ++it)
+            (*it)->invalidateCache();
 
-    NodeListsNodeData::NodeListSet::iterator end = data->nodeLists()->m_listsWithCaches.end();
-    for (NodeListsNodeData::NodeListSet::iterator i = data->nodeLists()->m_listsWithCaches.begin(); i != end; ++i)
-        (*i)->invalidateCache();
-
-    removeNodeListCacheIfPossible();
+        removeNodeListCacheIfPossible(node, data);
+    }
 }
-    
-void Node::removeNodeListCacheIfPossible()
-{
-    ASSERT(rareData()->nodeLists());
 
-    NodeRareData* data = ""
-    if (!data->nodeLists()->isEmpty())
-        return;
-    data->clearNodeLists();
-    treeScope()->removeNodeListCache();
-}
-
-void Node::notifyNodeListsChildrenChanged()
-{
-    for (Node* n = this; n; n = n->parentNode())
-        n->notifyLocalNodeListsChildrenChanged();
-}
-
 void Node::notifyLocalNodeListsLabelChanged()
 {
     if (!hasRareData())
@@ -2833,8 +2823,6 @@
     
     document()->incDOMTreeVersion();
 
-    notifyNodeListsAttributeChanged(); // FIXME: Can do better some day. Really only care about the name attribute changing.
-    
     if (!document()->hasListenerType(Document::DOMSUBTREEMODIFIED_LISTENER))
         return;
 

Modified: trunk/Source/WebCore/dom/Node.h (102430 => 102431)


--- trunk/Source/WebCore/dom/Node.h	2011-12-09 05:23:18 UTC (rev 102430)
+++ trunk/Source/WebCore/dom/Node.h	2011-12-09 05:26:58 UTC (rev 102431)
@@ -517,13 +517,10 @@
     void showTreeForThisAcrossFrame() const;
 #endif
 
-    void removeNodeListCacheIfPossible();
     void registerDynamicNodeList(DynamicNodeList*);
     void unregisterDynamicNodeList(DynamicNodeList*);
-    void notifyNodeListsChildrenChanged();
-    void notifyLocalNodeListsChildrenChanged();
-    void notifyNodeListsAttributeChanged();
-    void notifyLocalNodeListsAttributeChanged();
+    void invalidateNodeListsCacheAfterAttributeChanged();
+    void invalidateNodeListsCacheAfterChildrenChanged();
     void notifyLocalNodeListsLabelChanged();
     void removeCachedClassNodeList(ClassNodeList*, const String&);
 

Modified: trunk/Source/WebCore/dom/NodeRareData.h (102430 => 102431)


--- trunk/Source/WebCore/dom/NodeRareData.h	2011-12-09 05:23:18 UTC (rev 102430)
+++ trunk/Source/WebCore/dom/NodeRareData.h	2011-12-09 05:26:58 UTC (rev 102431)
@@ -126,6 +126,7 @@
     void clearNodeLists() { m_nodeLists.clear(); }
     void setNodeLists(PassOwnPtr<NodeListsNodeData> lists) { m_nodeLists = lists; }
     NodeListsNodeData* nodeLists() const { return m_nodeLists.get(); }
+
     NodeListsNodeData* ensureNodeLists(Node* node)
     {
         if (!m_nodeLists)

Modified: trunk/Source/WebCore/dom/StyledElement.cpp (102430 => 102431)


--- trunk/Source/WebCore/dom/StyledElement.cpp	2011-12-09 05:23:18 UTC (rev 102430)
+++ trunk/Source/WebCore/dom/StyledElement.cpp	2011-12-09 05:26:58 UTC (rev 102431)
@@ -228,6 +228,7 @@
     } else if (attributeMap())
         attributeMap()->clearClass();
     setNeedsStyleRecalc();
+    invalidateNodeListsCacheAfterAttributeChanged();
     dispatchSubtreeModifiedEvent();
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to