Node.appendChild should throw TypeError when newChild is not an object

Source/bindings/core/v8/custom/V8NodeCustom.cpp

 /*
  * Copyright (C) 2007-2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 89 matching lines @@
     ExceptionState exceptionState(ExceptionState::ExecutionContext, "removeChild", "Node", info.Holder(), info.GetIsolate());
     Node* oldChild = V8Node::toNativeWithTypeCheck(info.GetIsolate(), info[0]);
     impl->removeChild(oldChild, exceptionState);
     if (exceptionState.throwIfNeeded())
         return;
     v8SetReturnValue(info, info[0]);
 }
 
 void V8Node::appendChildMethodCustom(const v8::FunctionCallbackInfo<v8::Value>& info)
 {
-    v8::Handle<v8::Object> holder = info.Holder();
-    Node* impl = V8Node::toNative(holder);
-
+    ExceptionState exceptionState(ExceptionState::ExecutionContext, "appendChild", "Node", info.Holder(), info.GetIsolate());
+    if (UNLIKELY(info.Length() < 1)) {
+        throwMinimumArityTypeErrorForMethod("appendChild", "Node", 1, info.Length(), info.GetIsolate());

[haraken, 2014/07/09 01:38:53]

Can we make this change in a separate change with a test?

[kangil_, 2014/07/09 02:53:20]

Sure, I will.

+        return;
+    }
+    Node* impl = V8Node::toNative(info.Holder());
     CustomElementCallbackDispatcher::CallbackDeliveryScope deliveryScope;
-
-    ExceptionState exceptionState(ExceptionState::ExecutionContext, "appendChild", "Node", info.Holder(), info.GetIsolate());
-    Node* newChild = V8Node::toNativeWithTypeCheck(info.GetIsolate(), info[0]);
+    Node* newChild;
+    {
+        v8::TryCatch block;
+        V8RethrowTryCatchScope rethrow(block);
+        if (info.Length() > 0 && !V8Node::hasInstance(info[0], info.GetIsolate())) {
+            exceptionState.throwTypeError("parameter 1 is not of type 'Node'.");
+            exceptionState.throwIfNeeded();
+            return;
+        }
+        TONATIVE_VOID_INTERNAL(newChild, V8Node::toNativeWithTypeCheck(info.GetIsolate(), info[0]));
+    }
     impl->appendChild(newChild, exceptionState);
-    if (exceptionState.throwIfNeeded())
+    if (exceptionState.hadException()) {
+        exceptionState.throwIfNeeded();
         return;
+    }
     v8SetReturnValue(info, info[0]);
 }
 
 v8::Handle<v8::Object> wrap(Node* impl, v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)
 {
     ASSERT(impl);
     switch (impl->nodeType()) {
     case Node::ELEMENT_NODE:
         // For performance reasons, this is inlined from V8Element::wrap and must remain in sync.
         if (impl->isHTMLElement())
@@ skipping 17 matching lines @@
         return wrap(toDocumentType(impl), creationContext, isolate);
     case Node::DOCUMENT_FRAGMENT_NODE:
         if (impl->isShadowRoot())
             return wrap(toShadowRoot(impl), creationContext, isolate);
         return wrap(toDocumentFragment(impl), creationContext, isolate);
     }
     ASSERT_NOT_REACHED();
     return V8Node::createWrapper(impl, creationContext, isolate);
 }
 } // namespace WebCore