Changeset 140784 in webkit

Timestamp:

Jan 24, 2013 10:45:39 PM (11 years ago)

Author:

morrita@google.com

Message:

Refactoring: The name ContainerNode::removeChildren and ContainerNde::removeAllChilren() is confusing
​https://bugs.webkit.org/show_bug.cgi?id=107640

Reviewed by Eric Seidel.

This change renames unsafe removeAllChilren() function to
removeDetachedChildren() and move it to protected visibility.

In theory, the removed nodes should be detached() before. But some
tests violates that assumption. It should be fixed.

No new tests. No behavior change.

• dom/ContainerNode.cpp:

(WebCore::ContainerNode::removeDetachedChildren):
(WebCore::ContainerNode::takeAllChildrenFrom):
(WebCore::ContainerNode::~ContainerNode):

• dom/ContainerNode.h:

(ContainerNode):

• dom/ContainerNodeAlgorithms.h:

(WebCore::removeDetachedChildrenInContainer):
(WebCore):

• dom/Document.cpp:

(WebCore::Document::removedLastRef):

• dom/ShadowRoot.cpp:

(WebCore::ShadowRoot::~ShadowRoot):

• svg/SVGElementInstance.cpp:

(WebCore::SVGElementInstance::detach):

• svg/SVGElementInstance.h:

(SVGElementInstance):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/ContainerNode.cpp (modified) (3 diffs)
• dom/ContainerNode.h (modified) (2 diffs)
• dom/ContainerNodeAlgorithms.h (modified) (2 diffs)
• dom/Document.cpp (modified) (2 diffs)
• dom/ShadowRoot.cpp (modified) (1 diff)
• svg/SVGElementInstance.cpp (modified) (1 diff)
• svg/SVGElementInstance.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-01-24  Hajime Morrita  <morrita@google.com>
+
+        Refactoring: The name ContainerNode::removeChildren and ContainerNde::removeAllChilren() is confusing
+        https://bugs.webkit.org/show_bug.cgi?id=107640
+
+        Reviewed by Eric Seidel.
+
+        This change renames unsafe removeAllChilren() function to
+        removeDetachedChildren() and move it to protected visibility.
+
+        In theory, the removed nodes should be detached() before. But some
+        tests violates that assumption. It should be fixed.
+
+        No new tests. No behavior change.
+
+        * dom/ContainerNode.cpp:
+        (WebCore::ContainerNode::removeDetachedChildren):
+        (WebCore::ContainerNode::takeAllChildrenFrom):
+        (WebCore::ContainerNode::~ContainerNode):
+        * dom/ContainerNode.h:
+        (ContainerNode):
+        * dom/ContainerNodeAlgorithms.h:
+        (WebCore::removeDetachedChildrenInContainer):
+        (WebCore):
+        * dom/Document.cpp:
+        (WebCore::Document::removedLastRef):
+        * dom/ShadowRoot.cpp:
+        (WebCore::ShadowRoot::~ShadowRoot):
+        * svg/SVGElementInstance.cpp:
+        (WebCore::SVGElementInstance::detach):
+        * svg/SVGElementInstance.h:
+        (SVGElementInstance):
+
 2013-01-24  Keishi Hattori  <keishi@webkit.org>
 

trunk/Source/WebCore/dom/ContainerNode.cpp

@@ -91 +91 @@
 }
 
-void ContainerNode::removeAllChildren()
-{
-    removeAllChildrenInContainer<Node, ContainerNode>(this);
+void ContainerNode::removeDetachedChildren()
+{
+    // FIXME: We should be able to ASSERT(!attached()) here: https://bugs.webkit.org/show_bug.cgi?id=107801
+    removeDetachedChildrenInContainer<Node, ContainerNode>(this);
 }
 
@@ -100 +101 @@
     NodeVector children;
     getChildNodes(oldParent, children);
-    oldParent->removeAllChildren();
+    oldParent->removeDetachedChildren();
 
     for (unsigned i = 0; i < children.size(); ++i) {
@@ -124 +125 @@
         documentInternal()->axObjectCache()->remove(this);
 
-    removeAllChildren();
+    removeDetachedChildren();
 }
 

trunk/Source/WebCore/dom/ContainerNode.h

@@ -102 +102 @@
     void parserInsertBefore(PassRefPtr<Node> newChild, Node* refChild);
 
-    // FIXME: It's not good to have two functions with such similar names, especially public functions.
-    // How do removeChildren and removeAllChildren differ?
     void removeChildren();
-    void removeAllChildren();
-
     void takeAllChildrenFrom(ContainerNode*);
 
@@ -155 +151 @@
     friend void Private::addChildNodesToDeletionQueue(GenericNode*& head, GenericNode*& tail, GenericNodeContainer*);
 
+    void removeDetachedChildren();
     void setFirstChild(Node* child) { m_firstChild = child; }
     void setLastChild(Node* child) { m_lastChild = child; }

trunk/Source/WebCore/dom/ContainerNodeAlgorithms.h

@@ -79 +79 @@
 // This applies to 'ContainerNode' and 'SVGElementInstance'
 template<class GenericNode, class GenericNodeContainer>
-inline void removeAllChildrenInContainer(GenericNodeContainer* container)
+inline void removeDetachedChildrenInContainer(GenericNodeContainer* container)
 {
     // List of nodes to be deleted.
@@ -121 +121 @@
 }
 
-// Helper methods for removeAllChildrenInContainer, hidden from WebCore namespace
+// Helper methods for removeDetachedChildrenInContainer, hidden from WebCore namespace
 namespace Private {
 

trunk/Source/WebCore/dom/Document.cpp

@@ -657 +657 @@
         // If removing a child removes the last self-only ref, we don't
         // want the scope to be destructed until after
-        // removeAllChildren returns, so we guard ourselves with an
+        // removeDetachedChildren returns, so we guard ourselves with an
         // extra self-only ref.
         guardRef();
@@ -678 +678 @@
         detachParser();
 
-        // removeAllChildren() doesn't always unregister IDs,
+        // removeDetachedChildren() doesn't always unregister IDs,
         // so tear down scope information upfront to avoid having stale references in the map.
         destroyTreeScopeData();
-        removeAllChildren();
+        removeDetachedChildren();
 
         m_markers->detach();

trunk/Source/WebCore/dom/ShadowRoot.cpp

@@ -87 +87 @@
     // runs so we don't go through TreeScopeAdopter for each child with a
     // destructed tree scope in each descendant.
-    removeAllChildren();
+    removeDetachedChildren();
 
     // We must call clearRareData() here since a ShadowRoot class inherits TreeScope

trunk/Source/WebCore/svg/SVGElementInstance.cpp

@@ -102 +102 @@
     m_correspondingUseElement = 0;
 
-    removeAllChildrenInContainer<SVGElementInstance, SVGElementInstance>(this);
+    removeDetachedChildrenInContainer<SVGElementInstance, SVGElementInstance>(this);
 }
 

trunk/Source/WebCore/svg/SVGElementInstance.h

@@ -162 +162 @@
 
     template<class GenericNode, class GenericNodeContainer>
-    friend void removeAllChildrenInContainer(GenericNodeContainer* container);
+    friend void removeDetachedChildrenInContainer(GenericNodeContainer*);
 
     template<class GenericNode, class GenericNodeContainer>