Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(201)

Unified Diff: third_party/WebKit/Source/core/dom/ContainerNode.cpp

Issue 2317573003: Introduce ContainerNode::insertNodeVector() to share code in insertBefore(), appendChild(), and rep… (Closed)
Patch Set: Update a copied comment; "this" -> "container" Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « third_party/WebKit/Source/core/dom/ContainerNode.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: third_party/WebKit/Source/core/dom/ContainerNode.cpp
diff --git a/third_party/WebKit/Source/core/dom/ContainerNode.cpp b/third_party/WebKit/Source/core/dom/ContainerNode.cpp
index d948b388f951a9e755aad04c8b9da44f09f8c28c..35e72222042f727a8808d2f804a360cde7f92763 100644
--- a/third_party/WebKit/Source/core/dom/ContainerNode.cpp
+++ b/third_party/WebKit/Source/core/dom/ContainerNode.cpp
@@ -150,6 +150,59 @@ bool ContainerNode::checkAcceptChildGuaranteedNodeTypes(const Node& newChild, co
return true;
}
+template <typename Functor>
+void ContainerNode::insertNodeVector(const NodeVector& targets, Node* next, const Functor& mutator)
+{
+ InspectorInstrumentation::willInsertDOMNode(this);
+ for (const auto& targetNode : targets) {
+ DCHECK(targetNode);
+ {
+ EventDispatchForbiddenScope assertNoEventDispatch;
+ ScriptForbiddenScope forbidScript;
+
+ if (!mutator(*this, *targetNode, next))
+ break;
+ }
+ updateTreeAfterInsertion(*targetNode);
+ }
+ dispatchSubtreeModifiedEvent();
+}
+
+class ContainerNode::AdoptAndInsertBefore {
+public:
+ bool operator()(ContainerNode& container, Node& child, Node* next) const
+ {
+ DCHECK(next);
+ // Due to arbitrary code running in response to a DOM mutation event
+ // it's possible that "next" is no longer a child of "container".
+ // It's also possible that "child" has been inserted elsewhere. In
+ // either of those cases, we'll just stop.
+ if (next->parentNode() != &container)
+ return false;
+ if (child.parentNode())
+ return false;
+
+ container.treeScope().adoptIfNeeded(child);
+ container.insertBeforeCommon(*next, child);
+ return true;
+ }
+};
+
+class ContainerNode::AdoptAndAppendChild {
+public:
+ bool operator()(ContainerNode& container, Node& child, Node* next) const
+ {
+ // If the child has a parent again, just stop what we're doing, because
+ // that means someone is doing something with DOM mutation -- can't
+ // re-parent a child that already has a parent.
+ if (child.parentNode())
+ return false;
+ container.treeScope().adoptIfNeeded(child);
+ container.appendChildCommon(child);
+ return true;
+ }
+};
+
Node* ContainerNode::insertBefore(Node* newChild, Node* refChild, ExceptionState& exceptionState)
{
// insertBefore(node, 0) is equivalent to appendChild(node)
@@ -191,35 +244,8 @@ Node* ContainerNode::insertBefore(Node* newChild, Node* refChild, ExceptionState
return newChild;
}
- InspectorInstrumentation::willInsertDOMNode(this);
-
ChildListMutationScope mutation(*this);
- for (const auto& targetNode : targets) {
- DCHECK(targetNode);
- Node& child = *targetNode;
-
- // Due to arbitrary code running in response to a DOM mutation event it's
- // possible that "next" is no longer a child of "this".
- // It's also possible that "child" has been inserted elsewhere.
- // In either of those cases, we'll just stop.
- if (next->parentNode() != this)
- break;
- if (child.parentNode())
- break;
-
- {
- EventDispatchForbiddenScope assertNoEventDispatch;
- ScriptForbiddenScope forbidScript;
-
- treeScope().adoptIfNeeded(child);
- insertBeforeCommon(*next, child);
- }
-
- updateTreeAfterInsertion(child);
- }
-
- dispatchSubtreeModifiedEvent();
-
+ insertNodeVector(targets, next, AdoptAndInsertBefore());
return newChild;
}
@@ -366,37 +392,10 @@ Node* ContainerNode::replaceChild(Node* newChild, Node* oldChild, ExceptionState
return child;
}
- InspectorInstrumentation::willInsertDOMNode(this);
-
- // Add the new child(ren).
- for (const auto& targetNode : targets) {
- DCHECK(targetNode);
- Node& child = *targetNode;
-
- // Due to arbitrary code running in response to a DOM mutation event it's
- // possible that "next" is no longer a child of "this".
- // It's also possible that "child" has been inserted elsewhere.
- // In either of those cases, we'll just stop.
- if (next && next->parentNode() != this)
- break;
- if (child.parentNode())
- break;
-
- treeScope().adoptIfNeeded(child);
-
- // Add child before "next".
- {
- EventDispatchForbiddenScope assertNoEventDispatch;
- if (next)
- insertBeforeCommon(*next, child);
- else
- appendChildCommon(child);
- }
-
- updateTreeAfterInsertion(child);
- }
-
- dispatchSubtreeModifiedEvent();
+ if (next)
+ insertNodeVector(targets, next, AdoptAndInsertBefore());
+ else
+ insertNodeVector(targets, nullptr, AdoptAndAppendChild());
return child;
}
@@ -625,32 +624,8 @@ Node* ContainerNode::appendChild(Node* newChild, ExceptionState& exceptionState)
return newChild;
}
- InspectorInstrumentation::willInsertDOMNode(this);
-
- // Now actually add the child(ren).
ChildListMutationScope mutation(*this);
- for (const auto& targetNode : targets) {
- DCHECK(targetNode);
- Node& child = *targetNode;
-
- // If the child has a parent again, just stop what we're doing, because
- // that means someone is doing something with DOM mutation -- can't re-parent
- // a child that already has a parent.
- if (child.parentNode())
- break;
-
- {
- EventDispatchForbiddenScope assertNoEventDispatch;
- ScriptForbiddenScope forbidScript;
-
- treeScope().adoptIfNeeded(child);
- appendChildCommon(child);
- }
-
- updateTreeAfterInsertion(child);
- }
-
- dispatchSubtreeModifiedEvent();
+ insertNodeVector(targets, nullptr, AdoptAndAppendChild());
return newChild;
}
« no previous file with comments | « third_party/WebKit/Source/core/dom/ContainerNode.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698