Morph the APIs for Node, ParentNode, and Element closer to the specs

sky/engine/core/dom/Node.cpp

Index: sky/engine/core/dom/Node.cpp
diff --git a/sky/engine/core/dom/Node.cpp b/sky/engine/core/dom/Node.cpp
index 6ff1d2c4ee935c46e4b4327b91585d8c90026f4b..bc38290f44eda819eeabb756e91cd5020a8d6707 100644
--- a/sky/engine/core/dom/Node.cpp
+++ b/sky/engine/core/dom/Node.cpp
@@ -379,6 +379,58 @@ PassRefPtr<Node> Node::appendChild(PassRefPtr<Node> newChild, ExceptionState& ex
     return nullptr;
 }
 
+Element* Node::previousElementSibling()
+{
+    return ElementTraversal::previousSibling(*this);
+}
+
+Element* Node::nextElementSibling()
+{
+    return ElementTraversal::nextSibling(*this);
+}
+
+void Node::newInsertBefore(Vector<RefPtr<Node>>& nodes, ExceptionState& es)
+{
+    RefPtr<Node> protect(this);
+    for (auto& node : nodes) {
+        ContainerNode* parent = parentNode();
+        if (!parent)
+            return;
+        parent->insertBefore(node.release(), this, es);
+        if (es.had_exception())
+            return;
+    }
+}
+
+void Node::newInsertAfter(Vector<RefPtr<Node>>& nodes, ExceptionState& es)
+{
+    RefPtr<Node> refNode = this;
+    for (auto& node : nodes) {

[eseidel, 2015/02/14 00:12:23]

Living the dream.

+        ContainerNode* parent = refNode->parentNode();
+        if (!parent)
+            return;
+        parent->insertBefore(node, refNode.get(), es);
+        if (es.had_exception())
+            return;
+        refNode = node.release();

[esprehn, 2015/02/13 23:53:48]

This doesn't look right, why are you reffing and then dropping each new node as
you go? You need to ref yourself, not each child as you insert them.

[abarth-chromium, 2015/02/13 23:59:45]

The question is how should this code behave if there are tree mutations inside
this loop.  There are a couple of different semantics you could choose.  This
one always inserts the next element in the list after the previous one, even if
you move the previous one to some other crazy place.  Another approach would be
to walk the list backwards and always insert after |this| even if |this| moved
somewhere else, but we thought it was more sensible to insert the elements in
the order given.

[esprehn, 2015/02/14 01:09:25]

There can't be mutations inside this loop.

[abarth-chromium, 2015/02/14 01:28:26]

Ok, I'll change this to ASSERT that.

+    }
+}
+
+void Node::replaceWith(Vector<RefPtr<Node>>& nodes, ExceptionState& es)
+{
+    if (RefPtr<Node> next = m_next) {
+        remove(es);
+        if (es.had_exception())
+            return;
+        next->newInsertBefore(nodes, es);
+    } else if (RefPtr<ContainerNode> parent = parentNode()) {
+        remove(es);
+        if (es.had_exception())
+            return;
+        parent->append(nodes, es);

[esprehn, 2015/02/13 23:53:48]

You don't need this branch, we can just call insertBefore with null for the next
sibling.

[abarth-chromium, 2015/02/13 23:59:45]

I don't follow.  Which function would you like me to call if |m_next| is null?

[esprehn, 2015/02/14 01:09:24]

Just always call

RefPtr<Node> next = m_next;
remove();
parent()->insertBefore(this, m_next);

[abarth-chromium, 2015/02/14 01:28:26]

That doesn't work because |this| isn't a child of |parent| anymore because
|this| is removed on the previous line.

+    }
+}
+
 void Node::remove(ExceptionState& exceptionState)
 {
     if (ContainerNode* parent = parentNode())
@@ -862,7 +914,7 @@ Document* Node::ownerDocument() const
     return doc == this ? 0 : doc;
 }
 
-ContainerNode* Node::ownerScope() const
+ContainerNode* Node::owner() const
 {
     if (inDocument())
         return &treeScope().rootNode();