Node.replaceChild and Node.removeChild do not use custom binding anymore

Source/core/dom/ContainerNode.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2013 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
@@ skipping 300 matching lines @@
     newChild->updateAncestorConnectedSubframeCountForInsertion();
 
     ChildListMutationScope(*this).childAdded(*newChild);
 
     ChildrenChange change = {ChildInserted, newChild->previousSibling(), &nextChild, ChildrenChangeSourceParser};
     childrenChanged(change);
 
     notifyNodeInserted(*newChild);
 }
 
-void ContainerNode::replaceChild(PassRefPtrWillBeRawPtr<Node> newChild, Node* oldChild, ExceptionState& exceptionState)
+PassRefPtrWillBeRawPtr<Node> ContainerNode::replaceChild(PassRefPtrWillBeRawPtr<Node> newChild, PassRefPtrWillBeRawPtr<Node> oldChild, ExceptionState& exceptionState)
 {
 #if !ENABLE(OILPAN)
     // Check that this node is not "floating".
     // If it is, it can be deleted as a side effect of sending mutation events.
     ASSERT(refCount() || parentOrShadowHostNode());
 #endif
 
     RefPtrWillBeRawPtr<Node> protect(this);
 
     if (oldChild == newChild) // nothing to do
-        return;
+        return oldChild;
 
-    if (!oldChild) {
+    if (!oldChild.get()) {

[haraken, 2014/07/12 06:08:12]

.get() is not necessary.

[kangil_, 2014/07/12 06:15:19]

Done.

         exceptionState.throwDOMException(NotFoundError, "The node to be replaced is null.");
-        return;
+        return nullptr;
     }
 
+    RefPtrWillBeRawPtr<Node> child = oldChild;
+
     // Make sure replacing the old child with the new is ok
-    if (!checkAcceptChild(newChild.get(), oldChild, exceptionState))
-        return;
+    if (!checkAcceptChild(newChild.get(), child.get(), exceptionState)) {
+        if (exceptionState.hadException())

[haraken, 2014/07/12 06:08:11]

Is it possible that checkAcceptChild returns false without setting
exceptionState.hadException()?

[kangil_, 2014/07/12 06:15:19]

Please refer to https://codereview.chromium.org/375293002/#msg27
Surprisingly it happened.

+            return nullptr;
+        return child;
+    }
 
     // NotFoundError: Raised if oldChild is not a child of this node.
-    if (oldChild->parentNode() != this) {
+    if (child->parentNode() != this) {
         exceptionState.throwDOMException(NotFoundError, "The node to be replaced is not a child of this node.");
-        return;
+        return nullptr;
     }
 
     ChildListMutationScope mutation(*this);
 
-    RefPtrWillBeRawPtr<Node> next = oldChild->nextSibling();
+    RefPtrWillBeRawPtr<Node> next = child->nextSibling();
 
     // Remove the node we're replacing
-    RefPtrWillBeRawPtr<Node> protectRemovedChild = oldChild;
-    ASSERT_UNUSED(protectRemovedChild, protectRemovedChild);
-    removeChild(oldChild, exceptionState);
+    removeChild(child, exceptionState);
     if (exceptionState.hadException())
-        return;
+        return nullptr;
 
     if (next && (next->previousSibling() == newChild || next == newChild)) // nothing to do
-        return;
+        return child;
 
     // Does this one more time because removeChild() fires a MutationEvent.
-    if (!checkAcceptChild(newChild.get(), oldChild, exceptionState))
-        return;
+    if (!checkAcceptChild(newChild.get(), child.get(), exceptionState)) {
+        if (exceptionState.hadException())

[haraken, 2014/07/12 06:08:11]

Ditto.

[kangil_, 2014/07/12 06:15:19]

Done.

+            return nullptr;
+        return child;
+    }
 
     NodeVector targets;
     collectChildrenAndRemoveFromOldParent(*newChild, targets, exceptionState);
     if (exceptionState.hadException())
-        return;
+        return nullptr;
 
     // Does this yet another check because collectChildrenAndRemoveFromOldParent() fires a MutationEvent.
-    if (!checkAcceptChild(newChild.get(), oldChild, exceptionState))
-        return;
+    if (!checkAcceptChild(newChild.get(), child.get(), exceptionState)) {
+        if (exceptionState.hadException())

[haraken, 2014/07/12 06:08:12]

Ditto.

[kangil_, 2014/07/12 06:15:19]

Done.

+            return nullptr;
+        return child;
+    }
 
     InspectorInstrumentation::willInsertDOMNode(this);
 
     // Add the new child(ren)
     for (NodeVector::const_iterator it = targets.begin(); it != targets.end(); ++it) {
         ASSERT(*it);
         Node& child = **it;
 
         // Due to arbitrary code running in response to a DOM mutation event it's
         // possible that "next" is no longer a child of "this".
@@ skipping 12 matching lines @@
             if (next)
                 insertBeforeCommon(*next, child);
             else
                 appendChildCommon(child);
         }
 
         updateTreeAfterInsertion(child);
     }
 
     dispatchSubtreeModifiedEvent();
+    return child;
 }
 
 void ContainerNode::willRemoveChild(Node& child)
 {
     ASSERT(child.parentNode() == this);
     ChildListMutationScope(*this).willRemoveChild(child);
     child.notifyMutationObserversNodeWillDetach();
     dispatchChildRemovalEvents(child);
     document().nodeWillBeRemoved(child); // e.g. mutation event listener can create a new range.
     ChildFrameDisconnector(child).disconnect();
@@ skipping 87 matching lines @@
     ChildFrameDisconnector(*this).disconnect();
 }
 
 void ContainerNode::trace(Visitor* visitor)
 {
     visitor->trace(m_firstChild);
     visitor->trace(m_lastChild);
     Node::trace(visitor);
 }
 
-void ContainerNode::removeChild(Node* oldChild, ExceptionState& exceptionState)
+PassRefPtrWillBeRawPtr<Node> ContainerNode::removeChild(PassRefPtrWillBeRawPtr<Node> oldChild, ExceptionState& exceptionState)
 {
 #if !ENABLE(OILPAN)
     // Check that this node is not "floating".
     // If it is, it can be deleted as a side effect of sending mutation events.
     ASSERT(refCount() || parentOrShadowHostNode());
 #endif
 
     RefPtrWillBeRawPtr<Node> protect(this);
 
     // NotFoundError: Raised if oldChild is not a child of this node.
     // FIXME: We should never really get PseudoElements in here, but editing will sometimes
     // attempt to remove them still. We should fix that and enable this ASSERT.
     // ASSERT(!oldChild->isPseudoElement())
-    if (!oldChild || oldChild->parentNode() != this || oldChild->isPseudoElement()) {
+    if (!oldChild.get() || oldChild->parentNode() != this || oldChild->isPseudoElement()) {

[haraken, 2014/07/12 06:08:12]

.get() is not necessary.

[kangil_, 2014/07/12 06:15:19]

Done.

         exceptionState.throwDOMException(NotFoundError, "The node to be removed is not a child of this node.");
-        return;
+        return nullptr;
     }
 
     RefPtrWillBeRawPtr<Node> child = oldChild;
 
     document().removeFocusedElementOfSubtree(child.get());
 
     if (FullscreenElementStack* fullscreen = FullscreenElementStack::fromIfExists(document()))
         fullscreen->removeFullScreenElementOfSubtree(child.get());
 
     // Events fired when blurring currently focused node might have moved this
     // child into a different parent.
     if (child->parentNode() != this) {
         exceptionState.throwDOMException(NotFoundError, "The node to be removed is no longer a child of this node. Perhaps it was moved in a 'blur' event handler?");
-        return;
+        return nullptr;
     }
 
     willRemoveChild(*child);
 
     // Mutation events might have moved this child into a different parent.
     if (child->parentNode() != this) {
         exceptionState.throwDOMException(NotFoundError, "The node to be removed is no longer a child of this node. Perhaps it was moved in response to a mutation?");
-        return;
+        return nullptr;
     }
 
     {
         HTMLFrameOwnerElement::UpdateSuspendScope suspendWidgetHierarchyUpdates;
 
         Node* prev = child->previousSibling();
         Node* next = child->nextSibling();
         removeBetween(prev, next, *child);
         ChildrenChange change = {ChildRemoved, prev, next, ChildrenChangeSourceAPI};
         childrenChanged(change);
         notifyNodeRemoved(*child);
     }
     dispatchSubtreeModifiedEvent();
+    return child;
 }
 
 void ContainerNode::removeBetween(Node* previousChild, Node* nextChild, Node& oldChild)
 {
     NoEventDispatchAssertion assertNoEventDispatch;
 
     ASSERT(oldChild.parentNode() == this);
 
     if (!oldChild.needsAttach())
         oldChild.detach();
@@ skipping 798 matching lines @@
         return true;
 
     if (node->isElementNode() && toElement(node)->shadow())
         return true;
 
     return false;
 }
 #endif
 
 } // namespace WebCore